search

AlaskaAirlines / auro-menu

Custom element that provides a list of options for the user to choose from
https://auro.alaskaair.com/components/auro/menu
Apache License 2.0
2 stars 3 forks source link

a11y review and functional refactor #81

Closed blackfalcon closed 2 years ago

blackfalcon commented 2 years ago

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.

Checklist:

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

jason-capsule42 commented 2 years ago

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 commented 2 years ago

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