WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 3 weeks ago

#32399 closed enhancement (fixed)

List table: Lists of items should inform users about the "current view" being displayed

Reported by: afercia Owned by: afercia
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: Administration Keywords: aria-current has-patch
Focuses: ui, accessibility Cc:

Description

See related #32147

Several screens in the admin present items in a list and you can filter these lists choosing which "view" you want, whether they're Posts in a List Table or lists generated by other components (see Themes and Plugins), they all have some "views" links.

When you select a view, there's no clear, semantic, indication in the markup about the current view being displayed. There's just some bold text.

Users, especially the ones who use assistive technologies, would need some feedback about which kind of "view" they're currently in. For example, something like you're currently viewing the Scheduled Posts. Of course this is the semantics that should be conveyed, not the best possible phrasing.

There are ways to do this but the main issue would be about translatable strings, as pointed out in #32147.

Any thoughts and patches more than welcome :)

Screenshot for reference:

https://cldup.com/KE9MdelYnE.png

Attachments (3)

32399.patch (824 bytes) - added by joedolson 2 years ago.
Adds context to heading for view and page in post views.
32399.diff (11.7 KB) - added by afercia 4 weeks ago.
32399.2.diff (11.5 KB) - added by afercia 3 weeks ago.

Download all attachments as: .zip

Change History (40)

#1 follow-up: @joedolson
2 years ago

It seems to me like the data should be pushed into the heading for the page. E.g., click 'Scheduled', heading changes from 'Posts' to 'Scheduled Posts'. That would be simple, uncluttered, and semantic.

#2 @joedolson
2 years ago

Attached patch only applies to post & post types, as an example of getting that data into the heading.

It also shows up an issue with the language used for post statuses; they aren't consistent - e.g. "Scheduled" vs. "Trash". This is evident when used in a construction: Scheduled Posts is totally logical, but "Trash Posts" doesn't make so much sense.

@joedolson
2 years ago

Adds context to heading for view and page in post views.

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


2 years ago

#4 @joedolson
2 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.3
  • Summary changed from Lists of items should inform users about the "current view" being displayed to List table: Lists of items should inform users about the "current view" being displayed

#5 in reply to: ↑ 1 @afercia
2 years ago

Replying to joedolson:

It seems to me like the data should be pushed into the heading for the page.

That would be ideal. Especially now that the main heading is going to be changed in a H1 see #31650
For Posts (Custom Post Types, Pages) we could simply extend the labels provided by register_post_status() in order to have fully translatable strings e.g.:

register_post_status( 'publish', array(
	'label'       => _x( 'Published', 'post' ),
	'h1'          => __( 'Published Posts' ),
	...

Other screens, e.g. Users, Themes, Plugins, would require something different but definitely worth considering this and I'd highly recommend contextual, descriptive, headings.

#6 @joedolson
2 years ago

I don't think we want to push complex strings into the strings registered in a post type; I'd rather see linguistic ur-types, such as:

register_post_status( 'publish', array(
	'label'     => _x( 'Published', 'post' ),
	'adjective' => __( 'Published' ),
	'verb'      => __( 'Publish' ),
	...

This allows us to create any needed string combination, rather than making something that's *only* usable in specific contexts.

#7 @joedolson
2 years ago

We might be able to make a case for adding this information to the status messages block.

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


2 years ago

#9 @johnbillion
2 years ago

  • Keywords needs-patch added; has-patch removed
  • Type changed from defect (bug) to enhancement
  • Version 4.2 deleted

Adding stand-alone adjectives/verbs/etc as individual labels is a non-starter because it's impossible to localise these.

We can get around it by using phrases which aren't specific to the post type, such as "Status: Published" instead of "Published Posts". This is also more inline with the sort of phrases you'd expect to see when filtering lists of records.

#10 @afercia
2 years ago

Yup or see an alternate approach suggested by @ocean90 here:
https://wordpress.slack.com/archives/accessibility/p1433185516000481

#11 @obenland
2 years ago

Beta is going to hit in two weeks. Does this ticket have a realistic chance to get in until then?

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


2 years ago

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


2 years ago

#14 @obenland
2 years ago

  • Milestone changed from 4.3 to Future Release

Not a lot of movement recently, let's look at it again in a future release.

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


2 years ago

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


11 months ago

#17 @joedolson
11 months ago

  • Milestone changed from Future Release to 4.8
  • Owner set to joedolson
  • Status changed from new to accepted

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


6 months ago

#19 @jbpaul17
6 months ago

  • Milestone changed from 4.8 to 4.8.1

Punting to 4.8.1 as there doesn't appear to be a patch coming shortly. If one is ready, then we can pull back into 4.8.

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


5 months ago

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


3 months ago

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


3 months ago

#23 @afercia
3 months ago

  • Milestone changed from 4.8.1 to 4.9

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


3 months ago

#25 @afercia
3 months ago

  • Keywords aria-current added

#26 @joedolson
3 months ago

Note for reference: use aria-current to indicate active view.

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


3 months ago

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


4 weeks ago

#29 @joedolson
4 weeks ago

  • Owner joedolson deleted
  • Status changed from accepted to assigned

@afercia
4 weeks ago

#30 @afercia
4 weeks ago

  • Keywords has-patch added; needs-patch removed

32399.diff is a first pass to add aria-current="page" to all the "views" link in the list tables and in the theme.js "views" link. Some testing would be nice :)

#31 @afercia
4 weeks ago

  • Owner set to afercia

#32 @flixos90
3 weeks ago

32399.diff looks good to me.

I'm a little worried about the code there getting more and more complex with more of these (necessary) changes going in. I wonder whether we should introduce a general function to deal with creating the arrays of links that are so commonly used in the backend.
It could be a function get_admin_status_links( $link_data ) (let's find a better name though) where $link_data is an associative array where each key is the link identifier and each value is an array containing data for the actual link element (href and label at least). The logic to transform that data into a link element string could be handled in that function, and changes like those from this ticket would be much simpler as they'd only need to be changed in one place.

Such a function should be introduced in a separate ticket though. Depending on how urgent this fix here is, we could do it one or the other way. It would be a bit easier to have the function first, but it's more initial work to set this up, so if we need to have this here in 4.9, we can always still create the function later.

#33 @afercia
3 weeks ago

a general function to deal with creating the arrays of links that are so commonly used in the backend ... The logic to transform that data into a link element string could be handled in that function

@flixos90 thanks. I'd agree that decoupling what get_views() does (which needs to be different depending on the table, since it checks for post status, or user role, or current tab, etc...) with the generation of the links markup could help making the code a bit more clean. The users list table is a special case though, my concern is it would require some refactoring.

I'd agree the propose function should be evaluated in a separate ticket.

Note: maybe 32399.diff could be simplified a bit to avoid some ternarys but at the cost of a reduced code readability.

@afercia
3 weeks ago

#34 @afercia
3 weeks ago

32399.2.diff simplifies things a bit. Also, in the Posts list table, there's the need to exlude other edit links used for example for the post formats.

#35 @flixos90
3 weeks ago

@afercia I like 32399.2.diff better, since the current class and the aria-current attribute really need to always be set at the same time anyway, so I think the new patch has less code churn.

Feel free to open another ticket for the outsourcing part later and ping me, I'd be happy to work on that.

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


3 weeks ago

#37 @afercia
3 weeks ago

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

In 41683:

Accessibility: List Tables: use aria-current for the views current link.

The aria-current attribute is a simple, effective way to help assistive
technologies users orientate themselves within a list of items. Continues the
introduction in core of the aria-current attribute after [41359] and [41371].

Props joedolson, flixos90, afercia.
Fixes #32399.

Note: See TracTickets for help on using tickets.