-
Notifications
You must be signed in to change notification settings - Fork 2
Distinguish samey links with aria attributes #455
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
base: main
Are you sure you want to change the base?
Conversation
|
In addition to other comments, while looking at this I found a few places where "signup" is used - I find that voiceover can't pronouce this properly, as it is two words run together, needs to be "sign up or "sign-up". |
components/news/list_item.html
Outdated
| "> | ||
| <div class="max-lg:mt-8 lg:m-6"> | ||
| {% with rel_path=item.url label="Read more" %} | ||
| {% with rel_path=item.url label="Read more" describedby=item.title|slugify %} |
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.
When testing with voiceover I have a chaining of almost identical aria here, it comes in the pattern of
- "Read more" (text of the link itself)
- The article title linked as a describedby
- The aria for the article landmark - which is the article title (line 4).
So I end up with the article title being read aloud twice, one after the other.
May I suggest pointing the Read more describeby at the article landmark rather than the article title?
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.
Doing away with the article landmark and putting the news items in a list resolves this issue, I believe. I've also changed the strong around the item title to an appropriate heading, either h2 or h3.
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.
Something's not right here. The view all bars on the announcements-blog page are not being described in a way that tells me where that link goes.
What I get is:
Read our news section
link, announcement, complementary
Hear our ideas section
link, blog, complementary
News from our journals section
link, all, complementary
I think the complementary comes from it being an 'aside'. But this isn't an aside, it is part of the section as above - if it could be in that section, then it should read out that landmark instead of 'complementary' which could be enough not to need more description. However I do think the names need to be different (i.e. announcement, blog, do not give enough context for user to know where they are going).
I don't think that is is what is being passed in by announcements-blog.html
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.
Great catch--there was a bug and a less than ideal use of aside here, and I've fixed both.
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.
see view_all_bar.html comment. The label being passed to view_all_bar doesn't seem to match what is being put on the page by that component.
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.
Download links need to make it clear they are for download not a link to another page - Elsewhere we use "Download PDF" but here we don't.
I also think it woudl be good to have the download links as a list.
|
Regarding the |
StephDriver
left a comment
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.
thank you - I particularly like your 'download' button solution.


Fixes #251.
I tested all these with Android TalkBack.
There was a slight complication in that buttons are now sensitive to variables called
labelledbyanddescribedbyandaria_labelfrom the parent context, so care has to be taken when including the template not to let something leak down and pollute the correct label, such as from an h2 when the best reference is a closer h3. I tested for all possible occurrences of this and made fixes to avoid them. This can be protected against in a more general way if we upgrade the button templates to components using Django Components, which has a syntax for controlling component context and scope, but I believe that's beyond the scope of this fix.