Page MenuHomePhabricator

Add keyboard shortcuts to move between revisions
Closed, ResolvedPublic

Description

Motivation
As a user, I can't easily move to the previous (or next) set of revisions easily without using the mouse.

Solution
(Old suggestion: From https://www.mediawiki.org/wiki/Topic:Tnzvxtfzp9oqsw5v :)

terminology as on https://www.mediawiki.org/wiki/Extension:RevisionSlider

image.png (424×1 px, 27 KB)

The basic implementation will cover moving the pointers by keyboard, just like dragging them. There are currently no specs for the equivalent of hover states or moving both knobs together.

1– Tabbing order for setting focus to elements (explained here for left to right languages, for right to left languages it should be the other way round):

  • 1. left change-view arrow (if there),
  • 2. right change view arrow (if there) ,
  • 3. yellow knob
  • 4. blue knob
  • 5. help button

2– Behaviour
If the tab focus is on one of the knobs, following actions are possible on the focused knob. :

  • An arrow left on the keyboard moves one revision to the left
  • An arrow right on the keyboard moves one revision to the right

as if it would have been dragged there. When the key is released (Why released? A change triggers a redraw of the diffs and many diffs in succession would cause many redraws), the action is executed.

No other actions are specified/implemented

Edge cases:
If the slider is at the end of its range (either range of the loaded revisions or next to the other slider: No action. (There would be some plausible action that could be taken, but dragging does not do an additional action, so we stay within that behavior)

3– Styles

For now, the styles applied when focus is on a slider, would be the same as the one applied on hover: The slider line is less transparent, the pointer brighter (this needs to be looked up in the current implementation since the same styles and possibly CSS classes can apply). The style is un-applied when the focus moves somewhere else.

Background of the RevisionSlider
The RevisionSlider is an extension that adds a slider view to the diff page so that you can easily move between revisions.

Event Timeline

Relevant for that topic, too: T155499: Simultaneous moving of both pointers
Here, another idea proposed is that by pressing a key on the keyboard while using the mouse, you automatically move both pointers

@Jan_Dittrich Could you add your notes on keyboard focus here as well?

I think we should go with a standard approach, because:

  • We know it works
  • Our users know how it works (in particular users that are dependent on keyboard use)

What would that entail?

  • It should be able to tab-key to the slider element
  • It should get a "focused" state shows that it is currently in focus
  • If the user then presses the arrow keys the slider moves.

So it would be like this jQuery Slider

For screenreader support see as well default recommendations here: https://www.w3.org/TR/wai-aria-practices/#slider

Jan_Dittrich updated the task description. (Show Details)
Jan_Dittrich updated the task description. (Show Details)
Lea_WMDE triaged this task as Medium priority.Aug 2 2018, 12:40 PM
Lea_WMDE updated the task description. (Show Details)

Change 459678 had a related patch set uploaded (by SrishtiSethi; owner: srish):
[mediawiki/extensions/RevisionSlider@master] Add keyboard shortcuts to move between revisions

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

Change 459678 merged by jenkins-bot:
[mediawiki/extensions/RevisionSlider@master] Add keyboard shortcuts to move between revisions

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

Yay to moving knobs! :)
I just did not manage to navigate to anything else: I pressed enter to open the revisionSlider, and then tabbed. First tab brought me to one knob, and the second tab to the second knob. A third tab brought me to search (outside of the revisionSlider), then the url field, then search again, and then back to the knobs in the revisionSlider.
I expected tabs to follow this order:

  1. left change-view arrow (if there), (i.e. the big arrows that allow you to exchange the range of revisions you see)
  2. right change view arrow (if there) ,
  3. yellow knob
  4. blue knob
  5. help button

@Lea_WMDE I am trying to reproduce this issue on my end with no luck. The tabbing order works as desired. I am sharing a video that shows the same. May be I am missing something? :-)

Works for me too. Maybe browser specific? @Lea_WMDE You're testing on which browser?

Oh no, this seems to be a firefox/OS problem maybe? I just tested it on Chrome and it works fine, but with Firefox 62 it is working as described above...

Change 473217 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/RevisionSlider@master] Fix (accidentally?) reversed blue and yellow lines

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

Change 473217 merged by jenkins-bot:
[mediawiki/extensions/RevisionSlider@master] Fix (accidentally?) reversed blue and yellow lines

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

Change 473710 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/RevisionSlider@wmf/1.33.0-wmf.4] Fix (accidentally?) reversed blue and yellow lines

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

Change 473711 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/RevisionSlider@wmf/1.33.0-wmf.3] Fix (accidentally?) reversed blue and yellow lines

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

Change 473711 merged by jenkins-bot:
[mediawiki/extensions/RevisionSlider@wmf/1.33.0-wmf.3] Fix (accidentally?) reversed blue and yellow lines

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

Change 473710 merged by jenkins-bot:
[mediawiki/extensions/RevisionSlider@wmf/1.33.0-wmf.4] Fix (accidentally?) reversed blue and yellow lines

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

Mentioned in SAL (#wikimedia-operations) [2018-11-15T12:37:06Z] <lucaswerkmeister-wmde@deploy1001> Started scap: php-1.33.0-wmf.3/extensions/RevisionSlider/modules/ext.RevisionSlider.SliderView.js [[gerrit:473710|Fix (accidentally?) reversed blue and yellow lines (T162119, T208238)]]

Mentioned in SAL (#wikimedia-operations) [2018-11-15T12:37:17Z] <lucaswerkmeister-wmde@deploy1001> sync aborted: php-1.33.0-wmf.3/extensions/RevisionSlider/modules/ext.RevisionSlider.SliderView.js [[gerrit:473710|Fix (accidentally?) reversed blue and yellow lines (T162119, T208238)]] (duration: 00m 11s)

Mentioned in SAL (#wikimedia-operations) [2018-11-15T12:38:33Z] <lucaswerkmeister-wmde@deploy1001> Synchronized php-1.33.0-wmf.3/extensions/RevisionSlider/modules/ext.RevisionSlider.SliderView.js: [[gerrit:473710|Fix (accidentally?) reversed blue and yellow lines (T162119, T208238)]] (duration: 00m 54s)

Mentioned in SAL (#wikimedia-operations) [2018-11-15T12:59:47Z] <tarrow@deploy1001> Synchronized php-1.33.0-wmf.4/extensions/RevisionSlider/modules/ext.RevisionSlider.SliderView.js: [[gerrit:473710]] Fix (accidentally?) reversed blue and yellow lines SWAT T208238 T162119 again (duration: 00m 55s)

@thiemowmde Oops! Exactly what @WMDE-Fisch commented on the patch, the flip was done to support tabbing in the desired order. My bad that in this process, I ignored the flip caused on the interface.

What would be the next step in this ticket? I've some time now and would like to work on it! If there are any new changes, then should they be uploaded in a new patch?

@thiemowmde Oops! Exactly what @WMDE-Fisch commented on the patch, the flip was done to support tabbing in the desired order. My bad that in this process, I ignored the flip caused on the interface.

What would be the next step in this ticket? I've some time now and would like to work on it! If there are any new changes, then should they be uploaded in a new patch?

Yeah that was a bit unfortunate - also that I missed that while reviewing the patch. I see two alternatives to bring the tab order back:

  • add tab-indexes to all the UI elements to overcome the DOM order
  • change the DOM but rebuild the styling in all relevant aspects so it works with the changed order

The latter is definitely more complicated since I can not overlook what this might imply and what ( maybe hidden ) dependencies in layout and pointer position calculations are affected. The former is not the cleanest way to solve this though, since we never know what other UI elements might want to use the tab-index to order the tabbing.

@thiemowmde any opinions form you side?

I would go with the second option, change the order in the DOM, but make sure the layout will be the exact same as before. I had a brief look and it looks like the only code that needs touching is some CSS (in ext.RevisionSlider.less). I gave it a quick shot and found this possible solution. Not sure it's the best.

.mw-revslider-pointer-container-older {
    margin-top: 8px;
}
.mw-revslider-pointer-container-newer {
    margin-top: -12px;
}

Change 475684 had a related patch set uploaded (by SrishtiSethi; owner: srish):
[mediawiki/extensions/RevisionSlider@master] Change the DOM order of pointers and rebuild styling

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

Oh no, this seems to be a firefox/OS problem maybe? I just tested it on Chrome and it works fine, but with Firefox 62 it is working as described above...

I investigated this issue a little bit - it looks like it is MAC OS X+Firefox issue mainly.

I tested it on Fedora 28/Firefox 63.0.1, and the tabbing order works as desired. I tested it on MAC OS X 10.11.6/Firefox 63.0.3, and the tabbing does not work in the same way. It looks like on OS X a elements
aren't accessible by default, with or without the tabindex property explicitly set to pointers for this feature. As explained in this article https://www.alexlande.com/articles/cross-browser-tabindex-woes/, after changing my keyboard settings to "Full Keyboard Access," the tabbing works!

Thanks for looking into it even more @SrishtiSethi! If I get it right we went all the reasonable ways we could think of for now, didn't we? Then I would be ok with keeping it the way it is.

Change 475684 merged by jenkins-bot:
[mediawiki/extensions/RevisionSlider@master] Change the DOM order of pointers and rebuild styling

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