Page MenuHomePhabricator

Missing accesskeys for user links buttons
Closed, ResolvedPublic3 Estimated Story Points

Description

Background

User page, watchlist buttons should have accesskey attributes, just like in legacy Vector.

AC

  • User page button should have an accesskey "." attribute
  • Watchlist button should have an accesskey "l" attribute
  • Search toggle button (shows up on smaller screen sizes) should have an accesskey "f" attribute
  • All buttons above can be accessed using accesskeys

Developer notes

The user page and watchlist links both are missing accesskeys because they have different ids from legacy (i.e. pt-userpage-2 and pt-watchlist-2), and the ids are used by Linker to provide accesskey attributes. We could fix this while keeping the new ids by adding single-id properties to the user page and watchlist data in core. However considering all the other user links use the original ids, I think it could be more consistent and maintainable to do the same for user page and watchlist as well.

There are also a few other cases in Vector where we manually provide link attributes (1, 2). Since we don't really have a consistent way to provide accesskey and title attributes in Vector, this might be a good opportunity to remove those in favor of using the correct id and relying on Linker to provide the attributes.

Adding a accesskey "f" attribute to the search toggle button works fine, but some reason it has to be reordered to go before the vue search in the DOM.

Event Timeline

Search toggle button (shows up on smaller screen sizes) should have an accesskey "f" attribute

I would suggest rather than adding another access key, making the access key for search automatically toggle on search.

Note that the watchlist icon also lost the tooltip, which is arguably more important. I'm reporting it here because accesskey and titleAttrib were historically maintained together in the skin and likely caused by the same problem.

I was actually rather puzzled on fr.wikipedia.org today as I hadn't seen the new icon before and was unable to guess what it represented despite having the UI language set to English.

b.png (522×1 px, 265 KB)
a.png (406×1 px, 110 KB)

The tooltip is already fixed in this week's train (reference https://www.mediawiki.org/wiki/MediaWiki) so this is unrelated to the changes proposed here. Please see the note at T289619#7634895

User page button should have an accesskey "." attribute

It seems access keys must be visible on the page to work so I am wondering if instead of an access key for the user page button, we could wire it up so that any access key of menu items are used we trigger it. Having to use 2 access keys - one to open the menu and one to select the item seems a bit of a cop-out. We might also want to consider hiding the menu via some other mechanism than display: none

Watchlist button should have an accesskey "l" attribute

Doing the above would also fix this issue, as the access key would already be associated with an item in the user menu.

Search toggle button (shows up on smaller screen sizes) should have an accesskey "f" attribute

my understanding is this would mean two "f" key presses to do a search. Could we make this also focus the input?

It seems access keys must be visible on the page to work so I am wondering if instead of an access key for the user page button, we could wire it up so that any access key of menu items are used we trigger it. Having to use 2 access keys - one to open the menu and one to select the item seems a bit of a cop-out. We might also want to consider hiding the menu via some other mechanism than display: none

This is currently working as expected in old Vector, so why not just use that method? When there are multiple action links like this, they get grouped together and hidden behind a "More" button, which shows when you hover it. However, the contents of the box that are hidden are still accessible by accesskeys.

This brings me to a semi-related issue: In 2022 Vector, this "More" button doesn't open when you hover it; you have to click it. Could that be changed as well to the "old behaviour"? When you're used to the hovering method, it's a bit frustrating to suddenly have to click something in order to do what you want.

button doesn't open when you hover it; you have to click it.

See T277322 instead

The user page and watchlist links both are missing accesskeys because they have different ids from legacy (i.e. pt-userpage-2 and pt-watchlist-2), and the ids are used by Linker to provide accesskey attributes. We could fix this while keeping the new ids by adding single-id properties to the user page and watchlist data in core. However considering all the other user links use the original ids, I think it could be more consistent and maintainable to do the same for user page and watchlist as well.

There are also a few other cases in Vector where we manually provide link attributes (1, 2). Since we don't really have a consistent way to provide accesskey and title attributes in Vector, this might be a good opportunity to remove those in favor of using the correct id and relying on Linker to provide the attributes. @Jdlrobson thoughts?

This is currently working as expected in old Vector, so why not just use that method?

Elements with access keys must be visible elements. Old Vector uses visibility: hidden; so these work. On the other hand the 2022 Vector uses display: block;. I'm not sure if there is a good reason for 2022 Vector using display but if there isn't good thing then sure: we can do the same thing.

@Jdlrobson thoughts?

The features are already accessible by access key, so rather than add new access keys, I'd work with the ones we have, and making sure they are "visible" in the sense that the access keys work by default. The search toggle does not need an access key. Only the input itself. Some JavaScript may be needed to make sure the input key toggle works even when search is collapsed.

I think it'd be simpler to just ensure that all user links have the correct attributes, whether they are inside the user menu dropdown or in the overflow section. Trying to make the accesskeys work for hidden links in the user menu would increase the complexity of our CSS, and might have accessibility implications (i.e. a duplicate hidden link could be available to screenreaders).

