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

Fixes usage of aria-owns/controls in trigger #633

Merged
merged 1 commit into from
Nov 30, 2021

Conversation

matthew-robertson
Copy link
Contributor

Closes #557.

This mostly just fixes the merge conflicts present in #558.
To re-iterate, this commit uses aria-controls instead of aria-owns to build the relationship between the trigger and its content. In addition, the aria-owns property is migrated to the trigger's parent.

This fixes an issue I ran into using Apple's VoiceOver software, in which I couldn't use the movement commands (VO+right-arrow) to navigate into dropdown menus created with ember-basic-dropdown.

@cibernox
Copy link
Owner

Is this for standalone use of the dropdown or for it's use within ember-power-select? If it's for the latter, I just released today 5.0.0 of ember-power-select with improved a11y, which may fix your problems.

@matthew-robertson
Copy link
Contributor Author

This is intended for standalone use of the dropdown. Those recent changes to ember-power-select were written by a coworker of mine, I believe, but this is intended for places where basic-dropdown is used directly.

In our case, I could forcibly set aria-owns="" in all of our uses of the component and it seems like that would get things working. It seemed cleaner to help get the change in upstream and close that previously existing PR though.

@matthew-robertson
Copy link
Contributor Author

Sidenote: what's up with the CI failures? Seems like it's having trouble loading the tests at all?

Global error: Uncaught Error: Could not find module `qunit` imported from `(require)` at http://localhost:7357/assets/vendor.js, line 256

The suite seemed to run without error locally, but if I've messed something up that's breaking CI, I'll figure it out.

@cibernox
Copy link
Owner

I'm woring on a fix on #634

@cibernox
Copy link
Owner

CI should be better now

Closes cibernox#557.

This mostly just fixes the merge conflicts present in cibernox#558.
To re-iterate, this commit uses `aria-controls` instead of `aria-owns`
to build the relationship between the trigger and its content.

In addition, the `aria-owns` property is migrated to the trigger's parent.
This fixes an issue I ran into using Apple's VoiceOver software, in
which I couldn't use the movement commands (VO+right-arrow) to navigate into dropdown
menus created with ember-basic-dropdown.
@matthew-robertson
Copy link
Contributor Author

Seems like the CI's passing, and I've fixed the merge conflicts I encountered. Lemme know if there's anything else I can fix up with it.

@cibernox
Copy link
Owner

I missed your last push. CI is running, I'll merge when it's done.

@cibernox cibernox merged commit 7415eb2 into cibernox:master Nov 30, 2021
@cibernox
Copy link
Owner

Merged! Thanks! I’ll release a new version in a couple days. Ping me if you see I don’t by Friday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aria-owns should not be used on the trigger, but on its parent
2 participants