sitegeist / Sitegeist.Archaeopteryx

The missing link editor for Neos
MIT License
21 stars 12 forks source link

add a new link variant : Custom Link #27

Closed FahimNsr closed 11 months ago

FahimNsr commented 1 year ago

Custom Link

The CustomLink link type allows the editor to add a link in any format. With this you give the editor an escape hatch if the other link types are insufficient for the desired use case. As any other link type the CustomLink can be deactivated via the configuration. If you want to deactivate the CustomLink link type globally you can use a preset and a mixin, which is than used as a supertype in all link generating node types:

Define the preset:

Neos:
  Neos:
    nodeTypes:
      presets:
        properties:
          myVendor:
            noCustomLinkEditor:
              ui:
                inspector:
                  editor: 'Sitegeist.Archaeopteryx/Inspector/Editors/LinkEditor'
                  editorOptions:
                    linkTypes:
                      'Sitegeist.Archaeopteryx:CustomLink':
                        enabled: false

Use the preset in your link property, mixin or node type

'MyVendor.Base:Mixin.Link':
  abstract: true
  properties:
    link:
      options:
        preset: 'myVendor.noCustomLinkEditor'
lorenzulrich commented 1 year ago

@FahimNsr Without further description it's a little hard to imagine what the idea behind this feature was?

mhsdesign commented 12 months ago

I think its for entering links that are not convered by the other tabs.

For example

    isSuitableFor: (link: ILink) => !link.href.startsWith('asset://') && !link.href.startsWith('mailto:')
         && !link.href.startsWith('node://') && !link.href.startsWith('tel:')
         && !link.href.startsWith('http://') && !link.href.startsWith('https://'),

we only handle links with protocols we dont know.

But there is a catch. If we enter a link still starting with a known protocol, but continue to write something the default implementation doesnt cover, like adding an anchor to asset uris: asset://456877568#anchor. When opening the editor the next time this link will be opened in the asset editor again, which leads to loss of the anchor. ... this is just an edgecase we need to be aware of.

For cases like relative urls, fragments and unknown protocols it would work:

but we should rather focus on your real usecase ;) I dont think an editor knows about every detail of the uri specification and knows what to put in there.

mficzel commented 11 months ago

I understand the feature and think that it is a valid usecase a description in the pr would be nice to really get the use-case. The drawback @mhsdesign mentioned is valid but i think we can live with that. In the end this is for the cases we did not plan for so using it to edit protocols that have a dedicated dialog is your own fault.

It might make sense to have this as opt-in feature that has to be enabled.

lorenzulrich commented 10 months ago

@mficzel IMO it would have been ideal to disable this editor by default. This was released in a minor version and so everyone updating will have an additional link editor they might not need. Not a problem in my WIP project but just as a feedback regarding versioning. Thanks!

mhsdesign commented 10 months ago

@lorenzulrich i agree especially cause its such an edgecase feature and some editors might use it as footgun.

I thought we agreed on this? ^^

It might make sense to have this as opt-in feature that has to be enabled.

im a bit unhappy that this was merged so eagerly by @andrehoffmann30 ... but nothing a followup fix cant solve ;)

mhsdesign commented 4 months ago

So @andrehoffmann30 and martin rediscussed this again, and sorry for my hard words previously but we agreed to keep this feature