Besides, the title attribute still needs to be present on all links (i.e. the user page button in the overflow section is missing a title) either way. Considering we can get both title and accesskey attributes from core by providing the right id, this seems like a simpler solution.

The search toggle does not need an access key.

This is fine by me, I've removed it from the task description

I think it'd be simpler to just ensure that all user links have the correct attributes,

What would the plan be for accessing the elements inside the dropdown that are not outside the dropdown? e.g. user talk page. I presume these would need to be accessible still either via a JS solution or via changing the display.

Trying to make the accesskeys work for hidden links in the user menu would increase the complexity of our CSS, and might have accessibility implications (i.e. a duplicate hidden link could be available to screenreaders).

I think a change from display: none; to visibilty: hidden; wouldn't really increase the CSS complexity (it may probably not be quite that simple, but still). Besides, if accesskeys for common actions like delete or move aren't available, I'd say that's a sure-fire way of hindering power-users' adoption of Vector-2022.

Okay, I got confused, sorry.

  1. These menus are using visibility: visible and visibility: hidden.
  2. Majority of menu items are working
  3. The ones that are not working are "watchlist" and "user page". These are not working because there are 2 instances of these links: one in the dropdown at low resolutions and one outside the menu that's always accessible.
  4. The links outside the menu do not have access keys assigned. The collapsed ones are hidden using display: none so also do not work.
  5. In T299980#7800708 @bwang is suggesting we duplicate the access keys for these items by calling Linker::tooltipAndAccesskeyAttribs on https://gerrit.wikimedia.org/g/mediawiki/skins/Vector/+/dd65dbebf4f3ef34c500a54d22fd6c4727cef9a8/includes/Hooks.php#291
  6. In the case of the search we should add some JavaScript that makes the access key also expand the search.

Sorry @bwang for adding all this unnecessary confusion. This looks like the right approach.

No worries, sorry I could have been more clear. Regarding the search, I did some more testing and I don't think JS is required. If we duplicate the search accesskey on the search button, using the access key will expand the search and focus on the input in one go. Plus accesskey commands vary depending on the browser and OS, which would be awful to try to reproduce in JS.

One weird thing I noticed is that in order for the search accesskey to work, it has to be reordered to go before the vue search in the DOM. Unsure why, might have to do with how browsers implement accesskey when multiple elements have the same accesskey?

bwang removed bwang as the assignee of this task.Mar 24 2022, 5:19 PM
bwang updated the task description. (Show Details)
bwang removed the point value 3 for this task.
bwang updated the task description. (Show Details)
ovasileva set the point value for this task to 3.Apr 5 2022, 5:19 PM

Change 779934 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/core@master] Add single-id to personal tools links data

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

Change 779935 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Rely on core to provide accesskey/title attributes, update search toggle location in DOM

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

Change 779934 merged by jenkins-bot:

[mediawiki/core@master] Add single-id to personal tools links data, factor out buildWatchlistData()

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

Change 779935 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Rely on core to provide accesskey/title attributes, update search toggle location in DOM

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

Edtadros subscribed.

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Monterey
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

✅ AC1: User page button should have an accesskey "." attribute

Screen Shot 2022-04-19 at 8.00.45 AM.png (546×1 px, 114 KB)

✅ AC2: Watchlist button should have an accesskey "l" attribute
Screen Shot 2022-04-19 at 8.02.29 AM.png (546×1 px, 129 KB)

✅ AC3: Search toggle button (shows up on smaller screen sizes) should have an accesskey "f" attribute
Screen Shot 2022-04-19 at 8.02.48 AM.png (546×1 px, 122 KB)

❓ AC4: All buttons above can be accessed using accesskeys
@Jdlrobson, I am unable to get the access keys to work. I'm assuming there is a setting in MacOS that allows this. Any ideas?

@Jdlrobson, I am unable to get the access keys to work. I'm assuming there is a setting in MacOS that allows this. Any ideas?

In Chrome for MacOS it should work with ctrl+option e.g. ctrl+option+i

I can confirm that these accesskeys works :)

On beta:

  • ctrl+option+. takes me to my userpage
  • ctrl+option+l to watchlist

Search toggle button (shows up on smaller screen sizes) should have an accesskey "f" attribute

This last point I'm not sure about. On larger screens,ctrl+option+f focuses the search input when it's visible. I don't see anything happen on smaller screens when I press ctrl+option+f though, (on beta). @bwang does this work for you?

Gotcha, I did some more testing and it seems ctrl+option+f works on small screen sizes on Safari, kind of works for FF (it focuses the search button) and doesn't work for Chrome. My bad, I was testing on Safari during testing and I missed this. Accesskeys are known for poor browser support and inconsistent implementation anyway, so personally I feel like this is good enough. Espeically considering the accesskey for the search bar on larger screen sizes always works and we have updated the collapsible search icon for parity at smaller screen sizes.

Agreed. This is good enough.

Alright, signing this one off then :)