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

Image block: linked image do not have a pressed button in the block toolbar #56078

Closed
afercia opened this issue Nov 13, 2023 · 4 comments · Fixed by #56123
Closed

Image block: linked image do not have a pressed button in the block toolbar #56078

afercia opened this issue Nov 13, 2023 · 4 comments · Fixed by #56123
Assignees
Labels
[Block] Image Affects the Image Block [Block] Media & Text Affects the Media & Text Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Package] Components /packages/components [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Nov 13, 2023

Description

When some normal text in the editor is linked, the 'Link' button in the block toolbar communicates there is a link both visually and semantically. Screenshot:

Screenshot 2023-11-13 at 10 30 20

Instead, when an image is linked, the 'Link' button doesn't communicate the 'linked' stage. Screenshot:

Screenshot 2023-11-13 at 10 30 43

the button misses the 'is-pressed' CSS class and the aria-pressed="true" attribute.

Besides that, labelling, functionality and UI to unlink/edit link are completely different for text and images. Ideally, the UIs should be similar but that's not the scope of this issue.

Step-by-step reproduction instructions

  • Edit a post.
  • Select some text and add a link. Save.
  • Select the selected text.
  • Observe the Link button in the block toolbar has a 'pressed' state and styling (and an aria-pressed="true" attribute)
  • Add an Image. Add a link to the image. Save.
  • Select the image.
  • Observe the Link button in the block toolbar dos not have a 'pressed' state and styling (and no aria-pressed="true" attribute)

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@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). [Package] Block library /packages/block-library [Block] Image Affects the Image Block labels Nov 13, 2023
@richtabor
Copy link
Member

Related to #50893. If image and media-text use LinkControl, this would be resolved (as well as further consolidate the linking experience throughout the editor).

@jordesign jordesign added [Type] Enhancement A suggestion for improvement. and removed [Type] Bug An existing feature does not function as intended labels Nov 13, 2023
@afercia afercia self-assigned this Nov 14, 2023
@afercia
Copy link
Contributor Author

afercia commented Nov 14, 2023

While working on this I noticed there's a CSS problem with the is-pressed state. A few of the buttons used in the block toolbar have a components-toolbar__control class. This CSS rule:

// Hide focus rectangle on click.
&:active::before {
display: none;
}

Introduces a problem and an inconsistent behavior. It hides the button ::before CSS pseudo element while the button is being clicked and thus:

  1. The is-pressed dark background isn't applied any longer while clicikng, which makes the button icon stay white on a white background so that the button looks 'empty'.
  2. The focus outline disappears while clicking, which is inconsistent with all the other buttons.

Animated GIF to illustrate the second point. Using the Embed block as an example, but this can be reproduced on other buttons that use the components-toolbar__control class:

  • The first button shows the focus outline while it is being clicked. This is the behavior of the vast majority of the buttons in the editor.
  • The second button doesn't.

focus

I'm going to remove this CSS rule. It was introduced in this commit as part of the G2 redesign and it appears to be an inconsistent remnant.

@afercia afercia added [Block] Media & Text Affects the Media & Text Block [Package] Components /packages/components [Package] Block editor /packages/block-editor and removed [Package] Block library /packages/block-library labels Nov 14, 2023
@afercia
Copy link
Contributor Author

afercia commented Nov 14, 2023

Related to #50893. If image and media-text use LinkControl, this would be resolved (as well as further consolidate the linking experience throughout the editor).

Thank @richtabor it would be freat to use the same UI. In the meantime I'll push a simpke fix for the current implementation.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Nov 14, 2023
@afercia afercia added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels Nov 14, 2023
@afercia
Copy link
Contributor Author

afercia commented Nov 14, 2023

jordesign added [Type] Enhancement and removed [Type] Bug labels 17 hours ago

In its current state, this implementation is not accessible and also the visual behavior is broken, so I'd argue this is actually a bug, not an enhancement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Block] Media & Text Affects the Media & Text Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Package] Components /packages/components [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants