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

fix accessibility #2705

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

fix accessibility #2705

wants to merge 4 commits into from

Conversation

arthur-lemeur
Copy link
Collaborator

@arthur-lemeur arthur-lemeur commented Dec 11, 2024

Fixes #2698
Fixes #2700
Fixes #2709
Fixes #2710
Fixes #2711

(PS : The "Tag" element from React Aria does not allow for a "title" attribute ([https://react-spectrum.adobe.com/react-aria/TagGroup.html#tag]), I updated the "textValue" attribute (rendered as "aria-label") to be localised instead. According to the documentation, this is the attribute used for accessibility on this element.)

@gautierchomel
Copy link
Member

gautierchomel commented Dec 11, 2024

Aria is used by assistive technologies and, therefore, targets a subset of readers with accessibility needs. It is always better to have an accessible name available for all readers (as title) instead of a specific user agent category.

@gautierchomel
Copy link
Member

Any way I can't find the "textValue" attribute (rendered as "aria-label") in that PR.

Copy link
Member

@gautierchomel gautierchomel left a comment

Choose a reason for hiding this comment

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

If title does not refer to the HTML argument title="", then it should have another name to avoid confusion.

@@ -1043,7 +1043,7 @@ const AnnotationList: React.FC<{ annotationUUIDFocused: string, resetAnnotationU
</div>
</summary>
<TagList items={selectDrawtypesOptions} className={stylesAnnotations.annotations_filter_taglist}>
{(item) => <Tag id={item.name} className={stylesAnnotations.annotations_filter_drawtype} textValue={item.name}><SVG svg={item.svg} /></Tag>}
{(item) => <Tag id={item.name} className={stylesAnnotations.annotations_filter_drawtype} textValue={item.title}><SVG svg={item.svg} /></Tag>}
Copy link
Member

Choose a reason for hiding this comment

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

I undesrtand that title: ${__("reader.annotations.type.strikethrough")} }, is not referint to the HTML title argument, but used as a text value for an argument rendered as aria-label.

Therefore, it should have a different name to avoid confusion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to "textValue"

@gautierchomel gautierchomel self-requested a review December 11, 2024 14:01
@arthur-lemeur arthur-lemeur changed the title fix missing titles for accessibility in annotations menu fix accessibility Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants