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

How we can approach the introduction of a new link component within Gutenberg #18061

Closed
getdave opened this issue Oct 22, 2019 · 14 comments
Closed
Assignees
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Feature] UI Components Impacts or related to the UI component system [Package] Block editor /packages/block-editor

Comments

@getdave
Copy link
Contributor

getdave commented Oct 22, 2019

An effort is underway to create a new unified interface for adding / editing links within the editor. It may look something like this:

Screen Shot 2019-10-22 at 14 49 16

Work is already underway to implement this.

What follows below is an analysis and recommendation for how we can go about introducing this new standardised link interface into Gutenberg.

This has been prompted by several discussions as to the best technical route to take in order to allow us to introduce this new UI across the editor with a minimum of disruption.

I will begin with an overview of the existing interface components that provide link-related UI.

I will then analyse how/whether it is possible to modify the existing components to realise the new link UI design.

Following on from this, I will then discuss how the current link interfaces within Gutenberg are created.

Finally, I will propose a route forward centred around creating a single unified component for link interfaces within Gutenberg.

Overview of existing "link" Components

The following components are already available within Gutenberg and are used for creating "link" interfaces.

URLInput

Screen Capture on 2019-10-08 at 11-14-07

  • desc: a text input that accepts a search query or a URL. Provides search suggestions for matching Posts and Pages as you type. URLs are not handled as search suggestions.
  • location: packages/block-editor/src/components/url-input
  • features/functionality:
    • text input box for typing searches or for URL entry
    • search suggestion list appears when typing
    • search results "fetching"
    • a11y affordances around
      • listbox ARIA attrs
      • keyboard controls
      • focus management
  • usage notes: this component is almost always used as part of URLPopover.LinkEditor and is only ever used in isolation within packages/block-library/src/button/edit.js. This component is low level. It does not provide a popover or a settings drawer. Therefore when used in isolation it will not provide the design for the new Link UI. Therefore it is ill suited as a wrapper component.

URLPopover

Screen Capture on 2019-10-08 at 11-24-39
Screen Capture on 2019-10-08 at 11-21-19

  • desc: renders a popover for displaying a UI for adding/editing a link. Comes with a "drawer" for rendering contextual items relating to the URL (eg: additional settings such as "Open in New Tab"). Utilises URLInput and ExternalLink internally.
  • location: packages/block-editor/src/components/url-popover
  • features/functionality:
    • popover UI which displays above other content and contains the rest of the link UI
    • toggable link settings area - often used to display "new tab"...etc
    • additional controls area - space to render any custom UI within the popover but undereath the primary UI elements
  • usage notes: this component is most often utilised as a wrapper around URLInput, with URLInput provinding the input and search suggestions and URLPopover providing the settings drawer, "selected" states and a popover in which to contain the UI.

URLPopover also exposes the following sub components:

URLPopover.LinkEditor

  • location: packages/block-editor/src/components/url-popover/link-editor.js

This provides a simple wrapper around URLInput to provide a <form> tag and a Apply button.

URLPopover.LinkViewer

  • location: packages/block-editor/src/components/url-popover/link-viewer.js

This provides a simple style for "selected" links. It is a wrapper around ExternalLink and also provides an "Edit" button.

How are link interfaces currently creating within Gutenberg?

As it stands the two existing components URLInput and URLPopover are largely low level components. Therefore, it is important to appreciate that in almost every case where a link interface is required within Gutenberg, these components are consumed and controlled by the parent component (via props) to create a bespoke interface.

This happens each time a link UI is created.

Some good examples of this are:

What we can observe here is that each time an interface for adding links is required, the developer is currently required to wire together a bespoke implementation.

In short, there is currently no unified interface component for creating link UIs in Gutenberg.

Conclusion and Recommendation

Based on the evidence above, it appears that what is required is not extensive modification or refactoring of the existing URL* components, but rather the creation of a single component which wires together the lower-level components to provide a unified link interface.

Given that URLInput is a low level component which is already in use, attempting to refactor this to be a wrapper around a more complex, fully unified Link UI would be complex, time-consuming and provide little or no benefit.

Similarly, URLPopover is largely a visual component and should not be a candidate for managing state or a fully unified UI.

Therefore, the most effective and expedient solution is to create a new component which unifies the existing components under a single interface.

By doing this we will provide the following benefits:

  • developers will only need to consume a single component to create a link UI.
  • standardising under a single link UI component will bring visual and UX consistency to link interfaces throughout Gutenberg.
  • work on existing URLInput and URLPopover components can be preserved and reused - no need for deprecations.
  • small modifications to existing URLInput and URLPopover will make these more flexible for future if/when required to create bespoke/highly customised link UIs that cannot be serviced by the unified component.

The new component can be relased as "experimental" and then once accepted, it can gradually used to replace the existing bespoke usages of URLPopover and URLInput thereby standardising link intefaces across Gutenberg.

This approach has already been trialled in this PR.

Appendix A: do we need to modify the existing components to deliver the new design?

The following changes are required to both components in order to realise the new design:

Changes to URLInput

  • Update rendering of suggestion items to add in more information about the suggestion (see Design) - this can be achieved by either:

    • modifying the search suggestions render within the URLInput directly (note: this means all existing implementations will inherit this change).
    • adding a optional render prop to URLInput to allow parent/consuming components to modify the rendered output of suggestion items (as per current LinkControl PR).
  • Update the placeholder on the input - this is most easily achieved by exposing a placeholder prop on URLInput.

  • Fix to ensure that suggestions are hidden when the input is empty.

  • Design requires that when a URL is entered this is displayed as a search suggestion. Currently URLInput will ignore URLs when calling the function prop which fetches search suggestion data. This can be fixed by:

Changes to URLPopover

Given that it is mainly a wrapper component, it is unlikely that it would be useful to make changes to URLPopover. Almost every example of a Link UI within Gutenberg consumes URLPopover as a controlled component. All state management and customisation is handled outside of URLPopover.

The only major changes required to realise the new design would be:

Appendix B: Related Links

@getdave getdave added Needs Technical Feedback Needs testing from a developer perspective. [Package] Block editor /packages/block-editor labels Oct 22, 2019
@mtias mtias added the [Feature] UI Components Impacts or related to the UI component system label Oct 25, 2019
@draganescu draganescu removed their assignment Dec 19, 2019
@aduth
Copy link
Member

aduth commented Jan 21, 2020

I left a comment at #19523 (comment) which might have been more appropriate to discuss here.

In that comment, I included an annotated screenshot of what I considered to be proposed as the layers of abstraction in some final implementation of the new link creation UI:

image

I do kinda wonder / worry, especially in considering a separation between LinkControl and Popover, whether the design of LinkControl is reusable enough outside of how we're currently using it in popovers.

Especially the UI for "viewing" a link, like in the screenshot of #17557 (comment), where it's heavily optimized to be assumed to be shown in a popover. I doubt it would work quite as well if it were detached from Popover. It's fair to say that popovers would be the most common usage, but then it raises the question whether there's a ton of value in separating it from Popover (like in @youknowriad's #19638). Personally, I do like the idea of considering these as small composable units, so the refactor feels good to me, but it does leave LinkControl in this awkward place to not be very useful outside a Popover.

@chrisvanpatten
Copy link
Contributor

I could see cases where LinkControl would make sense as an inspector control, or within a modal. It would be nice to see it usable outside a popover, where perhaps the combobox area is visually rendered like a select dropdown or something like that. Sort of an "inline" mode vs a "dropdown" mode or something along those lines.

@draganescu
Copy link
Contributor

There already is in #19504 the need to use LinkControl outside of Popover in the new MediaReplaceControl and that is already using a popover b/c of the dropdown displaying all the toolbar menus.

@aduth
Copy link
Member

aduth commented Jan 22, 2020

I could see cases where LinkControl would make sense as an inspector control, or within a modal. It would be nice to see it usable outside a popover, where perhaps the combobox area is visually rendered like a select dropdown or something like that. Sort of an "inline" mode vs a "dropdown" mode or something along those lines.

In the above model, I think URLInput already satisfies much of the common usage for what you describe (at least in rendering an input and suggestions associated with its value at a given time).

My understanding of LinkControl's value proposition is in composing URLInput and extending it to...

  • Behave consistently with other "Control" suffixed components (like TextControl), where the implication is often a pairing of label and (sometimes) help text
  • Incorporate management of settings values which relate to how the chosen URL would be applied as a hyperlink ("Open in New Tab", etc)
  • (Maybe:) Present itself in a way wherein the display of the control would default to a read-only representation, and can be toggled into an editing "mode"

I think this is more-or-less satisfied in its current implementation, but the specifics of the display are heavily optimized to assume that it would be shown in a Popover component, and I don't think would work particularly well outside of one.

Consider the screenshot from #17557 (comment):

LinkControl

  • Would the arrangement of the "Change" button work very well in a wider container, or would it start to lose its association to the adjacent link text?
    • Maybe the default behavior should render the Change button in a vertical orientation (under the link text, flex-direction: column), and the usage in the popover can reorient this as customized styling (flex-direction: row)
  • Does the horizontal rule hr in the screenshot work well outside a Popover ? I seem to recall that the Block Inspector sidebar has some padding, so it could cause some gaps on the left-and-right edges. Additionally, it might mislead a user to think that the "Open in New Tab" is separate -- and therefore not related to -- the link above the line.
    • Again, maybe it's simply a matter of changing the default to avoid the use of the line, and re-add this as part of the styling specific to its usage in the popover.

@aduth
Copy link
Member

aduth commented Jan 22, 2020

Since images can probably speak better than words in this instance, I'll try to illustrate with screenshots.

Consider that I have a modal which renders a form. I might expect something like the following code:

function ExampleForm() {
	return (
		<Modal title="Example Form">
			<TextControl label="First name" />
			<TextControl label="Last name" />
			<LinkControl label="Website" />
			<SelectControl label="Country" options={ [ { label: 'United States', value: 'us' } ] } />
		</Modal>
	);
}

As far as the usage of the components, they are quite aligned. But for the rendered output, you can see that one of these is not like the rest:

Screen Shot 2020-01-22 at 1 57 27 PM

The same example, with a value assigned:

function ExampleForm() {
	return (
		<Modal title="Example Form">
			<TextControl label="First name" />
			<TextControl label="Last name" />
			<LinkControl label="Website" value={ { url: 'https://example.com' } } />
			<SelectControl label="Country" options={ [ { label: 'United States', value: 'us' } ] } />
		</Modal>
	);
}

Screen Shot 2020-01-22 at 1 54 28 PM

(Full diff)

@youknowriad
Copy link
Contributor

This is very nice to see. I guess we should have a page in Storybook showing all controls together (in order to reason about them together).

