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

Adds aria-expanded for See more options. #3187

Merged
merged 1 commit into from
Jul 6, 2023
Merged

Conversation

justinlittman
Copy link
Contributor

refs #3170

Why was this change made? 🤔

How was this change tested? 🤨

⚡ ⚠ If this change involves consuming from or writing to another service (or shared file system), run integration test create_object_h2_spec.rb and/or test manually in [stage|qa] environment, in addition to specs. ⚡

Does your change introduce accessibility violations? 🩺

⚡ ⚠ Please ensure this change does not introduce accessibility violations (at the WCAG A or AA conformance levels); if it does, include a rationale. See the Infrastructure accessibility guide for more detail. ⚡

@justinlittman justinlittman marked this pull request as ready for review July 6, 2023 11:19
@@ -39,7 +39,7 @@
</div>

<% unless mixed_material_type? %>
<a href="#" class="more-options collapsed" data-action="more-types#toggleMoreTypes" data-more-types-target="moreTypesLink">
<a href="#" class="more-options collapsed" data-action="more-types#toggleMoreTypes" data-more-types-target="moreTypesLink" aria-expanded="false" role="button">
Copy link
Contributor

@edsu edsu Jul 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need a aria-controls attribute when using aria-expanded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@@ -65,7 +65,7 @@

<div class="mb-4 subtype-container" data-work-type-modal-target="subtype"></div>

<a href="#" class="more-options collapsed" data-action="work-type-modal#toggleMoreTypes" data-work-type-modal-target="moreTypesLink" hidden>
<a href="#" class="more-options collapsed" data-action="work-type-modal#toggleMoreTypes" data-work-type-modal-target="moreTypesLink" hidden aria-expanded="false" role="button">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question here re: aria-controls

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@justinlittman justinlittman force-pushed the see_more_accessibility branch from 1487999 to d86f408 Compare July 6, 2023 15:45
@justinlittman justinlittman requested a review from edsu July 6, 2023 15:45
this.continueButtonTarget.focus()
}

hideMoreTypes() {
this.moreTypesTarget.hidden = true
this.moreTypesLinkTarget.innerHTML = 'See more options'
this.moreTypesLinkTarget.classList.toggle('collapsed', true)
this.moreTypesLinkTarget.setAttribute('aria-expanded', false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker, but do we need aria-controls here too?

@ndushay ndushay added the accessibility accessibility improvements label Jul 6, 2023
@justinlittman justinlittman merged commit 1460f44 into main Jul 6, 2023
@justinlittman justinlittman deleted the see_more_accessibility branch July 6, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility accessibility improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants