-
Notifications
You must be signed in to change notification settings - Fork 416
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
Combobox: ARIA attribute fixes #2004
Combobox: ARIA attribute fixes #2004
Conversation
Read-only combobox had two copies of ARIA attributes
Added due to autocomplete bug in browsers. Invalid value of "test" is passed into `autoComplete` intentionally.
- Remove duplicate attributes - align node IDs
ID was invalid since it was not rendered on the screen at the time
section tag should only be used for Popover. See SLDS markup
0523521
to
f297a18
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@interactivellama running npm run test:a11y
fails for me
@kevinparkerson Do the combobox a11y tests pass? |
You will need to do an |
…to aria-attributes-fix # Conflicts: # components/tooltip/__docs__/__snapshots__/storybook-stories.storyshot # package-lock.json # package.json
@kevinparkerson Updated |
autoComplete="test" | ||
id="id_from_input_prop" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@interactivellama why was this id removed? not needed for snapshot testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it was not needed.
@interactivellama still seeing this locally: |
When evaluating '.slds-combobox[aria-haspopup="dialog"' selector
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All combobox a11y tests now pass... code checks out. It's good to go!
Fixes #2003
Additional description
This allows aXe audit to pass for Combobox.
npm run test:a11y
@jcjuett @kevinparkerson
@aswinshenoy @pulkonet When you run
npm run test:a11y
on your new work (and you should! -- but it is not currently in the TravisCI tasks list for all pull requests today). This PR may help with fixing some of the errors.The issues were duplicate ARIA attributes used when only one was set was needed and that HTML IDs were being referenced when they were not on the page, since it's React convention to only add visible HTML nodes to the page such as when displaying menus, popovers, and tooltips.
CONTRIBUTOR checklist (do not remove)
Please complete for every pull request
npm run lint:fix
has been run and linting passes.components/component-docs.json
CI tests pass (npm test
).REVIEWER checklist (do not remove)
components/component-docs.json
tests.Required only if there are markup / UX changes
last-slds-markup-review
inpackage.json
and push.last-accessibility-review
, topackage.json
and push.npm run local-update
within locally cloned site repo to confirm the site will function correctly at the next release.