-
Notifications
You must be signed in to change notification settings - Fork 46
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
Voiceover announcement and programmatic association to field #54
base: master
Are you sure you want to change the base?
Conversation
maria-gourevitch
commented
Sep 1, 2023
- Add role=text on the actual number of words. This reduces the changes of Voiceover (especially on MacOS) garbling the announcement.
- Programmatically associate the counter with the text field. This will allow screen reader users to hear this information when they focus in the field (either right away or by asking for more details, depending on their screen reader settings). This is to support WCAG success criterion 1.3.1 Info & Relationships, using advisory technique ARIA1
This should prevent VoiceOver announcing just "remaining" (VoiceOver has a well-known issue where it does not announce the entire element when there are inline tags). There should be no effect on other screen readers.
…is a conflict with the auto-generated ID). Use it to programmatically associate counter with textarea.
…avoid random-sounding announcements on ajax submissions (observed in Drupal 9).
…s helps avoid random-sounding announcements on ajax submissions (observed in Drupal 9)." This reverts commit d1f8844.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Just had a couple small comments, but this would be very helpful to merge into the project.
@@ -135,7 +135,7 @@ countOverflowText : "Maximum %type exceeded by %d", // count overflow | |||
countOverflowContainerClass : "text-count-overflow-wrapper", // class applied to the count overflow wrapper | |||
minDisplayCutoff : -1, // maximum number of characters/words above the minimum to display a count | |||
maxDisplayCutoff : -1, // maximum number of characters/words below the maximum to display a count | |||
|
|||
counterId' : null, // custom ID for the counter element (e.g. to prevent conflicts). If one is not provided, a default ID will be assigned based on the ID of the element. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
counterId' : null, // custom ID for the counter element (e.g. to prevent conflicts). If one is not provided, a default ID will be assigned based on the ID of the element. | |
counterId : null, // custom ID for the counter element (e.g. to prevent conflicts). If one is not provided, a default ID will be assigned based on the ID of the element. |
@@ -21,12 +21,14 @@ | |||
base.init = function() { | |||
base.options = $.extend({}, $.textcounter.defaultOptions, options); | |||
|
|||
var counterElementId = base.options.counterId ?? base.el.id + '_counter'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only concern with setting it this way is if there are multiple instances on a page, with an empty ID, then this would create duplicate IDs. Perhaps something like jQuery UIs uniqueId
function behavior could be duplicated here instead: https://github.com/jquery/jquery-ui/blob/1be45388176080418aef84f3c98fda2c10fcdc42/ui/unique-id.js#L43