Fix mismatching link control action buttons visual order and DOM order #56042
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
afercia
added
[Type] Bug
|
Size Change: -34 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
draganescu
approved these changes
Nov 13, 2023
There was a problem hiding this comment.
Tested this and the code change makes sense, thank you!
As an unrelated separate note, while testing it felt surprising the tabbing was not constrained in the popover (at least in Firefox). I am just talking about perception in my case not some rule I expected 😄
|
@draganescu thanks for reviewing.
Yeah, this is something that has always been a bit controversial. I think there was some reasoning about not constraining tabbing within these 'inline' popovers. However, many things have changed since then. RIght now, the situation is very mixed and largely inconsistent. Most popovers in the UI outside of the post content do implement constrained tabbing. Some popovers that can be accessed from the post content (e.g. the link edit) do not. Now that these popovers are rendered outside of the post content iframe I think they should all implement constrained tabbing. In fact, tabbing away from these popovers brings users very farm from they originally were (inside the post content). |
cbravobernal
pushed a commit
that referenced
this pull request
Nov 14, 2023
#56042) * Fix visual and dom order by using HStack. * Fix padding of tertiary buttons within the visual editor. * Fix top padding in media replace popover.
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.
Fixes #56039
What?
First pass at fixing the mismatching visual and DOM order of the 'Cancel' and 'Save' buttons in the link control UI.
Why?
For accessibility, visual order and DOM order must always match.
It is important to make all designers and developers aware of this. Pinging all the folks who worked on #46933
Cc @getdave @draganescu @jameskoster @richtabor @scruffian @jasmussen @joedolson @SaxonF
How?
Uses the
HStackcomponent instead of custom CSS rules. This makes these Cancel/Save buttons consistent with all similar button paris in use in the edifor, for example in the:Testing Instructions
Additionally:
Testing Instructions for Keyboard
See above.
Screenshots or screencast
Before:
After: