WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#35051 closed defect (bug) (fixed)

Remove title attributes: the Network Themes list table

Reported by: afercia Owned by: afercia
Milestone: 4.5 Priority: normal
Severity: normal Version: 4.4
Component: Networks and Sites Keywords: has-screenshots, has-patch, commit, title-attribute
Focuses: ui, accessibility, multisite Cc:
PR Number:

Description

See related #24766 and all the following tickets about title attributes.

All the title attributes on the links in the Network Themes list table basically repeat the same information already conveyed by the link text. I'd say to just remove them all. See screenshot:

https://cldup.com/FZrpqLtAXB.png

For accessibility and consistency I'd suggest to add an aria-label attribute to expand the links text, as done for example in the Plugins list table, e.g.:

<a ... aria-label="Enable Intergalactic">Enable</a>

Attachments (3)

35051.patch (3.9 KB) - added by afercia 4 years ago.
35051.2.patch (4.1 KB) - added by afercia 4 years ago.
35051.3.patch (4.8 KB) - added by SergeyBiryukov 4 years ago.

Download all attachments as: .zip

Change History (10)

This ticket was mentioned in Slack in #core-multisite by afercia. View the logs.


4 years ago

@afercia
4 years ago

#2 @afercia
4 years ago

  • Keywords has-patch added
  • Owner set to afercia
  • Status changed from new to assigned

First pass. Would need some feedback because, as far as I see, the Themes list table is now used only in the main site and no more used in the network single sites so I guess this kind of checks for the row action links text is now pointless? Or maybe I'm missing something :)
( $this->is_site_themes ? __( 'Enable' ) : __( 'Network Enable' ) )

@afercia
4 years ago

#3 @afercia
4 years ago

  • Keywords commit added

Refreshed patch. As @jeremyfelt pointed out, the check for $this->is_site_themes is still necessary when editing sites in the "Themes" tab. Thus, also the aria-label should provide a double text to be used for "Enable" or "Network Enable".
Also, uses $theme->display( 'Name' ) instead of $theme->get( 'Name' ).

Would appreciate a final review :)

#4 @SergeyBiryukov
4 years ago

One issue with ternary operators is that translator comment will only be parsed for the first string, but not for the second:

/* translators: %s: theme name */ 
sprintf( ( $this->is_site_themes ? __( 'Enable %s' ) : __( 'Network Enable %s' ) ), $theme->display( 'Name' ) )

35051.3.patch replaces string concatenation with add_query_arg() and sprintf(), which fixes the issue and makes these lines more readable.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#5 @afercia
4 years ago

Thanks @SergeyBiryukov definitely more readable and elegant :)

#6 @afercia
4 years ago

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

In 35924:

Accessibility: Remove title attributes from the Network Themes list table.

Title attributes in the "Themes" screen and in the "Edit Site" screen Themes tab
are now replaced with aria-label attributes. Also, replaces string
concatenation with add_query_arg() and sprintf() to allow translator
comments to be properly parsed and for better code readability.

Props SergeyBiryukov, afercia.
Fixes #35051.

#7 @afercia
3 years ago

  • Keywords title-attribute added
Note: See TracTickets for help on using tickets.