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

Add hidden span labels for help links. #2630

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

somiaj
Copy link
Contributor

@somiaj somiaj commented Nov 22, 2024

This adds a hidden span that gives a label for the help links for the main help link using the helpMacro and the help links on the course configuration page. There is now an aria-labelledby setting for this description which describes what the help link is for.

The page title is used for the description for the main help link. For the course configuration links, the setting short description is used for the title. A few of the short descriptions ended in a period, those were all removed to make the description "desc help." consistent and not have a period before the help statement.

I couldn't figure out a better label for the course configuration help statements than the short description. This seems a little long in some cases, but overall identifies what setting the help buttons is for.

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

When we talked about this in the meeting I think that you got mislead on a few things.

You mentioned that the visually hidden spans for the help icons on the course configuration page merely say "help", and that they should be more specific. However, that isn't true. Those should just say "help". The thing is that when designing a page for a screen reader, it is important to control the verbosity and prevent text from being read multiple times. If these buttons say, for instance, "Title for course displayed on the Assignments page help" and the text for the table also says "Title for course displayed on the Assignments page", then the screen reader reads both of those together making it rather verbose. In the context since the text in the table will be read with together with the text for the icon, "help" is sufficient.

The same is true for the help links for the page by the page title. Of course, those should not have the question mark for the content of the visually hidden span since that is not read by screen readers, but should have "help" for the content instead.

Also, the actual icons should are for sighted users only, and don't add anything for those using a screen reader. So they should not have the aria-labelledby attribute, and instead should have the aria-hidden attribute. This is so that the visually hidden span becomes what is read for link and the icon is completely ignored. So the point is that the visually hidden spans as they were are structurally and semantically correct.

I actually also did some work on this after we discussed this in the meeting, and created a branch to fix the "?" help links, and removed the addition of the visually hidden spans by javascript. I will go ahead and put that in as a pull request for comparison.

@somiaj
Copy link
Contributor Author

somiaj commented Nov 22, 2024

Ahh, I was considering the links in isolation too much, and thought having a lot of "Help link" would not be clear what the help was for, so added more to make that clear without considering where the link was being placed. Thanks.

Closing in favor of #2631.

@somiaj somiaj closed this Nov 22, 2024
@drgrice1
Copy link
Member

Actually, I am going to reopen this. I would like @Alex-Jordan's opinion on this (or anyone else who has experience in this area). I am certain that the aria-labelledby is not correct, but the more verbose text I am not certain of. I tested with both Gnome Orca and Android Talkback. With Gnome Orca the headers are read with the link, and it does seem rather redundant in that case. However, with Android Talkback the link is read separately, although it is still the next thing that is read after the header.

@drgrice1 drgrice1 reopened this Nov 22, 2024
@drgrice1
Copy link
Member

It seems that it is difficult to get screen reader support right. Largely due to the fact that different screen readers seem to do rather different things. It seems that screen reader support is not entirely standardized.

@somiaj somiaj force-pushed the help-hidden-labels branch 2 times, most recently from 320540a to 1dbf2e3 Compare November 23, 2024 01:39
@somiaj
Copy link
Contributor Author

somiaj commented Nov 23, 2024

I removed the aria-labelledby tags and ids. If we do end up needing to with the more verbose text, I'll also update this to remove more of the alt text being added by javascript like #2631.

I am unsure what is best here, I do know with normal links the standard seems to be descriptive (don't use things like 'click me') and that links that do different things should have different text (which is partly why I thought all the links saying "help" might not be best). Anyways waiting to see if others know the best accessible standards here.

This adds a hidden span that gives a label for the help links for
the main help link using the helpMacro and the help links on the
course configuration page.

The page title is used for the description for the main help link.
For the course configuration links, the setting short description
is used for the title. A few of the short descriptions ended in a
period, those were all removed to make the description "desc help."
consistent and not have a period before the help statement.
@somiaj somiaj force-pushed the help-hidden-labels branch from 1dbf2e3 to 3b7b004 Compare December 10, 2024 19:48
. $c->tag(
'span',
class => 'visually-hidden',
$c->maketext('[_1] help.', $args->{help_label} // $c->page_title)
Copy link
Member

Choose a reason for hiding this comment

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

I wanted to point out why I like my approach better. The reason is mainly the change here. My approach uses the help page layout title which already is translated and includes "help", and doesn't add a new optional argument to the helpMacro method. So there is no need to go through all of the helpMacro calls and add this help_label argument. For example, the set detail page help title is "Set Detail Help", and that title is used for the help link to that help. I also moved the html generated here via the tag method into the help_macro.html.ep template which is cleaner because you can just use the i and span html tags directly.

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 didn't consider using those titles, and agree since they are already there they should be used. I think this also makes sense since in another view point, these are links to those headers, which should line up. Now that I understand the difference, I to like your approach better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants