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

Missing semantic differentiation between status and heading #119

Merged
merged 5 commits into from
Jul 24, 2022
Merged

Missing semantic differentiation between status and heading #119

merged 5 commits into from
Jul 24, 2022

Conversation

swashbuck
Copy link
Contributor

@swashbuck swashbuck commented Jul 5, 2022

fixes #118

  • Change plus icon to checkmark once the item has been visited.
  • Use visited instead of complete/incomplete

@swashbuck swashbuck self-assigned this Jul 5, 2022
@swashbuck swashbuck changed the title Issue/118 Missing semantic differentiation between status and heading Jul 5, 2022
@oliverfoster
Copy link
Member

Could I have a rationale for the parentheses please? How does it change the way the screen reader reads?

@swashbuck
Copy link
Contributor Author

Could I have a rationale for the parentheses please? How does it change the way the screen reader reads?

The company that did our external accessibility testing recommended using parentheses to ensure that there's a pause in the screenreader's reading. Some screenreaders (JAWS maybe?) will sort of skip over commas or semicolons. Periods may be sufficient, though, and we could do some further testing if that's preferred?

@joe-replin
Copy link

Could I have a rationale for the parentheses please? How does it change the way the screen reader reads?

Hello, we've tried a few configurations including a semi-colon. Here's the quote from the testing that came back to us.

The addition of the semi-colon to the table of contents heading has inconsistent output with different screen readers. JAWS announces the status as "complete semicolon, [Course Title]", while NVDA announces the semicolon with a slight pause that does not make the semantic distinction as clear.
In addition, the text complete by itself at the beginning of the heading can still make the purpose of the heading misleading without additional context. Consider moving status text to the end of the heading with context as to what the status represents (ex"[Course Title], course completed")

Additionally, the 'status parenthesis' was approved and committed to headings.hbs in core. We were looking to normalize this structure across the individual component items as well.

@oliverfoster
Copy link
Member

oliverfoster commented Jul 6, 2022

I mean. I strongly disagree with the whole notion of moving the completion status to the end of the sentence as it dramatically slows down completion checking in a course.

If, for example, I'm navigation with the "H" key, to iterate back and forth through the headers, I now have to wait for each header to read before I can determine the section's completion status, hugely elongating the time required to determine what I have / haven't completed, it's also inverse to the visual appearance of the status bar and title in the course.

It is worth noting that the recommendation to move it to the end isn't supported by the wcag standard, as completion reporting isn't featured in the wcag standard, the recommendation is therefore a personal preference argument. It is worth noting that any unusual UI component will be confusing to a user, but the placement at the start was intended as a tool for quick navigation. There has so far been no competing use-related argument for changing it. I don't understand how it is less confusing at the end of the title than at the beginning but I do understand how it is less useful being there.

Adding an extra pause with parenthesis is mind blowing to me, is that meant to further reduce confusion?

Standardising it is a good idea. If adding parenthesis gives an intended pause, then that's fine, I'm just curious.

@swashbuck
Copy link
Contributor Author

I know that it was recommended to use parentheses, but it seems that periods pause more consistently across screenreaders. JAWS actually reads out the parentheses, which seems odd since we're just trying to add a pause. What is everybody's opinion on this?

https://www.deque.com/blog/dont-screen-readers-read-whats-screen-part-1-punctuation-typographic-symbols/

parenthesis
period

@oliverfoster
Copy link
Member

oliverfoster commented Jul 6, 2022

You'd need to test a double period. The content may end with a period, if you're adding another in the template then it'll have two before the status.

Generally speaking, trying to change the intonation of a screen reader is a fools game. As you've already said, they all read differently and some read differently in different browsers and some will read differently in different languages.

Switch aria-label to visited status instead of complete/incomplete.
Move visited status after item title and wrap in parentheses when it exists.

Then, add period after status.
@swashbuck
Copy link
Contributor Author

I've reverted the following changes:

  • Switch aria-label to visited status instead of complete/incomplete.
  • Move visited status after item title and wrap in parentheses when it exists.

Then, I've added period after the status so that it reads:

[status]. [title]

@swashbuck swashbuck requested a review from oliverfoster July 6, 2022 17:15
@oliverfoster
Copy link
Member

you need to do the complete/incomplete > visited conversion

@swashbuck
Copy link
Contributor Author

you need to do the complete/incomplete > visited conversion

Apologies, I misread that chat yesterday. Should be good now.

Copy link
Contributor

@joe-allen-89 joe-allen-89 left a comment

Choose a reason for hiding this comment

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

Tested with FW v5.20.1, screen reader NVDA in chrome, all seems to be working as expected/described in the comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@oliverfoster oliverfoster merged commit 6bb82e8 into adaptlearning:master Jul 24, 2022
github-actions bot pushed a commit that referenced this pull request Jul 24, 2022
## [7.2.1](v7.2.0...v7.2.1) (2022-07-24)

### Fix

* Improved accessibility, added checkmark (fixes #118) (#119) ([6bb82e8](6bb82e8)), closes [#118](#118) [#119](#119) [#118](#118) [#118](#118) [#118](#118) [#118](#118)
@github-actions
Copy link

🎉 This PR is included in version 7.2.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot pushed a commit to deltanet/adapt-contrib-accordion-audio that referenced this pull request Aug 18, 2022
# [4.3.0](v4.2.0...v4.3.0) (2022-08-18)

### chore

* added package.json ([5c04e24](5c04e24))
* added package.json ([7feeca5](7feeca5)), closes [#3072](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/3072)

### Fix

* Added gitignore for release automation (adaptlearning#124) ([4d08acd](4d08acd)), closes [adaptlearning#124](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/124)
* Added package-lock.json (adaptlearning#123) ([70465a1](70465a1)), closes [adaptlearning#123](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/123)
* Added release automation (adaptlearning#122) ([3dbefe6](3dbefe6)), closes [adaptlearning#122](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/122)
* Improved accessibility, added checkmark (fixes adaptlearning#118) (adaptlearning#119) ([6bb82e8](6bb82e8)), closes [adaptlearning#118](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/118) [adaptlearning#119](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/119) [adaptlearning#118](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/118) [adaptlearning#118](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/118) [adaptlearning#118](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/118) [adaptlearning#118](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/118)

### New

* Issue and pr project addition automation (refs adaptlearning/adapt_framework#3315) (adaptlearning#121) ([ce0888f](ce0888f)), closes [adaptlearning#121](https://github.com/deltanet/adapt-contrib-accordion-audio/issues/121)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing semantic differentiation between status and heading
5 participants