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

Components: Switch screen-reader-text to VisuallyHidden #18165

Merged
merged 3 commits into from Nov 4, 2019

Conversation

mkaz
Copy link
Member

@mkaz mkaz commented Oct 29, 2019

Description

This is the follow up PR from comment in #18127 it addresses changing the other spots in components that use screen-reader-text to using VisuallyHidden component.

The two spots changed are: base-control and form-text-token

How has this been tested?

I created a story in Storybook to be committed in separate PR and confirmed that using the component shows the label as intended,

Types of changes

In essence it changes <span className="screen-reader-text"> ... </span> to
<VisuallyHidden> ... </VisuallyHidden>

The base-control change uses label, and had a bit more logic wrapped around it.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@mkaz mkaz changed the title Switch screen-reader-text to VisuallyHidden Components: Switch screen-reader-text to VisuallyHidden Oct 29, 2019
@mkaz mkaz added the [Package] Components /packages/components label Oct 29, 2019
Copy link
Contributor

@brentswisher brentswisher left a comment

Choose a reason for hiding this comment

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

I tested and this seems to work great on the FontSizePicker in #18149. I left two notes but nothing I think is a big problem.

@gziolo gziolo added the [Type] Code Quality Gotta shave those yaks. label Oct 30, 2019
Copy link
Contributor

@brentswisher brentswisher left a comment

Choose a reason for hiding this comment

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

Looks good to me

@mkaz mkaz force-pushed the fix/18127-screen-reader-text branch from 410e6a8 to dc14595 Compare October 31, 2019 02:36
@gziolo
Copy link
Member

gziolo commented Oct 31, 2019

Travis raises some concerns in e2e tests related to taxonomies. I restarted them but they might be valid looking at the scope of the changes. It might be also related to some unrelated changes in WordPress core.

@mkaz
Copy link
Member Author

mkaz commented Oct 31, 2019

@gziolo yeh, I can't see why those tests are failing. I tried rebasing to see if that was part of the issue but didn't help.

@mkaz mkaz requested review from nerrad and ntwb as code owners November 1, 2019 16:13
@mkaz
Copy link
Member Author

mkaz commented Nov 1, 2019

@gziolo - it was a valid issue with the test. It had a check for .screen-reader-text which was changed in the form-input-text. I've corrected it and tests are now green 👍

I've gotten to used to restarting tests to get them to pass. 😆

gziolo
gziolo approved these changes Nov 4, 2019
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Cool, I'm glad it was easy to sort out. Everything looks good, thanks for working on it 🚢

@mkaz mkaz merged commit 3d2b41a into master Nov 4, 2019
@mkaz mkaz deleted the fix/18127-screen-reader-text branch November 4, 2019 13:02
daniloercoli added a commit that referenced this pull request Nov 5, 2019
…rnmobile/gb-mobile-872-JSApplicationIllegalArgumentException-in-RCTAztecView

* 'master' of https://github.com/WordPress/gutenberg: (56 commits)
  Update: Default gradients. (#18214)
  Fix: setting a preset color on pullquote default style makes the block invalid (#18194)
  Fix default featured image size (#15844)
  Fix postmeta radio regression. (#18183)
  Components: Switch screen-reader-text to VisuallyHidden (#18165)
  [rnmobile] Release 1.16.0 to master (#18261)
  Template Loader: Add theme block template resolution. (#18247)
  Add a README file for storybook directory (#18245)
  Add editor-gradient-presets to get_theme_support (#17841)
  Add "Image Title Attribute" as an editable attribute on the image block (#11070)
  enables horizontal movers in social blocks (#18234)
  [RNMobile] Add mobile Spacer component (#17896)
  Add experimental `ResponsiveBlockControl` component (#16790)
  Fix mover for floats. (#18230)
  Rename Component to WPComponent in docstring (#18226)
  Colors Selector: replace `Aa` text by SVG icon (#18222)
  Removed gif from README (#18200)
  makes the submenu items stacked vertically (#18221)
  Add block navigator to sidebar panel for nav block (#18202)
  Fix: consecutive updates may trigger a blocks reset (#18219)
  ...
@youknowriad youknowriad added this to the Gutenberg 6.9 milestone Nov 11, 2019
CreativeDive pushed a commit to CreativeDive/gutenberg that referenced this pull request Nov 12, 2019
)

* Switch screen-reader-text to VisuallyHidden

* Update packages/components/src/form-token-field/token.js

Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]>

* Fix test check for visually hidden text
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Code Quality Gotta shave those yaks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants