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

fix(toggle): make props.labelText optional #13196

Merged
merged 4 commits into from
Mar 3, 2023

Conversation

janhassel
Copy link
Member

Closes #12999

Changelog

New

  • Added "with accessible labels" story to demonstrate how to use the Toggle component

Changed

  • props.labelText is no longer required
    • The prop docs have been updated to inform the user about the appropriate ways of using this component

Testing / Reviewing

As per #12999 (comment)

  • <Toggle labelText="Label" />
    • The label is rendered on top
    • .cds--toggle__label is a <label> element with its for attribute set to the toggle's id
    • Side labels ("on" / "off") are rendered
  • <Toggle labelText="Label" hideLabel />
    • The label is rendered on the side instead of "on" / "off"
    • .cds--toggle__label is a <label> element with its for attribute set to the toggle's id
    • .cds--toggle__label-text is visually hidden but still in the a11y tree
  • <Toggle hideLabel aria-labelledby="external-id" />
    • The <button> receives the aria-labelledby attribute
    • .cds--toggle__label is a <div> element
  • <Toggle hideLabel />
    • .cds--toggle__label is a <div> element

@janhassel janhassel requested a review from a team as a code owner February 21, 2023 12:56
@netlify
Copy link

netlify bot commented Feb 21, 2023

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 6014ff9
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/64024f39b142e300082adff8
😎 Deploy Preview https://deploy-preview-13196--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@janhassel janhassel requested a review from a team as a code owner February 21, 2023 14:09
@janhassel janhassel changed the title fix(menu): make props.labelText optional fix(toggle): make props.labelText optional Feb 21, 2023
@tw15egan
Copy link
Collaborator

tw15egan commented Mar 2, 2023

@janhassel I noticed that we're getting this violation in the storybook
Screenshot 2023-03-02 at 10 57 45 AM

This violation is already present in the current storybook, so it's not a regression, but I wanted to know if there was any way to address this since it seems related to these changes.

I could resolve all but 1 of the violations in the example by changing the button on L101 to aria-labelledby={ariaLabelledby ? null : id}, but I was wondering about your thoughts on this issue. We can also roll this into a separate PR / Issue. The rest of the changes in this PR look great 👍🏻

@janhassel
Copy link
Member Author

@tw15egan I think we talked about this presumedly false positive of the a11y checker last year in this PR: #11972 (comment)

@tw15egan
Copy link
Collaborator

tw15egan commented Mar 2, 2023

@janhassel thanks for reminding me! 🙃

Copy link
Collaborator

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

LGTM 👍 ✅

@netlify
Copy link

netlify bot commented Mar 3, 2023

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 6014ff9
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/64024f39f64f18000885c01d
😎 Deploy Preview https://deploy-preview-13196--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@kodiakhq kodiakhq bot merged commit 5aff418 into carbon-design-system:main Mar 3, 2023
@janhassel janhassel deleted the 12999 branch March 6, 2023 10:47
kodiakhq bot added a commit that referenced this pull request Mar 14, 2023
* refactor(Dropdown): ariaLabel to aria-label

* fix(menu): fix arrow key navigation when onClose is called but not closed (#13195)

* fix(menu): only clear registered items when root is closing

* fix(menu): auto-cleanup registered items

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* feat(theme): add $icon-interactive token (#13219)

* feat(theme): add $icon-interactive token

* Update packages/themes/src/tokens/v11TokenGroup.js

Co-authored-by: Alison Joseph <[email protected]>

* Update packages/themes/src/tokens/__tests__/metadata-test.js

Co-authored-by: Alison Joseph <[email protected]>

* Update packages/themes/src/tokens/__tests__/__snapshots__/v11-test.js.snap

* Update packages/themes/src/tokens/__tests__/__snapshots__/v11-test.js.snap

* chore(snapshot): update snapshots

---------

Co-authored-by: Alison Joseph <[email protected]>
Co-authored-by: Francine Lucca <[email protected]>

* feat(TextInput): add typescript types (#13142)

* feat(TextInput): add typescript types

* fix(Text Input): submcompnent workaround + add helperID prop

* chore(format): format `Popover`, `Listbox` files (#13284)

* chore(format): format Popover file

* chore(format): format ListBox files

* chore(format): update format command

* chore(release): v11.24.0 (#13282)

Co-authored-by: tay1orjones <[email protected]>

* fix(toggle): make props.labelText optional (#13196)

* fix(toggle): make props.labelText optional

* docs(toggle): add 'with accessible labels' story

* test(toggle): update public api snapshot

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(ListBox): a11y - move filter tag as sibling of menu trigger, esc key improvements (#13173)

* fix(MultiSelect): a11y - move filter tag as sibling of menu trigger

* fix(Multiselect): remove tag from tab order

* fix(FilterableMultiselect): remove clear, tag from tab order

* fix(ComboBox): retain input value when menu is open and esc is pressed

* fix(FilterableMultiSelect): a few more UX tweaks

* style(MultiSelect): update styles to fix invalid issues when focused

* style(FluidMultiselect): adjust focus styles when invalid

* chore(log): remove console log

---------

Co-authored-by: Andrea N. Cardona <[email protected]>

* docs(style): add section on authoring inline styles (#13289)

* docs(style): add section on authoring inline styles

* docs(style): rephrase inline style docs

* docs(style): typo

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* refactor(ComboBox): ariaLabel to aria-label (#13273)

* refactor(ComboBox): ariaLabel to aria-label

* fix: formatting

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: Andrea N. Cardona <[email protected]>

* fix(tabs): remove max-width (#13247)

* fix(tabs): remove max-width

* docs(tabs): add max width guidance

* docs(tabs): format

---------

Co-authored-by: Andrea N. Cardona <[email protected]>
Co-authored-by: TJ Egan <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(ListBox): Standardize `ListBox` menu open behavior (#13268)

* style(MultiSelect): update styles to fix invalid issues when focused

* style(FluidMultiselect): adjust focus styles when invalid

* chore(log): remove console log

* fix(ListBox): align open behaviors across ListBox components

---------

Co-authored-by: Andrea N. Cardona <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* Add Typescript annotations to Select, SelectItem and SelectItemGroup (#13235)

* refactor(typescript): type annotations on Select*

Add Typescript annotations to Select, SelectItem and SelectItemGroup.

* fix(lint): removed unused imports

Left SelectItem and SelectItemGroup in the imports after using them for
testing. Removed.

* fix(Select-test): update label to labelText

Previously removed label as it was not a property of Select.
On recommendation of maintainers reintroduced using labelText.

* Apply suggestions from code review

Co-authored-by: Andrea N. Cardona <[email protected]>

* Follow up to change suggestions

---------

Co-authored-by: Andrea N. Cardona <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* refactor(FilterableMultiSelect): ariaLabel to aria-label (#13276)

Co-authored-by: Andrea N. Cardona <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: TJ Egan <[email protected]>

* fix(SkeletonText): prevent negative px width values (#13270)

Co-authored-by: Alessandra Davila <[email protected]>

* feat(Popover): add `isTabTip` prop to `Popover` (#13283)

* feat(tabTipPopover): add tabTip to Popover

* feat(tabTipPopover): add tab-tip styles, story

* test(Popover): add e2e tests

* style(Popover): adjust storybook styles

* fix(Popover): add new prop to proptypes

* chore(snapshot): update snapshots

* docs(Popover): add close on esc to story

* docs(Popover): fix typo

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* chore(deps): bump minimist in /packages/react/examples/react-router (#13254)

Bumps [minimist](https://github.com/minimistjs/minimist) from 1.2.5 to 1.2.8.
- [Release notes](https://github.com/minimistjs/minimist/releases)
- [Changelog](https://github.com/minimistjs/minimist/blob/main/CHANGELOG.md)
- [Commits](minimistjs/minimist@v1.2.5...v1.2.8)

---
updated-dependencies:
- dependency-name: minimist
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: TJ Egan <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* refactor(StructuredList): ariaLabel to aria-label (#13261)

* refactor(StructuredListWrapper): ariaLabel to aria-label

* fix(StructuredList test): use 'aria-label'

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* feat(DataTable): add rowIds to the sortRows extra params list (#13236)

Co-authored-by: TJ Egan <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* feat(typescript): add various typescript typings (#13234)

* feat(TableToolbar): add typescript typings

* feat(TableToolbarAction): add typescript typings

* feat(TableToolbarContent): change to `tsx`

* feat(TableToolbarMenu): add typescript typings

* feat(OverflowMenuItem): add typescript typings

* fix: format

---------

Co-authored-by: Francine Lucca <[email protected]>
Co-authored-by: Francine Lucca <[email protected]>

* fix(RadioTile): forward ref to input node (#13130)

Forward ref to input node of RadioTile,
such that it behaves the same as other form inputs.

Co-authored-by: Francine Lucca <[email protected]>

* feat: migration examples prefix selectors (#13264)

* feat(ContentSwitcher): preliminary unstable refactor

* feat: added prefix and id prefix examples

* fix: delete content switcher directory

* feat: update demos

* Update examples/id-prefix/src/App.jsx

Co-authored-by: Francine Lucca <[email protected]>

* Update packages/react/src/components/IdPrefix/index.js

* feat: added docs

* fix: update context

* fix: yarn install

* fix: update context 2

* chore: update yarn packages

* chore: udpate yarn carbon/react

---------

Co-authored-by: Francine Lucca <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* feat(DataTableSkeleton): 12513 - Add TS types for props (#13245)

* feat(DataTableSkeleton): 12513 - Add TS types for props

* fix: yarn run format

---------

Co-authored-by: Francine Lucca <[email protected]>
Co-authored-by: Francine Lucca <[email protected]>

* test(datatable): refactor to use RTL (#13294)

* chore: wip

* chore: wip

* test(datatable): refactor to use RTL

* chore: typo

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(modal): aria-label a11y bug (#13242)

Co-authored-by: Francine Lucca <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* test(HeaderSideNavItems): add tests (#13303)

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* docs(Storybook): update storybook config so actions display properly (#13302)

* docs(Storybook): update storybook config so actions display properly

* fix(DatePicker): change action names

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(tabs): deprecate light prop (#13262)

Co-authored-by: Andrea N. Cardona <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(menu): focus first item when opened (#13277)

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(CodeSnippet): a11y tabstop + attributes (#13310)

* fix(CodeSnippet): a11y tabstop + attributes

* fix(CSS): add Carbon CSS to focus outline

* refactor(CodeSnippet): relocate parens

* refactor(OverflowMenu): ariaLabel to aria-label (NEW) (#13280)

* fix(CodeSnippet): a11y tabstop + attributes

* fix(CSS): add Carbon CSS to focus outline

* refactor(OverflowMenu): ariaLabel to aria-label NEW

* Delete CodeSnippet.js

error from a rebase/merge

* Delete _code-snippet.scss

error from rebase/merge

* revert(missingFiles): they'll be ignored on merge

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* chore(test): remove enzyme (#13316)

* test(accordion): refactor from enzyme to RTL

* chore(test): remove enzyme

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* refactor(CodeSnippet): ariaLabel to aria-label (#13274)

Co-authored-by: Taylor Jones <[email protected]>

* feat(TextArea): add warning state (#13293)

* feat(TextArea): add warning state

* chore: update snapshots

* fix: textarea error bottom border and default value in story

* chore: hide resize fluid textarea invalid and warn

---------

Co-authored-by: Andrea N. Cardona <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* refactor: convert TextAreaSkeleton to TypeScript (#13275)

* refactor: convert TextAreaSkeleton to TypeScript

* chore: remove unneeded omit

* chore: remove unneeded omit

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(batchToolTips): make tooltips on batch visible (#13328)

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* refactor(Notification): ariaLabel to aria-label (#13281)

* refactor(Notification): ariaLabel to aria-label

* fix(Notification): rm stories default dupes

* fix(InlineNotification): rm deprecated prop

---------

Co-authored-by: Francine Lucca <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(format): tsx formatting

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Jan Hassel <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: TJ Egan <[email protected]>
Co-authored-by: Alison Joseph <[email protected]>
Co-authored-by: Francine Lucca <[email protected]>
Co-authored-by: Bianca Sparxs <[email protected]>
Co-authored-by: carbon-automation[bot] <103539138+carbon-automation[bot]@users.noreply.github.com>
Co-authored-by: tay1orjones <[email protected]>
Co-authored-by: Andrea N. Cardona <[email protected]>
Co-authored-by: jpsorensen <[email protected]>
Co-authored-by: andrew <[email protected]>
Co-authored-by: Alessandra Davila <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Chris Knabe <[email protected]>
Co-authored-by: GalvinGao <[email protected]>
Co-authored-by: Francine Lucca <[email protected]>
Co-authored-by: remolueoend <[email protected]>
Co-authored-by: Alison Joseph <[email protected]>
Co-authored-by: Sierra Wetmore <[email protected]>
Co-authored-by: Austin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants