Page MenuHomePhabricator

NewImpact: Table list component is not keyboard accessible
Closed, ResolvedPublic

Description

We should be able to tab through entries on the table list component:

image.png (1×1 px, 208 KB)

Currently, you can tab to the "info" icons and open/close the dialogs.

Acceptance Criteria

  1. Should be able to
    1. tab through each title in the table (thumbnail and title are highlighted together? else, just tab to the title and not the thumbnail)
    2. tab to the page views sparkline, and be able to open the page views URL
    3. tab to the clock icon, and upon pressing enter, it should open a tooltip with the explanatory text
Completion checklist

Functionality

  • The patches have been code reviewed and merged
  • The task passes its acceptance criteria

Engineering

  • There are existing and passing unit/integration tests
  • Tests for every involved patch should pass
  • Coverage for every involved project should have improved or stayed the same

Design & QA

  • If the task is UX/Design related: it must be reviewed and approved by the UX/Design team
  • Must be reviewed and approved by Quality Assurance.

Documentation

  • Related and updated documentation done where necessary

Event Timeline

kostajh triaged this task as Medium priority.Nov 10 2022, 12:02 PM
kostajh created this task.
kostajh added a project: Accessibility.
kostajh edited projects, added Growth-Team (Current Sprint); removed Growth-Team.

Could it be that we’re falling into this Mac OS + Firefox scenario? Testing in Chrome or setting accessibility.tabfocus to 7 on Firefox seem to work well.

Regarding skipping the thumbnail as a focusable element I guess it's possible to implement but not sure if it's worth or desired. With the current implementation using the keyboard would have the same interaction as using a mouse or a touch device. I don't think it's needed to change anything right now for accessibility reasons unless we want to have a different accessible text for the article thumbnail than "Go to article". But since we don't have a thumbnail caption to use and the article title is right beside I think it's usable enough.

Could it be that we’re falling into this Mac OS + Firefox scenario?

Ah, I see. OK, macOS seems implicated. But even after switching "Keyboard navigation" on, I still can't get tab focus for items in the article table list with Safari.

image.png (718×964 px, 94 KB)

safaritab.gif (928×1 px, 494 KB)

I had to check also the option Press Tab to highlight each item on a web page on Safari's Preferences > Advanced

Screenshot 2022-11-25 at 17.37.16.png (902×1 px, 230 KB)

then the behavior is like in Chrome or Firefox (see attachement). Could it be this in your tests as well?
keyboard_safari_impact.gif (1×626 px, 257 KB)

Mind that if testing this in betalabs (like in the gif) while T323838: pageviewsUrl is missing in the server-side exported user impact data is not resolved will not allow to navigate to the pageviews link because the anchor does not have a href attribute.

I had to check also the option Press Tab to highlight each item on a web page on Safari's Preferences > Advanced

Screenshot 2022-11-25 at 17.37.16.png (902×1 px, 230 KB)

then the behavior is like in Chrome or Firefox (see attachement). Could it be this in your tests as well?
keyboard_safari_impact.gif (1×626 px, 257 KB)

Aha, right. When I check that, it works in Safari. Thanks!

Mind that if testing this in betalabs (like in the gif) while T323838: pageviewsUrl is missing in the server-side exported user impact data is not resolved will not allow to navigate to the pageviews link because the anchor does not have a href attribute.

Ack. T323838 should be resolved now.

I think the last item here is what @RHo commented on in T313274#8424377; one should be able to tab to the clock icon, and it should also open a tooltip with the explanatory text.

I think the last item here is what @RHo commented on in T313274#8424377; one should be able to tab to the clock icon, and it should also open a tooltip with the explanatory text.

Yes please! Re-posting screenshots over here for convenience:
The clock icon tooltip should appear and behave in the same way as before in the original impact module, which is a pop-over info text that is shown either on hover or via keyboard select:

Expected:
image.png (370×766 px, 55 KB)
Actual:
image.png (172×706 px, 34 KB)

Change 862223 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@master] NewImpact: show popover instead of tooltip in the clock icon

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

Pushed a patch for this. Since our "popover" component (CInfoBox/CPopper) does not have yet the clipping arrow the informational text just displays above the clock icon. Hopefully that's fine for now @RHo ?

Screenshot 2022-11-30 at 10.55.43.png (1×854 px, 145 KB)

Test wiki created on Patch demo by RHo (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/21a27f92de/w

Pushed a patch for this. Since our "popover" component (CInfoBox/CPopper) does not have yet the clipping arrow the informational text just displays above the clock icon. Hopefully that's fine for now @RHo ?

Screenshot 2022-11-30 at 10.55.43.png (1×854 px, 145 KB)

Hi @Sgs - I tried to create a patchdemo to have a quick look at this but am setill seeing the old impact module (I tried appending ?new-impact=1 but nothing happened):

image.png (546×774 px, 59 KB)

It looks mainly fine for now, but had wanted to confirm that it appears on mouse hover over the clock as well? And secondly, if we should create a separate task to make the styling changes later (adding in the pointer arrow)?

Pushed a patch for this. Since our "popover" component (CInfoBox/CPopper) does not have yet the clipping arrow the informational text just displays above the clock icon. Hopefully that's fine for now @RHo ?

Screenshot 2022-11-30 at 10.55.43.png (1×854 px, 145 KB)

Hi @Sgs - I tried to create a patchdemo to have a quick look at this but am setill seeing the old impact module (I tried appending ?new-impact=1 but nothing happened):

image.png (546×774 px, 59 KB)

It looks mainly fine for now, but had wanted to confirm that it appears on mouse hover over the clock as well? And secondly, if we should create a separate task to make the styling changes later (adding in the pointer arrow)?

On Special:Impact, at least, the ?new-impact=1 query seems to work:

https://patchdemo.wmflabs.org/wikis/21a27f92de/w/index.php?title=Special:Impact/Patch_Demo&new-impact=1

Pushed a patch for this. Since our "popover" component (CInfoBox/CPopper) does not have yet the clipping arrow the informational text just displays above the clock icon. Hopefully that's fine for now @RHo ?

Screenshot 2022-11-30 at 10.55.43.png (1×854 px, 145 KB)

Hi @Sgs - I tried to create a patchdemo to have a quick look at this but am setill seeing the old impact module (I tried appending ?new-impact=1 but nothing happened):

image.png (546×774 px, 59 KB)

It looks mainly fine for now, but had wanted to confirm that it appears on mouse hover over the clock as well? And secondly, if we should create a separate task to make the styling changes later (adding in the pointer arrow)?

On Special:Impact, at least, the ?new-impact=1 query seems to work:

https://patchdemo.wmflabs.org/wikis/21a27f92de/w/index.php?title=Special:Impact/Patch_Demo&new-impact=1

Although, it's not really that useful for testing the clock change, because we export static data instead of allowing it to be dynamically generated. Let me see about changing that.

Pushed a patch for this. Since our "popover" component (CInfoBox/CPopper) does not have yet the clipping arrow the informational text just displays above the clock icon. Hopefully that's fine for now @RHo ?

Screenshot 2022-11-30 at 10.55.43.png (1×854 px, 145 KB)

Hi @Sgs - I tried to create a patchdemo to have a quick look at this but am setill seeing the old impact module (I tried appending ?new-impact=1 but nothing happened):

image.png (546×774 px, 59 KB)

It looks mainly fine for now, but had wanted to confirm that it appears on mouse hover over the clock as well? And secondly, if we should create a separate task to make the styling changes later (adding in the pointer arrow)?

On Special:Impact, at least, the ?new-impact=1 query seems to work:

https://patchdemo.wmflabs.org/wikis/21a27f92de/w/index.php?title=Special:Impact/Patch_Demo&new-impact=1

Although, it's not really that useful for testing the clock change, because we export static data instead of allowing it to be dynamically generated. Let me see about changing that.

I made this patch to change how the impact services are overridden (or rather, fixing it so they are not overridden) in Patch Demo.

Change 862350 had a related patch set uploaded (by Kosta Harlan; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@wmf/1.40.0-wmf.12] NewImpact: show popover instead of tooltip in the clock icon

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

Change 862223 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] NewImpact: show popover instead of tooltip in the clock icon

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

Change 862350 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@wmf/1.40.0-wmf.12] NewImpact: show popover instead of tooltip in the clock icon

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

Etonkovidova added a subscriber: Etonkovidova.

Checked in wmf.14 - works as expected:

Screen Shot 2022-12-15 at 12.17.23 PM.png (1×730 px, 127 KB)
Screen Shot 2022-12-15 at 12.29.29 PM.png (690×816 px, 80 KB)