Page MenuHomePhabricator

[BUG] Parameter buttons should have screenreader friendly tooltips
Closed, ResolvedPublic1 Story Points

Description

Pointed out by @TheDJ on the project talk page.

Problem:

The parameter buttons only contain the parameter description right now but don't convey the state (+/-) of the button.

Proposed solutions:

  • If the parameter has not been added (+ state) the tooltip should say: Add the parameter $paramName
    • For example if the parameter is Date of Birth it should be Add the parameter Date of Birth
  • If the parameter has been added (- state), the tooltip should say: Remove the parameter $paramName
    • For example if the parameter is Date of Birth it should be Remove the parameter Date of Birth

Event Timeline

Niharika triaged this task as Normal priority.Sep 5 2018, 8:37 PM
Niharika created this task.
Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptSep 5 2018, 8:37 PM

Change 458292 had a related patch set uploaded (by Mooeypoo; owner: Mooeypoo):
[mediawiki/extensions/TemplateWizard@master] Add tooltips to parameter buttons for accessibility

https://gerrit.wikimedia.org/r/458292

Mooeypoo claimed this task.Sep 5 2018, 8:41 PM
Mooeypoo set the point value for this task to 1.
Mooeypoo moved this task from Ready to Needs Review/Feedback on the Community-Tech-Sprint board.

@Niharika there was a question raised during review by @Samwilson that requires your input: We've changed the titles/tooltips of the +/- field buttons, as per spec, but we also have "required" buttons that have the checkmark, and cannot be added or removed.

In the current state (before this commit) all buttons displayed the description of the field, but we're replacing that to describe the actual action of the +/- field.... and aren't sure what to do with the 'required' fields now. Should we add some text about the field being required? Should we put any other text in there?

Ideally, we'd have no action on it at all, but for consistency, these are still buttons that are disabled, so we need to find a good text for these.

Niharika updated the task description. (Show Details)Sep 10 2018, 2:27 PM

First, I forgot something - we use parameter everywhere and not field. Let's switch the messages to say parameter instead (I updated the ticket description). Sorry about this.

@Niharika there was a question raised during review by @Samwilson that requires your input: We've changed the titles/tooltips of the +/- field buttons, as per spec, but we also have "required" buttons that have the checkmark, and cannot be added or removed.
In the current state (before this commit) all buttons displayed the description of the field, but we're replacing that to describe the actual action of the +/- field.... and aren't sure what to do with the 'required' fields now. Should we add some text about the field being required? Should we put any other text in there?
Ideally, we'd have no action on it at all, but for consistency, these are still buttons that are disabled, so we need to find a good text for these.

Oh that's a good point. Since on the other parameters we are saying the parameter name, how about we say "Required parameter $paramName" for the required ones? That would make it clear to the user why that field is disabled.
Should we also try to make it clear that the parameter is already added to the right side?
@Mooeypoo @Samwilson @TheDJ thoughts?

My personal opinion is that it's probably sufficient. If we consider screen readers to represent the state of whatever the user is focused on, then we need to make sure the name of the field is there, as well as its state. If we add the action-related text for fields you can add/remove and have a string saying the name of the field (with its state) for required field, that sounds like it would be a good descriptor.

I'll change the commit; we can iterate, perhaps, if we see more problems?

TheDJ added a comment.Sep 13 2018, 9:45 PM

So from an Accessibility point of view, the best html for this url is:

<ul>
<li>
  Required parameter $paramName" for the required ones? 
</li>
</ul>
<ul>
<li>
  <span aria-description="long explanation">Param name</span> <button>add/remove parameter paramname</button>
</li>
<ul>

I note that while the required elements 'look' like a button, they never behave like a button and there is no situation in which you allow them to behave like that. As such it can be questioned if they are buttons at all...

Change 458292 merged by jenkins-bot:
[mediawiki/extensions/TemplateWizard@master] Add tooltips to parameter buttons for accessibility

https://gerrit.wikimedia.org/r/458292

So from an Accessibility point of view, the best html for this url is:

<ul>
<li>
  Required parameter $paramName" for the required ones? 
</li>
</ul>
<ul>
<li>
  <span aria-description="long explanation">Param name</span> <button>add/remove parameter paramname</button>
</li>
<ul>

I note that while the required elements 'look' like a button, they never behave like a button and there is no situation in which you allow them to behave like that. As such it can be questioned if they are buttons at all...

At the moment, both required and add/remove are buttons; the required button is disabled, and the add/remove are operational. So we don't have a <ul> list, they're all buttons.

There was a tentative plan to transform the sidebar to a menu, which would make it potentially easier to make the top items labels, and solve this issue "properly" -- but for the moment, @TheDJ I think that considering they're all buttons, the 'title' attribute, on all of them (and on the disabled buttons, the text describes the label, which is what you have in the list structure), should work sufficiently with accessibility concerns?

TheDJ added a comment.Sep 25 2018, 9:12 AM

@Mooeypoo I think the biggest issue here is that the buttons don't have state, as I describe in the description of my video walkthrough.

The list is something that could deal with the lack of grouping, but it is less essential I think.

Niharika closed this task as Resolved.Nov 15 2018, 11:47 PM
Niharika moved this task from QA to Q2 2018-19 on the Community-Tech-Sprint board.

I think it's best to close this ticket right now because the specific issue has been handled. There are other problems and we can work on them when we have more time and a better understanding of the problems and how to fix them.