fix(core/form/inputs): fix issue with tabbing to popover toolbar buttons in PT-input #5057
+229
−169
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
skogsmaskin
changed the title
skogsmaskin
changed the title
|
No changes to documentation |
|
Component Testing Report Updated Dec 13, 2024 3:37 PM (UTC) ❌ Failed Tests (8) -- expand for details
|
skogsmaskin
force-pushed
the
fix/pt-input-tab-to-toolbar
branch
from
1c5d6a8 to
517727f
Compare
skogsmaskin
force-pushed
the
fix/pt-input-tab-to-toolbar
branch
from
517727f to
c8914ae
Compare
robinpyon
reviewed
Nov 2, 2023
There was a problem hiding this comment.
Thanks @skogsmaskin, this is a huge improvement.
Just had a look and noticed a few things:
- When pressing TAB for the first time over an annotation, it will try focus the next element briefly before then focusing buttons within the popover.
If you look at this video and pay attention to the 'No items' box, you'll see what I mean:
Screenflick.Movie.30.mp4
I'm very certain this is the case due to buttonRef.current.focus() being wrapped within a setTimeout. This does seem like a bit of a smell!
One possible alternative is that rather than focusing the button directly, we create an invisible anchor element (with tabIndex=-1), and then on TAB:
- initially focus the anchor element. Since it has
tabIndex=-1, it will be ignored in sequential keyboard navigation - the browser will then 'naturally' tab to the next button in the list
Here's an example video with that approach:
Screenflick.Movie.31.mp4
-
This popover doesn't trap focus – making it very easy to accidentally tab or shift-tab away completely out of the form. We should probably trap focus here.
-
Focus isn't restored when closing the popover – ideally, closing the popover with ESC returns you exactly where you were.
We could probably punt on 2 + 3, but should capture these if we do.
|
@skogsmaskin I'm assuming that this PR can be closed? 👀 |
skogsmaskin
force-pushed
the
fix/pt-input-tab-to-toolbar
branch
from
979a049 to
566e6b5
Compare
skogsmaskin
force-pushed
the
fix/pt-input-tab-to-toolbar
branch
from
566e6b5 to
d8c8257
Compare
skogsmaskin
force-pushed
the
fix/pt-input-tab-to-toolbar
branch
2 times, most recently
from
566e6b5 to
1adfba8
Compare
…able Text Input This will allow a user to purely use the keyboard when editing links and inline objects, which was not possible before.
… Portable Text Input
skogsmaskin
force-pushed
the
fix/pt-input-tab-to-toolbar
branch
from
1adfba8 to
3de982b
Compare
skogsmaskin
requested review from
a team and
juice49
and removed request for
a team
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
There is currently an issue with reaching the annotation and inline object edit toolbar buttons with the keyboard.
This will make sure that when these toolbars shows, it will be possible to press
Tabin order to get to the buttons with the keyboard.Also added component tests for this.
What to review
That you are able to tab into the buttons when the toolbar is showing.
Notes for release