Page MenuHomePhabricator

PopupButtonWidget's `aria-haspopup` provides no value in current implementation
Closed, ResolvedPublic

Description

Currently, as of v0.26.5, we're setting aria-haspopup on PopupButtonWidget, introduced in T87694
Not only do we set it on a non-focusable element (the surrounding span). We also don't change indicate the child link as triggering element clearly. Setting aria-haspopup there is of no use for screenreaders.

Apart from mid-future accessibility and usability improvements in T185752, it would be better to add an id to the description-holding div and add an aria-describedby=ID to the link on hover and focus.

Event Timeline

Volker_E created this task.May 8 2018, 5:12 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 8 2018, 5:12 AM
Volker_E renamed this task from PopupButtonWidget's `aria-haspopup` is of no value to PopupButtonWidget's `aria-haspopup` provides no value in current implementation.May 8 2018, 5:15 AM

We could probably add this.popupButtonWidget.$element, to

(
	this.fieldWidget.$input ||
	this.fieldWidget.$button ||
	this.fieldWidget.$element
)

Change 431702 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[oojs/ui@master] [WIP] PopupButtonWidget: Improve screenreader output

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

Yep, in its current form the aria-haspopup is not helping at all, removing it makes sense. Also, re-reading the spec makes me feel that no matter the element (surrounding span or a), this attribute doesn't belong here since the popup isn't a menu, listbox, tree, grid or dialog.

Authors MUST ensure that the role of the element that serves as the container for the popup content is menu, listbox, tree, grid, or dialog, and that the value of aria-haspopup matches the role of the popup container.

A tooltip is not considered to be a popup in this context.

We could probably add this.popupButtonWidget.$element, to

(
	this.fieldWidget.$input ||
	this.fieldWidget.$button ||
	this.fieldWidget.$element
)

I am not sure if this makes sense. This would only get triggered when there is a FieldLayout which has a config.help set, but doesn't have an input, button or any element, essentially an empty FieldLayout. I can't think of any real world place where one would want to do this (empty FieldLayout with help).

Volker_E moved this task from Backlog to Next-up on the OOUI board.May 29 2018, 9:48 PM
Volker_E triaged this task as High priority.Jun 4 2018, 4:01 PM

https://inclusive-components.design/tooltips-toggletips/ has some important insights, for example, that aria-decribedby might not be solving our current issue when the tooltip is hidden beforehand.

It might prove tricky to implement any accessibility guideline on PopupButtonWidget itself as its in being inherited and used in all sorts of non-tooltip ways — it is a table in Kartographer, colorpicker in RCFilter, as settings panel in ContentTranslation, and a map in UploadWizard.


The article that @Volker_E shared talks specifically about the way we are using PopupButtonWidget in a FieldLayout, and it defines it as a toggletip.

Toggletips are like tooltips in the sense that they can provide supplementary or clarifying information. However, they differ by making the control itself supplementary: toggletips exist to reveal information balloons, and serve no other purpose.

Often they take the form of little "i" icons.

According to it, the way we're doing it right now in FieldLayout, is accessible, but not good UX:

(
	this.fieldWidget.$input ||
	this.fieldWidget.$button ||
	this.fieldWidget.$element
).attr(
	'aria-describedby',
	this.popupButtonWidget.getPopup().getBodyId()
);

It then talks about the aria-describedby attribute, but in the context of the button, not as attached to the input (how we do it).

Crucially, this means the aria-describedby association is no longer appropriate. Why? Because a screen reader user would have access to the information before pressing the button, so pressing it would appear not to do anything. Technically, they have access to the information, making the control "accessible" — but the control simply wouldn't make sense. In other words, it's a user experience issue more than a strict accessibility one, but it's important.

This is an interesting point that I hadn't thought about earlier. According to their donts one shouldn't use aria-describedby on the toggletip, which we aren't, we use aria-describedby on the element that its describing (per code above), not the button. We still have the problem that on clicking the button, the screen reader has nothing to say. The user will still get the information when they reach the form element, but clicking the button would do nothing. The article recommends the following:

The trick is to make screen readers announce the information after the click event. This is a perfect use case for a live region. We can supply an empty live region, and populate it with the toggletip "bubble" when it is invoked. This will both make the bubble appear visually and cause the live region to announce the tooltip's information.

This is an interesting idea, but possibly out of the scope of this particular task (happy to raise a new one). Also, in our context it would be a UX improvement and not a a11y improvement, but an improvement nonetheless.


Given this, and the spec on aria-haspopup, I have the following proposal — when the PopupButtonWidget has a label we can assume that it is being used as a tooltip (or toggletip) and thus omit aria-haspopup since tooltips aren't considered popups in this context. And, when popup.$content is passed we can assume that something more complicated is going on and set aria-haspopup to dialog and make sure that $content has dialog as a role too.

In any case, just setting the attribute does nothing, and @Volker_E's patch to remove that makes sense.

Change 431702 merged by jenkins-bot:
[oojs/ui@master] PopupButtonWidget: Remove 'aria-haspopup' attribute

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

Vvjjkkii renamed this task from PopupButtonWidget's `aria-haspopup` provides no value in current implementation to 8edaaaaaaa.Jul 1 2018, 1:11 AM
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: gerritbot, Aklapper.
CommunityTechBot renamed this task from 8edaaaaaaa to PopupButtonWidget's `aria-haspopup` provides no value in current implementation.Jul 2 2018, 4:32 AM
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot added subscribers: gerritbot, Aklapper.
Volker_E lowered the priority of this task from High to Low.Dec 18 2018, 7:16 AM

Can this task be resolved as the aria-haspopup attribute has been removed (which is what the task summary is about)?

matmarex closed this task as Resolved.Feb 1 2019, 9:25 PM

I guess. Maybe we should file another one about adding it back in a different way, if that would be useful.

Volker_E claimed this task.Feb 5 2019, 1:37 AM