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

Remove clip & -webkit-clip-path for downloadable-block-list-item style.scss #66147

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

mediaformat
Copy link
Contributor

@mediaformat mediaformat commented Oct 15, 2024

What?

Addresses part of #65954

Why?

The utility .screen-reader-text CSS is used in several places throughout the Gutenberg code base. In #65409 it came up that we should update all the areas to match the changing CSS rules proposed in #65409.

How?

Remove the unnecessary clip: rect(1px, 1px, 1px, 1px); and -webkit-clip-path: inset(50%); rules from the packages/block-library/src/components/downloadable-block-list-item/style.scss

Style attribute

Before After
Screenshot 2024-09-17 at 17 48 21 Screenshot 2024-09-17 at 17 48 57

Copy link

github-actions bot commented Oct 15, 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: mediaformat <mediaformat@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>
Co-authored-by: colorful-tones <colorful-tones@git.wordpress.org>

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

Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @mediaformat! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 15, 2024
@mediaformat mediaformat changed the title Remove clip & -webkit-clip-path for screen-reader-text CSS Remove clip & -webkit-clip-path for downloadable-block-list-item style.scss Oct 15, 2024
@colorful-tones colorful-tones added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Code Quality Issues or PRs that relate to code quality labels Oct 15, 2024
@afercia
Copy link
Contributor

afercia commented Oct 16, 2024

Thanks @mediaformat
I think this rulest can be entirely replaced with a display: none. Currently, this rule is only set when a block from the blocks directory in the main inserter is in the process of being installed.
It is meant to temporarily hide the block author name when isInstalling is true. After the block is installed, the author name is visible again. Anyways, the whole content of the button in the inserter is overridde by the aria-label set on the button so there's really no need to use the screen-reader-text rule on the author name.

  • Open the main inserter.
  • Search for some term e.g. image
  • Scroll down the inserter and observe the Available to install section. Note: this is the blocks directory. It takes a while to load.
  • Choose one of the blocks from the blocks directory and click to install it.
  • Observe the content of the block item in the list changes and shows an Installing... text.
  • Observe the original content of the block item is visible again when the block is installed.
Default Installing
Screenshot 2024-10-16 at 09 19 43 Screenshot 2024-10-16 at 09 22 18

Copy link
Contributor

@afercia afercia left a comment

Choose a reason for hiding this comment

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

I'd suggest to replace the entire rule with a display: none.

@mediaformat
Copy link
Contributor Author

@afercia I see what you mean. I'll replace the entire rule with display: none; as suggested.

@colorful-tones
Copy link
Member

I'd suggest to replace the entire rule with a display: none.

Great catch and thanks @afercia !
Thanks for addressing with an update @mediaformat 👏

I tested with display: none and works as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants