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

Can't edit an existing link using only the keyboard #8266

Closed
afercia opened this issue Jul 28, 2018 · 13 comments · Fixed by #10983
Closed

Can't edit an existing link using only the keyboard #8266

afercia opened this issue Jul 28, 2018 · 13 comments · Fixed by #10983
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Milestone

Comments

@afercia
Copy link
Contributor

afercia commented Jul 28, 2018

In Gutenberg 2.9.0 it was possible to;

  • select an existing link in the content

screen shot 2018-07-28 at 18 08 03

  • press Tab and focus was moved on the link dialog: this way it was possible to edit a link using only the keyboard

screen shot 2018-07-28 at 18 10 12

This worked because the link toolbar was placed "inline" right after the block editable area. Pressing tab followed the normal tab order, moving focus to the following focusable element in the native tab sequence.

After the changes in #6911 this is no longer true. The link toolbar is now placed within a "popover" component and thus it's at the end of the markup, way far from the block content. The link toolbar is not the next following focusable element any longer.

Right now I can't find a way to edit an existing link using only the keyboard:

  • pressing Tab takes me elsewhere
  • pressing Cmd+K makes the link toolbar appear but it's in "insert mode" and doesn't allow me to modify the existing link

I'd like to remind everyone that so important changes should be marked with the Accessibility label and tested extensively also with keyboard only to make sure there are no regression.

I'd suggest to consider to replicate the Classic editor behavior. Pressing Cmd+K on an existing link should make the link toolbar appear (and get focused) in edit mode.

I've checked only the paragraph block, this should be checked for all the blocks that use the link toolbar.

Aside:
there's one more bit missing comparing the behavior with the Classic editor:

  • in Gutenberg there's the need to select the whole linked word(s)
  • in the Classic editor, placing the cursor within the linked word(s) is enough to edit the whole link
@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Regression Related to a regression in the latest release labels Jul 28, 2018
@abrightclearweb
Copy link

I noticed this today too.

Here is a gif of me creating a link and then trying to navigate back to it and edit it.
editing a link keyboard only

When I get the cursor within a link, I can press Shift+Tab to show the link popover, but then can't navigate into it. Pressing up, down, left or right just moves me right out.

The only way seems to be to use the Link button again to unlink the link, then recreate it. But that removes your link URL so not good.

Also, you can navigate to the "Open link in new tab" when you create a link but you can't toggle the option on until you have added a link already. So it is not much use if you can't go back and edit.

@gziolo
Copy link
Member

gziolo commented Oct 16, 2018

There is no corresponding PR for the issue, moving to 4.2.

@afercia
Copy link
Contributor Author

afercia commented Oct 16, 2018

Will 4.2 (or later) the one released in WP 5.0? Asking because this is a regression and I'd expect it to be fixed for release.

@gziolo
Copy link
Member

gziolo commented Oct 18, 2018

Will 4.2 (or later) the one released in WP 5.0? Asking because this is a regression and I'd expect it to be fixed for release.

Yes, 4.2 is going to be included in WP 5.0.

@talldan, as you are mastering all the logic related to the link UI, can you double check what is missing to have it working?

@talldan
Copy link
Contributor

talldan commented Oct 18, 2018

😄 Sure thing, I'll take a look.

@talldan
Copy link
Contributor

talldan commented Oct 18, 2018

Hi @afercia & @abrightclearweb

I did some testing and agree, this is far from easy for a keyboard user. I have a feeling the link popover visibility is tied to that of the toolbar. I had to use shift & left-arrow to select some link text. Then I could tab to the popover, but only after going through all the sidebar elements.

Just to make sure I work on the right thing, the desired behaviour would be

  1. The link popover is visible whenever the caret is positioned inside the link text.
  2. Pressing tab switches focus to the popover (first the link, second the edit button, and third the settings button).

Does that sound correct?

@afercia
Copy link
Contributor Author

afercia commented Oct 18, 2018

@talldan thanks for looking at this.
I'd say the Tab behavior shouldn't change, that wouldn't be expected at this point given the popover is placed far in the DOM.

I had to use shift & left-arrow to select some link text. Then I could tab to the popover, but only after going through all the sidebar elements.

I guess you've tested with just one paragraph in the post. When there are other blocks, tabbing brings you to the following block. At that point, the previous block with the link is no more "selected" and the link toolbar disappears. 🙂So we can't rely on tabbing.

I see something has changed in current master, specifically this point in the original report:

pressing Cmd+K makes the link toolbar appear but it's in "insert mode" and doesn't allow me to modify the existing link

Instead, now Cmd+K on an existing link opens the link toolbar in "edit mode". This solves great part of the problem, as the link is now editable.

Seems to me the only problem left compared to the previous behavior with the Classic Editor is that there's the need to select all the link text to make Cmd+K work and open the link toolbar. Instead, on Classic, having the caret positioned within the link is enough to make Cmd+K work.

Aside: since we're on it, can we change aria-label="URL" to aria-label="Link URL" ?

@talldan
Copy link
Contributor

talldan commented Oct 19, 2018

I guess you've tested with just one paragraph in the post. When there are other blocks, tabbing brings you to the following block. At that point, the previous block with the link is no more "selected" and the link toolbar disappears. 🙂So we can't rely on tabbing.

Good point 🤦‍♂️

Instead, on Classic, having the caret positioned within the link is enough to make Cmd+K work.

As suspected, this does seem to be tied to the visibility of the toolbar. The link popover is technically rendered within the toolbar (though within a portal, so it appears elsewhere in the DOM). That means it is only displayed when the toolbar is displayed. Here I hacked the toolbar so that it's always displayed and hey presto, the link popover is also displayed whenever the caret is within the link (and can be edited using the shortcut):

oct-19-2018 16-20-16

@talldan
Copy link
Contributor

talldan commented Oct 19, 2018

So I can think of two potential solutions

  1. Make sure the link popover isn't rendered as a child of the toolbar. This means the popover will be rendered, but not the toolbar. That looks like this:
    oct-19-2018 16-37-23

  2. Display the toolbar whenever the caret is within some formatting, thus causing the popover to also be rendered if the caret is within a link. (I don't have a gif for this one)

The second one would probably require a little bit more work. I think this has turned into a design question, so I'll add the Needs Design Feedback label.

@talldan talldan added the Needs Design Feedback Needs general design feedback. label Oct 19, 2018
@talldan
Copy link
Contributor

talldan commented Oct 19, 2018

If there's no answer next week, I'll go ahead an put together a PR of option 1.

Aside: since we're on it, can we change aria-label="URL" to aria-label="Link URL" ?

I'll make sure to include that 👍

@afercia
Copy link
Contributor Author

afercia commented Oct 19, 2018

Here I hacked the toolbar so that it's always displayed and hey presto, the link popover is also displayed whenever the caret is within the link

❤️

Option 1 seems reasonable to me and matches the classic editor experience. However, the link toolbar shouldn't move when moving the caret 😱 I seem to recall this was mentioned in a past issue as something to avoid and probably one of the reasons why the toolbar is now hidden?

Would it be possible to make the toolbar appear only on Cmd+K?

@karmatosed
Copy link
Member

Ideally I think 2 makes sense as you could want to interact with it. However, I am also ok if we just go with 1 to get a fix. I would agree, let's not have it move as that could be distracting.

@karmatosed karmatosed removed the Needs Design Feedback Needs general design feedback. label Oct 23, 2018
@talldan talldan self-assigned this Oct 24, 2018
@talldan
Copy link
Contributor

talldan commented Oct 24, 2018

I've put together a PR for the second option #10983, would be great to have some testing. I also have a branch locally for the first option.

I'm going to sort out a couple of things on separate PRs to make it easier to review and merge:

  • Changing the aria-label.
  • Making the sure the url popover stays centered (so that it doesn't follow the caret sideways.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants