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

Option to remove form aria attributes #1

Merged
merged 2 commits into from
Mar 15, 2021
Merged

Option to remove form aria attributes #1

merged 2 commits into from
Mar 15, 2021

Conversation

Skovvart
Copy link
Contributor

metonym/svelte-typeahead#19 did not solve the Lighthouse issues, but removing the role and aria-labelledby from the form does (Typeahead's [data-svelte-typeahead] wrapper should hopefully retain all the accessibility properties).
If the PR is accepted, Typeahead will need to be updated use this non-default option.

metonym/svelte-typeahead#19 did not solve the Lighthouse issues, but removing the `role` and `aria-labelledby` from the form does (Typeahead's `[data-svelte-typeahead]` wrapper should hopefully retain all the accessibility properties). 
If the PR is accepted, Typeahead will need to be updated use this non-default option.
@Skovvart
Copy link
Contributor Author

One other thing I forgot to mention - Typeahead will need an additional change besides using this option, which is always including the target list-item ul (but hidden and empty).

@metonym
Copy link
Owner

metonym commented Mar 15, 2021

One other thing I forgot to mention - Typeahead will need an additional change besides using this option, which is always including the target list-item ul (but hidden and empty).

Could you elaborate on this?

Do you mean the ul should still be rendered but be visually hidden?

@Skovvart
Copy link
Contributor Author

Do you mean the ul should still be rendered but visually hidden?

Yes, the DOM element should exist for the aria-owns attribute, but be visually hidden (as long as there are no li items, that may be sufficient?)

@metonym
Copy link
Owner

metonym commented Mar 15, 2021

Makes sense, let's give that a try

@metonym metonym merged commit d3901ca into metonym:master Mar 15, 2021
@metonym
Copy link
Owner

metonym commented Mar 15, 2021

Released in v1.1.0

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.

None yet

2 participants