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

feat(Dropdown): add hideTitleText prop #7594

Merged
merged 9 commits into from
Jan 27, 2021

Conversation

molyholy1
Copy link
Contributor

Closes #

Addresses this issue #7571

Changelog

New

  • Add a hideTitleText prop for the Dropdown. The titleText will still exist programatically but hiding it visually lends itself to smoother design (especially with inline Dropdowns). This sort of prop also exists in the Select component.

Testing / Reviewing

  1. Start up the React Storybook and navigate to the Dropdown component
  2. In the code, pass in the hideTitleText prop
  3. (Optional but highly recommended) Run the IBM Equal Accessibility Checker with hideTitleText being true. There should not be an error for "The WAI-ARIA property must reference a non-empty unique id of an existing element that is visible" which normally occurs when the titleText is missing entirely. However, because we have hidden it visually but not programatically, the error shouldn't appear.

@molyholy1 molyholy1 requested a review from a team as a code owner January 19, 2021 17:03
@netlify
Copy link

netlify bot commented Jan 19, 2021

Deploy preview for carbon-elements ready!

Built with commit b7c027d

https://deploy-preview-7594--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Jan 19, 2021

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit b7c027d

https://deploy-preview-7594--carbon-components-react.netlify.app

Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

would you mind renaming the prop to hideLabel to match the other Carbon components? also it looks like you will need to update snapshots in order to pass CI

edit: yeah it looks like only the listbox components use "title" instead of "label" but this may need to be updated in the future to match the existing components like TextInput, Select, etc.

@emyarod emyarod requested review from a team and kingtraceyj and removed request for a team January 20, 2021 15:01
@molyholy1 molyholy1 requested a review from a team as a code owner January 20, 2021 16:42
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.

Not seeing any a11y errors with the hideLabel prop enabled. Thanks for contributing! LGTM 👍 ✅

Copy link
Member

@kingtraceyj kingtraceyj 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

@kodiakhq kodiakhq bot merged commit 0dc1626 into carbon-design-system:master Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants