Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove LinkControl #24099

Closed
adamziel opened this issue Jul 21, 2020 · 6 comments
Closed

Remove LinkControl #24099

adamziel opened this issue Jul 21, 2020 · 6 comments
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting)

Comments

@adamziel
Copy link
Contributor

adamziel commented Jul 21, 2020

tl;dr:

Let’s completely remove the concept of LinkControl as it exists today:

88043215-72f80a80-cb4d-11ea-8341-249ae8ac1188

And expose the basic building blocks via @wordpress/components instead (names and details are up for discussion):

Zrzut ekranu 2020-07-21 o 18 47 27

Motivation

The link editing experience across Gutenberg is far from being consistent. We have plenty of link editing components and UI, most of them could be disposed of. Recently @shaunandrews proposed an in-toolbar link editing UI (#23375) which I really like. I only have two problems with it: 1. It’s yet another link editing interface 2. It requires using our basic building blocks in a ways they were not designed/programmed for. During my refactoring journey (#23869) I found that the link editing UI is a subject of endless issues and debates - there are bugs, missing features, inflexible abstractions. So instead of adding another UI on top of what’s already in place, I decided to take a look at the big picture first and see if there’s an elegant way of using this opportunity as a launchpad to solving the bulk of our link editing problems.

All existing link editing UIs listed in one place

To keep this issue nice and short, I listed all the existing UIs in a dedicated issue here: #24089

All the related issues listed in one place

The list below was put together by @youknowriad in the comment under a related PR:

#18061 #22439 #22438 #22437 #22435 #20271 #20138 #20136 #20051 #19526 #17293

The gist of these issues is that:

  • Every URL-editing UI should provide a consistent way of changing rel (at least noreferrer/noopener), target (at least new tab or same tab), a custom CSS class.
  • The code isn’t very well structured or DRY, features are being reinvented, building new things is not trivial.
  • A11y should be first-class citizen.

Analysis

We would have to either:

  • Maintain multiple link-editing components (LinkControl and all it’s composites + in-toolbar editing) and solve the above (and all the future) issues in all context, OR
  • Focus on the in-toolbar UI as the primary one for link editing, remove LinkControl altogether, use smaller building blocks as needed in cases where we only want to search for a link and not actually edit one.

The in-toolbar link editing by @shaunandrews (#23375) could replace almost all LinkControl usages listed in #24089. It would likely need some more discussion to decide about e.g. editing a CSS class, or linking to a media file, but apart from these tactical details it’s just what we need in general.

The only remaining LinkControl instance that wouldn’t be affected by the introduction of in-toolbar editing is PageSwitcher (see #24089):

Zrzut ekranu 2020-07-21 o 14 38 06-kopia

It doesn’t use the full power of LinkControl too - it’s just an input, a loading indicator, and navigable suggestions. There is no link preview, „create new page” control, or a way to use verbatim input as typed.

So I’d say, let’s remove the LinkControl component as it exists today:

  1. Most use-cases are better served by in-toolbar editing so we’re getting consistency for free.
  2. Those that aren’t could be trivially composed inline with proper building blocks available.
  3. The bugs and feature requests won’t have to be implemented in multiple set of UIs.

CC @talldan @youknowriad @shaunandrews @draganescu @tellthemachines @noisysocks @getdave

@adamziel adamziel self-assigned this Jul 21, 2020
@adamziel adamziel added [Block] Navigation Affects the Navigation Block [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) labels Jul 21, 2020
@tellthemachines
Copy link
Contributor

Great work collecting all the examples and thinking through this ⭐

I fully agree we shouldn't maintain multiple link editing components. What I'm not sure about is point 2:

Those that aren’t could be trivially composed inline with proper building blocks available.

The problem with this is that the input and the list of results aren't easily separable, because each have to contain aria attributes, labels and ids pointing to one another in order for the whole thing to make sense to a screen reader. So, my questions:

  • What are the functional differences between the in-toolbar link editing component you propose as the default, and the instances where you don't see it as a good fit? Would we be able to consolidate them as a single component with perhaps a few extra props?

  • Could we instead use a kind of skeleton component for the UI, such as ComboboxControl, that would accept different child components for rendering of results? (ComboboxControl doesn't do that right now, but perhaps we could alter it.)

@youknowriad
Copy link
Contributor

Thanks for starting this discussion and good analysis.

The screenshots above are very inconsistent, some come from old designs, some from half-refactored ones and new ones never implemented. I still have doubts about whether removing LinkControl is the way to go for several reasons:

  • LinkControl is a comprehensible API: a form control equivalent to the dozen *Control components we already have in the components package.
  • In the last screenshots (complex compositions), Both use inputs with suggestions, one is wrapped in a div/popover and one is in the toolbar, what if the common UI there is the "LinkControl" component? (Seems like you suggest that it's URLinput above but named as LinkControl is more inline with the rest of our components)
  • I'm concerned about exposing smaller components like "LinkSuggetion", "LinkPreview", "CreatePageLinkSuggestion"... As these will extend the public API of our ui components without a clear sense of whether they're to be used separately instead of using LinkControl directly.

I'd love to see desired mockups for all the actual use-cases in Gutenberg right now, as this would help us figure out the best API we want to expose.

@adamziel
Copy link
Contributor Author

adamziel commented Jul 22, 2020

The problem with this is that the input and the list of results aren't easily separable, because each have to contain aria attributes, labels and ids pointing to one another in order for the whole thing to make sense to a screen reader.

They are already separated though, the URLInput component accepts a __experimentalRenderSuggestions prop which is a callback that renders suggestions. This issue is more about rethinking higher-order abstractions than making difficult changes to how the internals are wired.

What are the functional differences between the in-toolbar link editing component you propose as the default, and the instances where you don't see it as a good fit?

I see it as a good fit for all the instances of editing a link that's somehow related to a block: a button, a media block, a link inside a paragraph block. It wouldn't work where there is no block and toolbar to start with, but at the moment we don't have any link editing in a non-block context so that's not a big issue.

Would we be able to consolidate them as a single component with perhaps a few extra props?

Potentially, it's always a tradeoff between customizability and the ease of use.

Could we instead use a kind of skeleton component for the UI, such as ComboboxControl, that would accept different child components for rendering of results? (ComboboxControl doesn't do that right now, but perhaps we could alter it.)

Potentially, sounds like something that's worth exploring 👍

I'd love to see desired mockups for all the actual use-cases in Gutenberg right now, as this would help us figure out the best API we want to expose.

@youknowriad you just described the scope of the design work required to bring in-toolbar editing to all the blocks :-) Maybe this issue came ahead of it's time - let's keep moving in-toolbar editing forward and have more atomic discussions on a per-use case basis in each specific PR. If/when in-toolbar editing is brought to all the blocks, let's revisit this issue. CC @shaunandrews

@tellthemachines
Copy link
Contributor

They are already separated though, the URLInput component accepts a __experimentalRenderSuggestions prop which is a callback that renders suggestions.

True, but it has a default suggestions markup with the correct attributes, and that experimental prop is only being used in LinkControl for now. I'd be much more comfortable if URLInput provided at least the base markup for the suggestions popover, and only took in a prop for styling of individual suggestions, as there would be less scope for the accessibility of the component to go wrong. Not sure how that would work if the suggestions had multiple style types though.

@getdave
Copy link
Contributor

getdave commented Jun 9, 2021

I'm still in favour of retaining <LinkControl>. I don't think we should underestimate how much this has allowed us to unify the experience of creating/adding a hyperlink within Gutenberg.

By removing this control we'd be going backwards towards asking developers to manually wire together several highly complex components. This will likely return us to a fragmented user experience and add additional overhead where's it's not needed.

@draganescu
Copy link
Contributor

This was heavily based on this

Most use-cases are better served by in-toolbar editing so we’re getting consistency for free.

which never materialized because of the difficult a11y issues. I'll close this, we can come back to this idea once we have a clear alternative in place of LinkControl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting)
Projects
None yet
Development

No branches or pull requests

5 participants