It makes me think we'd want:

  • Remove the padding from the control
  • Remove the separator by default
  • Change the border color of the input, maybe even the size (I think bigger is better but we need to be consistent, and it's better addressed for all inputs at the same time IMO)
  • Potentially offer a reusable LinkControlPopover component to add the padding and the separator back with CSS but tbh, I'd prefer avoiding it and relying on the fact that Dropdowns have padding by default.

@aduth
Copy link
Member

aduth commented Jan 30, 2020

I've been thinking about some specific tasks to get closer to a desirable separation between Popover, LinkControl, URLInput. The following set of tasks is a work-in-progress and should probably be separated out into discrete issues, but I want to at least get them written down somewhere rather than floating around in my brain:

Let me know if any of these don't make sense, or if there are others I am missing.

@youknowriad
Copy link
Contributor

This is a great list.

Unknown: Could URLPopover be repurposed to render the stylized LinkControl?

Sounds like it should, the name is not great though

(Maybe:) Move URLInput to @wordpress/components

LinkControl as well right?

Add "Forms" demo page to Storybook

😍

@aduth
Copy link
Member

aduth commented Feb 3, 2020

(Maybe:) Move URLInput to @wordpress/components

LinkControl as well right?

Possibly, yeah. I think it's generic enough. I also think there might be value in having something in block-editor which exists for the sole purpose of providing a "pre-wired-up" search suggestions behavior. A block should be able to just render a <LinkControl /> and not necessarily provide its behavior for fetchSearchLinkSuggestions. The only ways that I could see this being done is either to have some wrapper component in block-editor which reads from its editor settings (essentially what's done today to achieve this), or create a React context Consumer/Provider where something further up the React hierarchy defines the search behavior. The latter avoids the need for a wrapper component in block-editor, but I think is a weaker relationship in associating the search behavior.

@aduth
Copy link
Member

aduth commented Feb 3, 2020

Another observation from implementing this for the paragraph link format in #19462:

The presentation of a link preview including its title is nice:

LinkControl preview state

...but the way this is implemented in the Navigation Link block, it relies on the title attribute. This means that it does not ever show the title when inserting a link in a paragraph, even if the link was inserted using a post suggestion from a search term.

Furthermore, I have proposed for the title attribute to be removed from the Navigation block in #19990, which would break this behavior for this block as well. The primary concern in #19990 is largely around the use of the attribute as far as its implications in generating an <a title>, so it could technically be retained at least to preserve the nice UI for LinkControl. However, this seems like a problem that should not be addressed by the block, but rather supported by the LinkControl component itself. This would require some means for retrieving the title associated with the link, ideally purely based on its URL, though perhaps by its post ID if it is the only option.

I sense that the efforts described in #18042 could play a part here, though I'm not sure if that was specifically modeled for retrieving details associated with the current value of the link vs. details associated with a URL input value.

@aduth
Copy link
Member

aduth commented Feb 3, 2020

Another part of this that I've been thinking about is with regards to the extensibility we're exploring through the settings prop of this component. To me, I wonder what use-cases we're expecting to be supporting here. As I see it, there are problems with the current approach whether or not those use-cases exist.

  • If other use-cases do not exist, do we need to expose this as an API?
    • If we don't expose it as an API, can we be more explicit about how we handle "Opens in New Tab"? Also impacts whether or not this property should be included in the value of the component.
    • Similarly, if the use-case is "I want the option to hide 'Opens in New Tab'", can we express this more directly?
  • If other use-cases do exist, are we confident that ToggleControl (on/off state) is sufficient to cover those use-cases?
    • Do we need to support other types of controls (radio buttons, ranges, etc)? How would we define those? Is it part of the configuration of a setting (type: 'boolean'), where effectively we'd need to be able to construct some form from this declarative configuration syntax? Or should this be more like a render prop, where the developer would provide whatever controls they want in the render callback?
      • In a render callback, this has the advantage of separating the setting values from the value of the component. This is both good and bad, because it is more flexible, has greater distinction, but on the other hand, it makes "default" settings like "Opens in New Tab" more difficult to achieve if they must be provided explicitly.

@talldan
Copy link
Contributor

talldan commented Apr 21, 2020

What's remaining on this? Can it be closed?

@aduth
Copy link
Member

aduth commented Apr 21, 2020

There's still quite a few action items remaining here, though ideally should be broken down to disparate issues. I've listed some at #18061 (comment) . I can try to revisit this and create those issues to close in favor of.

@aduth
Copy link
Member

aduth commented May 18, 2020

I can try to revisit this and create those issues to close in favor of.

Closing in favor of:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Feature] UI Components Impacts or related to the UI component system [Package] Block editor /packages/block-editor
Projects
None yet
Development

No branches or pull requests