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

a11y review and functional refactor #81

Merged
merged 3 commits into from
Feb 4, 2022
Merged

Conversation

blackfalcon
Copy link
Member

@blackfalcon blackfalcon commented Jan 29, 2022

Alaska Airlines Pull Request

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Resolves: #64, #55, #66

Summary:

Please summarize the scope of the changes you have submitted, what the intent of the work is and anything that describes the before/after state of the project.

The scope of this PR will address the manual assignment of an index attribute to manage keyboard events and pre-selected options.

This PR also addresses the remaining a11y interactions and voice-over feedback.

Demo: https://alaskaairlines-auro-menu-pr-81.surge.sh/

Type of change:

Please delete options that are not relevant.

  • New capability
  • Revision of an existing capability
  • Infrastructure change (automation, etc.)
  • Other (please elaborate)

Checklist:

  • My update follows the CONTRIBUTING guidelines of this project
  • I have performed a self-review of my own update

By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.

Thank you for your submission!

-- Auro Design System Team

@blackfalcon blackfalcon self-assigned this Jan 29, 2022
@blackfalcon blackfalcon linked an issue Jan 29, 2022 that may be closed by this pull request
Base automatically changed from dsande/datavalue/#65 to v3.1-rc January 31, 2022 23:45
@blackfalcon blackfalcon force-pushed the dsande/a11yReview branch 4 times, most recently from 7a6cb36 to 75661b2 Compare February 2, 2022 00:37
@blackfalcon blackfalcon changed the title a11y review a11y review and functional refactor Feb 2, 2022
This was linked to issues Feb 2, 2022
@blackfalcon blackfalcon force-pushed the dsande/a11yReview branch 8 times, most recently from a4d2efc to 2963df2 Compare February 3, 2022 07:05
@blackfalcon blackfalcon marked this pull request as ready for review February 3, 2022 07:08
@blackfalcon blackfalcon requested a review from a team as a code owner February 3, 2022 07:08
@blackfalcon blackfalcon linked an issue Feb 3, 2022 that may be closed by this pull request
This commit removes the manual complexity with keyboard experiences
and adds support for aria roles to increase voice over experiences.

Changes to be committed:
modified:   src/auro-menu.js
modified:   src/auro-menuoption.js
modified:   src/style-menuoption.scss
modified:   test/auro-menu.test.js
modified:   web-test-runner.config.mjs
Changes to be committed:
modified:   README.md
modified:   demo/demo.md
modified:   docs/api.md
Changes to be committed:
modified:   docs/api.md
modified:   src/auro-menuoption.js
@jason-capsule42
Copy link
Member

Sorry for adding this late, thought for sure I already put this in here.

After linking this to dropdownmenu locally, updating the eventListener to the new name and accounting for the change away from using index to make a selection I noticed the following regressions:

  1. The selectedOption event is fired when first loading the menu even when no value is pre-selected.
  2. Making a selection manually after load is not firing the selectedOption event
  3. When the selectedOption event is fired it doesn't it is not sending event.target.value or displayValue. Both are necessary for dropdownmenu since dropdownmenu needs the displayValue to put into the trigger.

@blackfalcon
Copy link
Member Author

@jason-capsule42 dropdown menu has a dependency on menu, not the other way around. The scope of this PR is to refactor the interaction functions of menu to meet an a11y spec that the previous version did not.

After this merge, dropdown menu (which is not released) will need to be refactored.

src/auro-menu.js Show resolved Hide resolved
@blackfalcon blackfalcon merged commit 3ff3006 into v3.1-rc Feb 4, 2022
@blackfalcon blackfalcon deleted the dsande/a11yReview branch February 4, 2022 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants