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.

Sign up for GitHub

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

sul23-289 - aria atomic #98

Merged
merged 2 commits into from
Feb 9, 2024
Merged

sul23-289 - aria atomic #98

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a20278
sul23-289 - aria live
mdyoung3 Jan 11, 2024
1d29b96
Merge branch 'release-1.0.1' into SUL23-289-aria-label-results
pookmish Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/components/views/sul-study-place/study-place-filtering.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ const StudyPlacesFiltering = ({items}: {items: StudyPlace[]}) => {
</Conditional>

<Conditional showWhen={items.length > 0}>
<p aria-live="polite">
<p aria-live="polite" aria-atomic="true">
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there a requirement for this? To my understanding, this has no different affect since there are no child nodes within this element. There's only the 1 node that changes which is reflected to the screen reader on each change. If the html was structured like below, it would be appropriate:

<p...>
showing <strong>{length}</strong> of {total}
</p>

The way I read the documentation is that if length changed, that would be the only thing that would be read out to the screen reader. aria-atomic=true would read out the whole thing if anything inside it changed.

Also on this documentation it explains that with a ul/li list of users, when a new li element is added/removed screen readers only read that change, not the whole list. But then turning on atomic would read out the entire list each time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way I read the documentation is that if length changed, that would be the only thing that would be read out to the screen reader. aria-atomic=true would read out the whole thing if anything inside it changed.

This is how I interpret the documentation on aria-atomic. However:

TBD: Realistic use case for aria-atomic="true"

This doesn't bode well. Seems like there's not a good rules for this?

(The example page has multiple version, one example fits using an aria-atomic set to 'true'. https://pauljadam.com/demos/aria-atomic-relevant.html)

That is to say, I don't interpret this as a hard and fast rule, but a guideline. If you're using a screen reading, does it make more sense to has it say "x of 42" each time it updates? I suspect that would make more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing some light research in the WCAG docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not able to find a definitive answer to this. I ran axetools on both branches but the test didn't state having it on or off as an issue. To me, this confirms that it's not necessary but nice to have.

Not sure if it would be an issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since SODA did review this form and the aria attributes. I don't see a reason to push this up at this time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pookmish This came from ODA: https://stanfordits.atlassian.net/browse/SUL23-289?focusedCommentId=147663

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry. I should have sent this to you sooner.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. Ok, as annoying as it is that they didn't notice it the first several times, thanks for that!

Showing {itemsToDisplay.length} of {items.length}
<br/>
<Conditional showWhen={itemsToDisplay.length == 0}>
Expand All @@ -157,4 +157,4 @@ const StudyPlacesFiltering = ({items}: {items: StudyPlace[]}) => {
</div>
)
}
export default StudyPlacesFiltering;
export default StudyPlacesFiltering;
Loading