Page MenuHomePhabricator

Make buttons steal focus after click unless configured otherwise
Closed, ResolvedPublic

Event Timeline

Jdforrester-WMF raised the priority of this task from to Medium.
Jdforrester-WMF updated the task description. (Show Details)
Jdforrester-WMF added a project: OOUI.
Jdforrester-WMF moved this task to Backlog on the OOUI board.
Jdforrester-WMF changed Security from none to None.
Jdforrester-WMF subscribed.

So in other words, make the weird tabindex hack in ButtonElement optional? Why do we even have this hack? (Why do we prevent "focus stealing"?) It was added in 2120e338e6605e076638c2d27a88e72756030d40 and there's no explanation there as to why.

matmarex changed the task status from Open to Stalled.Feb 5 2015, 5:28 PM
matmarex lowered the priority of this task from Medium to Low.Feb 17 2015, 2:36 PM

This indirectly caused issues in API sandbox, see T128054.

matmarex added a subscriber: TheDJ.

@TrevorParscal, do you by any chance remember what was the rationale for buttons not taking focus when clicked?

@TheDJ, do you see any accessibility-related reason why buttons shouldn't take focus when clicked?

it's probably done, because otherwise it steals the focus from the editable area, this is often not desired in toolbars for textual input (because you loose your cursor position).

For keyboard accessibility, you would therefor use keyboard combinations (as documented in the help) menu. you don't go hopping around with your focus to use a button, while you are trying to write text into an area.

For screen readers, the focus doesn't really matter, the focus (mostly) follows the accessibility cursor automatically.

Yeah, but we don't use ButtonElement in toolbars (e.g. in VE, the "Save page" button is the only actual button, everything else is toolbar stuff).

I noticed that on Firefox and Chrome on MacOS the buttons don't get focus even after removing the hack from ButtonWidget. This is only the case with OOui buttons, normal buttons and links get focus, but not the visible focus state.

Explicitly giving them focus onMouseDown works

I was looking at this again and apparently the explicit focus was required because ButtonElement sets [[ https://doc.wikimedia.org/oojs-ui/master/js/#!/api/OO.ui.mixin.ButtonElement-static-property-cancelButtonMouseDownEvents | cancelButtonMouseDownEvents ]] to true. @matmarex, Would changing this for ButtonElement cause issues with the Toolbar?

Change 281638 had a related patch set uploaded (by Prtksxna):
ButtonWidget: Remove code to not let the button get focus after clicking

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

I was looking at this again and apparently the explicit focus was required because ButtonElement sets [[ https://doc.wikimedia.org/oojs-ui/master/js/#!/api/OO.ui.mixin.ButtonElement-static-property-cancelButtonMouseDownEvents | cancelButtonMouseDownEvents ]] to true. @matmarex, Would changing this for ButtonElement cause issues with the Toolbar?

With Toolbar? Probably not. I have no idea if there's something that would be negatively affected. I think we should try and see.

matmarex changed the task status from Stalled to Open.Jul 27 2016, 11:17 PM

Change 281638 merged by jenkins-bot:
ButtonWidget: Remove code to not let the button get focus after clicking

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

Jdforrester-WMF claimed this task.
Jdforrester-WMF reassigned this task from Jdforrester-WMF to Prtksxna.
Jdforrester-WMF moved this task from Doing to OOjs-UI-0.18.0 on the OOUI board.
Jdforrester-WMF edited projects, added OOUI (OOjs-UI-0.18.0); removed OOUI.
Jdforrester-WMF removed a project: Patch-For-Review.