FIX Add default link type title and icon

[sabina-talipova]

Parent issue

• Link type title and icon are not displayed if link record exist, but link type is not longer available for creating #176

[GuySartorelli]

This PR is still in draft, but in another comment you indicated it might be ready for merge - should this still be a draft?

[sabina-talipova]

Thank you. Yes, It's ready.

[GuySartorelli]

It would be better if the correct icon and "title" were used, since that information does exist. But that would require a bit more refactoring since that information is currently included in the types prop rather than as part of the link data.

I'm on the fence about whether that would be worth the effort.

@emteknetnz do you have any thoughts on this one? This is what it looks like with this PR if only a phone link is allowed:

It is an improvement from the current behaviour and I'm inclined to merge as-is, but I want a second opinion so we can avoid coming back to this later and going "we should show the actual icon, why are we doing this?" and redoing this work.

[emteknetnz]

Making the link read only as mentioned at https://silverstripeltd.slack.com/archives/GNHGJT5G8/p1705615609845979?thread_ts=1705612719.609119&cid=GNHGJT5G8 seems like a much better solution. Seems wrong to be able to edit existing though disallowed links

I think we'll need to refactor some PHP and JS so that ALL types including disallowed are passed by the Ajax request, though also add in an allowed bool for each link type

[GuySartorelli]

Making the link read only as mentioned at https://silverstripeltd.slack.com/archives/GNHGJT5G8/p1705615609845979?thread_ts=1705612719.609119&cid=GNHGJT5G8 seems like a much better solution. Seems wrong to be able to edit existing though disallowed links

This PR and issue has nothing to do with functionality - it's just about how the link is displayed in the list (specifically the icon and the "External link"/"Phone number"/"Link"/etc text). So that read-only part is out of scope, and will be dealt with separately.

I think we'll need to refactor some PHP and JS so that ALL types including disallowed are passed by the Ajax request, though also add in an allowed bool for each link type

Does this mean you want to show the correct link for any given link, regardless of whether that link is allowed? That was ultimately the question here.

[emteknetnz]

I'd say close this PR, if we're going to change this to readonly showing the correct icons then there's no point adding the code in this PR because we'll ultimately just remove it

[GuySartorelli]

I don't understand your logic? Making the modal readonly doesn't affect what icon is displayed.

[emteknetnz]

Sorry, including what I said #177 (comment) - show the correct icon + link type text rather than defaults

[GuySartorelli]

As discussed, please update this PR to show the correct icon and text for links, even when those links are not in the "allowed" list.

Some ways you could do that:

1. Always include that information with the link data itself (not optimal, since this isn't dependent on the link record - it's dependent on the link type, so ideally go for option 2)
2. Update the link types prop so that it always includes information about all link types, and includes an additional boolean for each type to indicate if that type is "allowed" or not.