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

Snackbar: Make the explicitDismiss string translatable #60368

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Apr 2, 2024

Submitting this PR without an associated issue because this is a trivial change that doesn't need a broader discussion.

What?

When snackbars have the explicitDismiss prop set to true, they show an X close button. The button aria-label is not translatable, it's a hardcoded string.

Why?

All user facing text, including non-visible ARIA properties, should be translatable.

How?

Makes the string translatable.

Testing Instructions

N/A

Testing Instructions for Keyboard

Screenshots or screencast

@afercia afercia requested a review from ajitbohra as a code owner April 2, 2024 12:41
Copy link

github-actions bot commented Apr 2, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: afercia <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: richtabor <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@afercia afercia added [Type] Bug An existing feature does not function as intended l10n Localization and translations best practices labels Apr 2, 2024
@afercia
Copy link
Contributor Author

afercia commented Apr 2, 2024

Introduced in #26952

@afercia afercia requested a review from mkaz April 2, 2024 12:44
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching the issue, @afercia!

@richtabor
Copy link
Member

Noting the copy should likely be "Dismiss notice".

Short, direct commands tend to be easier for users to scan and understand quickly. This phrasing still conveys the essential action without unnecessary words, maintaining clarity and effectiveness.

@afercia
Copy link
Contributor Author

afercia commented Apr 2, 2024

Thanks @Mamaduka
It seems the PHPunit tests fail very early, it happens on other PRs too for example #60364 so that it seems unrelated. Error message:
docker-compose: command not found lol

Any clue whether it's a known issue?

@Mamaduka
Copy link
Member

Mamaduka commented Apr 2, 2024

Sorry, I have no clue regarding the PHP test failure.

@afercia
Copy link
Contributor Author

afercia commented Apr 2, 2024

Noting the copy should likely be "Dismiss notice".

Unrelated to this PR, anyways a good opportunity to clarify a couple things:

  • This kind of common copy / labels etc. should be consistent throughout all the WordPress admin. In core, this copy has been Dismiss this notice since years. I'm not opposed to change it but that should be coordinated across the whole WordPress codebase which is made of Core + Gutenberg. Consistency is key.
  • In this specific case, the aria-label is used for an X close button. The button only shows an X graphics. Yes, the aria-label should be meaningful but it should also be understandable based on the graphics in use. For example, speech recognition software users do see an X. They need to understand what the accessible name of this button is, to be able to issue a voice command. An X is commonly used in association with a label that just says 'Close'. I doubt speech recognition software users will ever be able to guess the actual accessible name is 'Dismiss this notice'.

It's important to understand the technologies and devices we have to design for. Anyways, the Snackbar notices would need some important work to be fully accessible that has been discussed at length when they were introduced but no agreement was reached. In my personal opinion, while snaclbars may be a good pattern for small screen devices, they are less than ideal for large screens at the point I would like to see them entirely removed from WordPress.

That said, this PR only aims to fix a non-translatable string. Broader discussions should go in a separate issue.

@afercia afercia force-pushed the fix/snackbar-explicit-dismiss-non-translatable branch from 52bce86 to cf64527 Compare April 3, 2024 08:33
@afercia afercia merged commit 9d0dc01 into trunk Apr 3, 2024
55 of 56 checks passed
@afercia afercia deleted the fix/snackbar-explicit-dismiss-non-translatable branch April 3, 2024 09:46
@github-actions github-actions bot added this to the Gutenberg 18.2 milestone Apr 3, 2024
cbravobernal pushed a commit to garridinsi/gutenberg that referenced this pull request Apr 9, 2024
Co-authored-by: afercia <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: richtabor <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
l10n Localization and translations best practices [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants