WordPress.org

Make WordPress Core

Opened 18 months ago

Closed 7 months ago

#43376 closed defect (bug) (fixed)

Semantic elements for non-link links: class-wp-comments-list-table.php

Reported by: audrasjb Owned by:
Milestone: 5.1 Priority: normal
Severity: normal Version:
Component: Comments Keywords: semantic-buttons has-patch
Focuses: accessibility, javascript Cc:

Description

The "Quick Edit" links in comment list should be changed to buttons (or ARIA role button).

Related: #38677

Attachments (5)

43376_aria_role.diff (1.1 KB) - added by audrasjb 18 months ago.
Version 1: add button aria role to links with "#" anchor.
43376_button_element.diff (1.1 KB) - added by audrasjb 18 months ago.
Version 2: Replace link with button HTML element (with button-link class)
43376_button_element.2.diff (2.1 KB) - added by audrasjb 18 months ago.
Version 2 enhancement: adds role and aria-expandedattribute and fires it on click/cancel
43376_button_element.3.diff (2.1 KB) - added by audrasjb 18 months ago.
Fix error in previous patch: replaces role with type attribute…
43376.diff (3.5 KB) - added by afercia 18 months ago.

Download all attachments as: .zip

Change History (26)

@audrasjb
18 months ago

Version 1: add button aria role to links with "#" anchor.

@audrasjb
18 months ago

Version 2: Replace link with button HTML element (with button-link class)

#1 @audrasjb
18 months ago

  • Keywords has-patch added

Hi,

I uploaded two patches:

  • Version 1: 43376_aria_role.diff adds role="button" attribute to the links
  • Version 2: 43376_button_element.diff replaces a tag with button tag with the button-link class for similar rendering

I only managed links with href="#" attributes. The other links don't need button element because the href is not empty.

Both of the patches are tested and seems good to me.

Cheers,
Jb

Last edited 18 months ago by audrasjb (previous) (diff)

#2 @afercia
18 months ago

  • Keywords needs-refresh added
  • Version trunk deleted

@audrasjb thanks for the patch. I'd tend to think the version with tbe button is preferable. Please notice the button needs a type="button" attribute and would also benefit from an aria-expanded attribute to communicate to assistive technologies the expanded / collapsed status of the form (actually, more the collapsed status since when it opens, focus is moved and the button disappears). This would bring it in parity with what has been done for the Posts list Quick Edit in #38677 / [42725].

#3 @audrasjb
18 months ago

Great, thanks.
I've seen your final patch for #38677 and I'll send a new patch soon.

@audrasjb
18 months ago

Version 2 enhancement: adds role and aria-expandedattribute and fires it on click/cancel

#4 @audrasjb
18 months ago

  • Keywords needs-refresh removed

43376_button_element.2.diff works fine in my local install.
I tried to follow the path of #43382 for consistency @afercia.

@audrasjb
18 months ago

Fix error in previous patch: replaces role with type attribute…

@afercia
18 months ago

#5 @afercia
18 months ago

  • Focuses javascript added
  • Milestone changed from Awaiting Review to 5.0
  • Owner set to afercia
  • Status changed from new to assigned

The comments screen works in a slightly different way compared to the Posts and Terms screens. Changing the Quick Edit link to a button will change the Reply link too. That's fine because both work in the same way: they're not available when JavaScript is off and they open their own form. The JavaScript part needs to take into account both cases.

In 43376.diff :

  • the Reply and Quick Edit links are now buttons
  • they have an aria-expanded attribute, dynamically updated based on the form collapsed / expanded state
  • when closing the Reply or Quick Edit forms, focus is moved back to the proper button
  • I've slightly refactored the JS part to use more meaningful variable names: commentRow and replyRow
  • removes e.preventDefault() which is no longer necessary, as the links are not buttons with an attribute type="button", i.e.: there's no default action to prevent

Tested and looks good to me. Some more testing welcome!

#6 @afercia
18 months ago

Will open a new ticket for further improvements.

#8 @audrasjb
18 months ago

You won @afercia :P
I tested 43376.diff and it looks great to me.

#9 @afercia
18 months ago

There was already #38676 for the comments screen, not a big deal we can use this new ticket. Props to @Cheffeid for the patch on the old ticket.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


18 months ago

#11 @afercia
18 months ago

#38676 was marked as a duplicate.

#12 @afercia
18 months ago

  • Keywords semantic-buttons added

#13 @afercia
18 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 42767:

Accessibility: Change the comments "Quick Edit" and "Reply" links to buttons.

For better accessibility and semantics, user interface controls that perform an
action should be buttons. Links should exclusively be used for navigation.

Props Cheffeid, audrasjb, afercia.
See #43382, #38677.
Fixes #43376.

#14 @johnbillion
10 months ago

  • Keywords has-patch removed
  • Milestone changed from 5.0 to 5.0.1
  • Resolution fixed deleted
  • Status changed from closed to reopened

#15 @afercia
10 months ago

  • Owner afercia deleted
  • Status changed from reopened to assigned

#16 @pento
8 months ago

  • Milestone changed from 5.0.1 to 5.0.2

#17 @pento
8 months ago

  • Milestone changed from 5.0.2 to 5.0.3

#18 @audrasjb
8 months ago

  • Keywords has-patch added

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


8 months ago

#20 @audrasjb
8 months ago

  • Milestone changed from 5.0.3 to 5.1

Hi,

Per today's bug scrub, let's address this ticket in the next major, 5.1, scheduled in February.

#21 @SergeyBiryukov
7 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed in [42767], doesn't look like any additional changes are needed.

Note: See TracTickets for help on using tickets.