WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#34791 closed defect (bug) (fixed)

Comments screen: can't focus the row action links anymore

Reported by: afercia Owned by: jorbin
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.4
Component: Comments Keywords: has-screenshots has-patch needs-testing commit
Focuses: ui, accessibility Cc:

Description

Now that the comment date has being moved out of the comment content column and is within its own, new, column see [34504], when tabbing through focusable elements using just the keyboard there's no way to make the row action links appear.

Previously, the date was the element that, when focused, made the row actions appear:

https://cldup.com/dUGEtthDaV.png

Now, there's no focusable element unless the comment is a reply (in this case there's the original comment author link that helps).

https://cldup.com/kOu9P084nQ.png

Keyboard users (including screen reader users) won't have any chance to activate those links so this is an accessibility regression in trunk.

By the way, this is not the only case where the row action links show/hide is problematic for accessibility. For example, in the Posts Trash there's no focusable element to make the links appear (the post title is not linked), I guess this has always worked this way.

Also, when tabbing backwards, the links appear only *after* users have the chance to focus them, they're basically "skipped" when navigating backwards, I guess this has always worked this way too.

The accessibility testing group members often reported having troubles with the row action links, basically they can't get why sometimes they're available to their screen readers and sometimes not. Maybe, now we have a more clear view why they reported so :)

Thinking a solid fix would be finding a new way to hide/show these links (have something in mind) but also wondering if this could be a nice proposal for the WCUS Contributor Day. Any thoughts more than welcome.

Attachments (3)

34791.patch (611 bytes) - added by azaozz 5 years ago.
34791.2.patch (636 bytes) - added by afercia 5 years ago.
34791.3.patch (636 bytes) - added by afercia 5 years ago.

Download all attachments as: .zip

Change History (16)

@azaozz
5 years ago

#1 @azaozz
5 years ago

This is a regression that affects many users. As @afercia mentions in the ticket the best way to fix these is to change the method we use to hide the row actions.

In 34791.patch: restrict the parent element's width and height to 1px instead of setting it to visibility: hidden. That way for the browser the row actions links are always visible and can be focused. This resolves all of the problems, on the post trash screen, when tabbing backwards, etc.

Tested in Firefox, Chrome, IE11 and iOS Safari (not affected as these links are always visible). More testing would be nice.

#2 @azaozz
5 years ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Milestone changed from Awaiting Review to 4.4

#3 @afercia
5 years ago

@azaozz thanks :) Thinking one of the problems to solve here is that the row actions "row" still needs to take space when is invisible. If not, the table rows will have a smaller height and when hovered or focused the following rows will be suddenly "pushed down":

https://cldup.com/jnPeYwimDj.png

Maybe the only CSS property we can use to make the row actions take space but be "off screen" is position: relative. New patch incoming.

@afercia
5 years ago

#4 @afercia
5 years ago

New patch with the position: relative approach. More testing would be greatly appreciated. Would be great to test also with RTL languages. Places where to test:

  • All the list tables with row actions: Posts, Pages, Media, Comments, Users, (etc.?) check also the list tables in Network installations
  • Dashboard > Activity > Comments
  • Network > Users (last column: "Sites")

From an accessibility point of view this works, we should just be aware that suddenly many admin screens will have lots of "new" links reported by assistive technologies.

#5 @azaozz
5 years ago

  • Keywords commit added

34791.2.patch works well. The only thing I'd change is make the offset even larger: -9999em instead of px.

Was a bit concerned that position: relative may introduce scrollbar in some old browsers, but seems to work well. Tested in Firefox, Chrome, iOS Safari, IE11, IE8, and RTL. Some more testing would be nice.

@afercia
5 years ago

#6 @afercia
5 years ago

Refreshed patch uses em instead of px. Agree with @azaozz some more testing would be nice :)

#7 @afercia
5 years ago

Forgot to mention I've also added .no-js .row-actions to make sure all row action links are visible when JavaScript is off. That's because when JS is off, row action links may still work when hovering them with a mouse but definitely wouldn't work when focusing them using the keyboard.

#8 @perezlabs
5 years ago

Re-tested in Chrome, Firefox, Safari, IE8, IE9, IE10 through the web app browserstack and works well.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


5 years ago

#10 @wonderboymusic
5 years ago

+1 from me

#11 @jorbin
5 years ago

  • Owner set to jorbin
  • Resolution set to fixed
  • Status changed from new to closed

In 35771:

Make comment screen row actions focusable

In [34504], tabbing through row actions on comments that lacked links was broken. This restores the desired behavior and ensures that the row actions can be seen by no-js users.

Second Permanent Committer sign off was by WonderBoyMusic

See #15520
Fixes #34791
Props afercia, azaozz

#12 in reply to: ↑ description @Asajoarder
5 years ago

  • Severity changed from normal to blocker
  • Type changed from defect (bug) to enhancement

Replying to afercia:

Now that the comment date has being moved out of the comment content column and is within its own, new, column see [34504], when tabbing through focusable elements using just the keyboard there's no way to make the row action links appear.

Previously, the date was the element that, when focused, made the row actions appear:

https://cldup.com/dUGEtthDaV.png

Now, there's no focusable element unless the comment is a reply (in this case there's the original comment author link that helps).

https://cldup.com/kOu9P084nQ.png

Keyboard users (including screen reader users) won't have any chance to activate those links so this is an accessibility regression in trunk.

By the way, this is not the only case where the row action links show/hide is problematic for accessibility. For example, in the Posts Trash there's no focusable element to make the links appear (the post title is not linked), I guess this has always worked this way.

Also, when tabbing backwards, the links appear only *after* users have the chance to focus them, they're basically "skipped" when navigating backwards, I guess this has always worked this way too.

The accessibility testing group members often reported having troubles with the row action links, basically they can't get why sometimes they're available to their screen readers and sometimes not. Maybe, now we have a more clear view why they reported so :)

Thinking a solid fix would be finding a new way to hide/show these links (have something in mind) but also wondering if this could be a nice proposal for the WCUS Contributor Day. Any thoughts more than welcome.

#13 @johnbillion
5 years ago

  • Severity changed from blocker to normal
  • Type changed from enhancement to defect (bug)
Note: See TracTickets for help on using tickets.