Page MenuHomePhabricator

Regression: using keyboard cursor or tab keys makes parameter selection disappear.
Closed, DeclinedPublic

Description

Requirements

  • When entering into a list of parameters using keyboard navigation, any selected parameter will lose its selection/blue highlight. If a template name or wikitext element is highlighted in the sidebar, this remains (until a different element is selected).
  • When entering into a list of parameters using keyboard navigation, the first parameter should be hovered/focused, even if a parameter within the list was previously selected (currently it is jumping to selected parameter instead of starting at the top)

Background
Steps to reproduce:

  • Open the template dialog.
  • Use the <tab> key to get to a parameter list.
  • Use the up and down arrow keys to highlight a parameter, or <tab> to move focus to the next part.
  • Type the <space> key to select a parameter.

image.png (407×325 px, 27 KB)

  • Click an up or down arrow key.

What happens:

  • There's no indication that the previous parameter is still selected.

image.png (435×324 px, 27 KB)

What should have happened It has been determined that this is not possible, therefore the requirements have been adjusted as shown at the top.
* The previous parameter should be selected and show a blue background, the newly-highlighted item should show a gray background.

image.png (361×176 px, 10 KB)

Event Timeline

awight renamed this task from Regression: using keyboard cursor keys makes parameter selection disappear. to Regression: using keyboard cursor or tab keys makes parameter selection disappear..Jul 4 2022, 1:35 PM
awight updated the task description. (Show Details)
awight removed awight as the assignee of this task.Jul 4 2022, 2:01 PM
awight moved this task from Doing to Sprint Backlog on the WMDE-TechWish-Sprint-2022-06-22 board.

This isn't as easy as I'd hoped, because we rely on some OOUI functionality from SelectWidget which assumes that there can be multiple "selections" (checked boxes) but only zero or one "highlight". We already override the highlight concept a bit by making it gray when we're keyboard-navigating, and blue when not focused. This visually maps to a highlight / selection distinction, although it is not represented this way internally. The feature as I describe it in the task would require two highlighted items with different conceptual meanings.

I would simply decline this task, but there's still some UI funkiness that we might be able to handle better. For example, when the user tabs out of the parameter list, we could reassert the sidebar selection? I'll do this for now and will leave in the UX column for design review.

Change 811262 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Restore selection after keyboard parameter navigation

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

Change 811262 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Restore selection after keyboard parameter navigation

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

Updated the task description to reflect the decision after discussing with Lena, keeping the de-selecting behavior consistent, so that it deselects no matter which list the user tabs into.

The decision from story time to remove the behavior of "reassert the sidebar selection" also remains.

Change 811994 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Revert "Restore selection after keyboard parameter navigation"

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

Change 811996 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] [WIP] Clear selected parameter when focusing a selection list

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

awight removed awight as the assignee of this task.Jul 7 2022, 1:54 PM
awight moved this task from Doing to Tech Review on the WMDE-TechWish-Sprint-2022-07-06 board.

Chosen solution makes another regression more noticeable: T312542

Change 811994 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Revert "Restore selection after keyboard parameter navigation"

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

Patches to be merged don't seem to satisfy the requirements listed here.

Change 813224 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Safe to call setPage with a null page

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

Change 813225 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Unset selection through the top-level container

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

Change 813224 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Safe to call setPage with a null page

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

@ECohen_WMDE Just to mention, the code that was preventing the blue and gray highlighting from coexisting has been fundamentally changed, and your original design where the blue highlighting stays and the gray stuff moves around is already working. Try it out on the beta cluster!

Shall we leave it this way, or continue with the requirements in this task?

awight moved this task from Doing to UX/PM Review on the WMDE-TechWish-Sprint-2022-07-06 board.

@ECohen_WMDE Just to mention, the code that was preventing the blue and gray highlighting from coexisting has been fundamentally changed, and your original design where the blue highlighting stays and the gray stuff moves around is already working. Try it out on the beta cluster!

Even better! I think we only went with these requirements based on the understanding that that was not possible. Just tried out on beta cluster and it's looking good. Should this ticket be resolved or declined then?

Great! Declining for now. "Invalid" might have been fine as well, I don't really know...

Change 811996 abandoned by Awight:

[mediawiki/extensions/VisualEditor@master] Clear selected parameter when focusing a selection list

Reason:

We've decided against this behavior.

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

Change 813225 abandoned by WMDE-Fisch:

[mediawiki/extensions/VisualEditor@master] Unset selection through the top-level container

Reason:

Solved differently.

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