-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix the image link button pressed state. #56123
Conversation
Size Change: -10 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
0cfabb6
to
621a99b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@afercia Works as described. Might need a visual review though. Change looks sane but I can't see it.
@alexstine thanks for the review. I'd still like to have a review about the I see occurrences of a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tests well, and code-wise it looks good too, thanks 👍
Thanks everyone for your feedback and reviews. |
Fixes #56078
What?
In the Image block and in the Media & Text block, when the image is linked, the Libk bugton in the block toolbar doesn't have a pressed state. As such, there's no visual / semantic / accessible information the image is actually linked.
Why?
How?
isActive
prop to the relatedToolbarButton
.More details on the CSS rule:
I have the impression the whole content of the toolbar-button stylesheet is a remnant of some previous implementation and it's no lonver in use in the editor. See
gutenberg/packages/components/src/toolbar/toolbar-button/style.scss
Lines 1 to 28 in 5bcb309
All of the rules there are scoped to some kind of
data-subscript
attribute that I think is not used any longer. To my understanding it was meant to display some subscript CSS generated text within the buttons, for headings or maybe the quote block.Only the following rule wasn't scoped to a
subscript
thing:This rule hid the pseudo element while the button is being clicked. This is wrong for two reasons, see comments on the here #56078 (comment).
is-pressed
background styling to work while the button is being clicked.@jasmussen @richtabor when you have a chance, I'd kindly ask you to have a look at the content of this stylesheet. There's a chance it can be removed entirely.
Testing Instructions
is-pressed
CSS class.aria-pressed="true"
attribute.Additionally:
Testing Instructions for Keyboard
Screenshots or screencast