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

Revise Link Control design and UX #47310

Closed
5 of 10 tasks
getdave opened this issue Jan 20, 2023 · 18 comments
Closed
5 of 10 tasks

Revise Link Control design and UX #47310

getdave opened this issue Jan 20, 2023 · 18 comments
Assignees
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) Needs Accessibility Feedback Need input from accessibility

Comments

@getdave
Copy link
Contributor

getdave commented Jan 20, 2023

Link Control

Let's update the Link Control design and UX to match the design provided by @jameskoster in #46891 (comment).

Quoting from @jameskoster

I’ve removed the “Create new page” flow because I think that’s better served elsewhere, but it’s easy to add back if there are strong feelings.

  • Apply / Cancel buttons provide better understanding of how to confirm or discard changes, and address some a11y issues.
  • Settings section provides a home for the options we want to add, and for third parties to extend.
  • Added a "Target" dropdown rather than having separate switches for "Open in a new tab" and "Nofollow", but separate switches could work as an interim.

How do we feel about this? Does the cog work for the settings toggle button, or should we use the Settings icon? In terms of size, I used 36px to match the Apply/Cancel buttons, but I think there's a movement towards 32px or 40px for these controls... what do you think @jasmussen?

PRs

Visual UX

a11y

Bugs introduced by the above changes

Please don't put general bugs in here. This is reserved for bugs introduce specifically by any of the changes above.

@getdave getdave added the [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) label Jan 20, 2023
@richtabor
Copy link
Member

richtabor commented Jan 20, 2023

Does the cog work for the settings toggle button, or should we use the Settings icon? (ref)

@jameskoster Does the ellipsis icon for "Link options" make sense here?

@getdave
Copy link
Contributor Author

getdave commented Jan 20, 2023

Pasting this response from the old reference point:

How do we feel about this? Does the cog work for the settings toggle button, or should we use the Settings icon? In terms of size, I used 36px to match the Apply/Cancel buttons, but I think there's a movement towards 32px or 40px for these controls... what do you think

(@jasmussen) Yes, these would become 32 (#46734) because they are on a row with a toggle. I think it can work!

@jameskoster
Copy link
Contributor

Iirc all instances of the ellipsis icon open popover menus. Are you suggesting to employ that pattern and putting the settings in a popover? Or to retain the behaviour in the design above and just change the icon?

The former might feel a bit awkward (popover within a popover) and the latter feels like a it breaks an existing design convention.

Another option could be the settings icon...

Screenshot 2023-01-20 at 13 34 38

Or even just a chevron? 🤔

Screenshot 2023-01-20 at 13 35 38

Though I think that looks a little strange without a text label.

@jasmussen
Copy link
Contributor

The settings icon looks good.

@getdave getdave added the Needs Accessibility Feedback Need input from accessibility label Jan 21, 2023
@getdave
Copy link
Contributor Author

getdave commented Jan 21, 2023

FYI I flagged this in the Accessibility channel on WP Slack (requires signup). Also added the appropriate label to ensure we're looping folks in early for feedback.

@getdave
Copy link
Contributor Author

getdave commented Jan 21, 2023

Also noting that I remember a while back there were some folks who were against adding lots of settings controls to this interface. That said I'm aware of several requests for more fine grained settings controls such as those offered by Yoast (which they currently add by re-implementing the Link Control in their Plugin).

It's worth noting that there is currently no simple way to allow users to customise/extend the settings controls because whilst it's trivial to pass props to <LinkControl> it is not simple to have a single uniform means of updating and persisting those settings in all the various places the component is used across the editor.

@joedolson
Copy link
Contributor

A few comments from an accessibility standpoint:

  1. Can we make the legend less verbose? The 'currently' and 'selected' are redundant: instead of "Currently selected link settings", maybe just "selected link settings" - or even just "link settings", since the context never supports editing multiple links in one link control panel.

  2. You mentioned this in Add settings "drawer" to Link Control #47328, but definitely would be good to fix the tab sequence. In general, a toggle should be in the DOM before the controlled content. This makes a natural tab sequence of "open panel", tab, "editing things in panel". Right now, the user has to shift tab, which is less intuitive. It would really be preferable if the design supported a control that was located before the panel it opens.

  3. At the moment, there's no way to select a link from a search & adjust its settings in a single step. Since selecting the link creates the link and closes the panel, you have to re-select the link and edit to change its settings. This is a minor annoyance for mouse users, but fairly significant if you're navigating by keyboard.

  4. When editing a link, there's no 'cancel' option unless you're currently editing; the preview state of a link isn't cancellable. If a keyboard user tabs into the link control panel, they aren't offered a clear way to exit.

  5. The link control panel settings don't support the text control options. (Options > Preferences > Show button text labels)

@richtabor
Copy link
Member

richtabor commented Jan 23, 2023

(Once we get this solid) is there any reason why we can't use this same component on the Image block's link control? The new design is much nicer, and it'd be good to have 1:1 consistency. 🤔

We'd need to follow-up with a way to add the link shortcuts (currently media and attachment page).
CleanShot 2023-01-23 at 11 19 19@2x
CleanShot 2023-01-23 at 11 19 39@2x

@jameskoster
Copy link
Contributor

jameskoster commented Jan 23, 2023

use this same component on the Image block's link control?

Good idea.

We'd need to follow-up with a way to add the link shortcuts (currently media and attachment page).

I suspect linking to the media file is a very common use case so perhaps we'd need the shortcuts to begin with? A dropdown might work:

Screenshot 2023-01-23 at 09 54 34

Alternatively, we could avoid a dropdown altogether by simply pre-populating the destination with the associated token?

Screenshot 2023-01-23 at 10 05 53

@jasmussen
Copy link
Contributor

The token looks good. Is the radius 2px? Looks 3px. Maybe needs to be an inset border

@getdave
Copy link
Contributor Author

getdave commented Jan 26, 2023

@joedolson Thank you for taking the time to feedback here. I'll spin out Issues from your comment.

Update: I created Issues and included ones that are specifically related to the re-design in the Issue description above.

@ajaydsouza
Copy link

ajaydsouza commented Mar 2, 2023

Is there a way to keep the settings drawer open by default? Is it this issue by any chance?

Otherwise if you want to link to a new tab, you need to click quite a few times to get that done.

I currently have 15.2.4 installed in my test site.

@getdave
Copy link
Contributor Author

getdave commented Mar 2, 2023

Is there a way to keep the settings drawer open by default? Is it #47821 by any chance?

Yes we're working on that one.

@getdave
Copy link
Contributor Author

getdave commented Mar 9, 2023

Related #35073

@SaxonF
Copy link
Contributor

SaxonF commented Mar 13, 2023

@getdave are we able to link this new issue here? It suggests a few changes including always showing open in new tab setting which might conflict with this issue.

I'm struggling a little bit keeping up with all the navigation issues and it seems like a couple of them overlap. Let me know if I can help break the above one down into smaller tasks.

@getdave
Copy link
Contributor Author

getdave commented Mar 13, 2023

Thanks you for picking up and wrangling some of this. I'm intending to get onto this myself as my next priority (after some other pressing issues that are time sensitive).

@getdave are we able to link this [new issue](#48612 here? It suggests a few changes including always showing open in new tab setting which might conflict with this issue.

I'm concerned that Link Control needs to be considered in a holistic sense outside of just creation of links in the Nav block. In fact much of what we are landed with now is partly due to an over-focus on servicing the Nav block.

The aim of #47310 was to be a canonical place where we could do that. Then once good decisions have been made to service the entirety of the editor we can adjust them to better work for the Nav block.

Ideally we'd use #47310 as a canonical source of truth. We can adjust it as necessary. I'm very keen we don't loose sight of the the reason for the creation of the "settings drawer" which is to allow for the much requested addition of ancillary controls for links.

@getdave
Copy link
Contributor Author

getdave commented Mar 15, 2023

I've created this Issue which tracks the next iteration of the Link UI. I'll leave this Issue open for now but the overview can be found there.

@getdave
Copy link
Contributor Author

getdave commented May 25, 2023

Closing in favour of #50891

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) Needs Accessibility Feedback Need input from accessibility
Projects
None yet
Development

No branches or pull requests

7 participants