WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 5 months ago

#26167 reopened enhancement

Plugin activation links need to contain plugin name and the "Plugin" column should be marked as row header

Reported by: grahamarmfield Owned by:
Milestone: Future Release Priority: low
Severity: normal Version: 3.8
Component: Plugins Keywords: has-patch
Focuses: accessibility Cc:

Description

In the plugins table within admin, the Activate, Deactivate, Edit and Delete links need to contain the plugin name they relate to. This is to provide context for screen reader users.

If required, the plugin name portion of the link can be hidden from sighted users with the use of the 'screen-reader-text' CSS class. An example:

<a href="">Activate<span class="screen-reader-text"> Akismet</span></a>

This shouldn't clash with internationalisation processing since the plugin name won't change in different languages (I believe).

Attachments (5)

26167.patch (5.0 KB) - added by SergeyBiryukov 17 months ago.
26167-1.patch (6.2 KB) - added by bramd 17 months ago.
This patch depends on 25459-functions.patch (see #25459). It solves the internationalization issues by using a placeholder for the plugin name and making this string translatable. I've also sneaked in another little change that marks the column containing the plugin name as row header (<th>) instead of the checkbox which is more appropriate. Probably the ticket title/description should be updated to reflect this change.
26167-2.patch (6.3 KB) - added by bramd 12 months ago.
New patch that uses aria-label for an extended label for activate/deactivate/delete links.
26167.3.patch (6.3 KB) - added by rianrietveld 6 months ago.
26167.4.patch (5.9 KB) - added by rianrietveld 6 months ago.
refresh of 26167.2 also removing the title attributes from the link

Download all attachments as: .zip

Change History (26)

@SergeyBiryukov17 months ago

comment:1 @SergeyBiryukov17 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.8

This shouldn't clash with internationalisation processing since the plugin name won't change in different languages (I believe).

Actually, plugin headers (including title) are translatable, but I don't think that would cause an issue here either.

comment:2 follow-up: @ocean9017 months ago

  • Keywords needs-patch added; has-patch removed

I think this needs some more work since there are some translation issues.

First: This needs probably a swap in RTL.
Second: This needs some context since in German "Activate" is currently "Aktivieren". So a screen reader would read "Aktivieren Akismet", that's wrong. It must be "Aktiviere Akismet" in this context - or "Akismet aktivieren".

comment:3 @dd3217 months ago

Activate %s should probably be used with the span and translated plugin name injected.

comment:4 @MikeLittle17 months ago

It seems from the German example that the verb needs to be translatable in two different contexts. With the plugin name (for screen readers), and without the plugin name for visual readers. That is, with the name is it "verb object", without it is "command".
That cannot be handled with a single translation.

Suggested alternates (assume plugin name is wrapped in the span):
Activate: Plugin name
Activate (plugin name)

comment:5 @johnbillion17 months ago

  • Type changed from defect (bug) to enhancement

Related: #25459

comment:6 @matt17 months ago

  • Priority changed from normal to low

comment:7 in reply to: ↑ 2 @daveshine17 months ago

Replying to ocean90:

I think this needs some more work since there are some translation issues.

First: This needs probably a swap in RTL.
Second: This needs some context since in German "Activate" is currently "Aktivieren". So a screen reader would read "Aktivieren Akismet", that's wrong. It must be "Aktiviere Akismet" in this context - or "Akismet aktivieren".

+1 to this!
This patch needs more testing and I doubt that it is ready for 3.8 already. We should test more and maybe include with 3.9. The translation issues are very important as it directly effects the user experience. The current view should not be changed for sighting users, IMHO.

My suggestion is, to use @MikeLittle's approach with translation context here, maybe in combination with sprintf() and string placeholders.

comment:8 @daveshine17 months ago

  • Cc daveshine added

comment:9 @ocean9017 months ago

  • Milestone changed from 3.8 to Future Release

@bramd17 months ago

This patch depends on 25459-functions.patch (see #25459). It solves the internationalization issues by using a placeholder for the plugin name and making this string translatable. I've also sneaked in another little change that marks the column containing the plugin name as row header (<th>) instead of the checkbox which is more appropriate. Probably the ticket title/description should be updated to reflect this change.

comment:10 @bramd17 months ago

  • Cc bramd added
  • Keywords has-patch added; needs-patch removed
  • Status changed from new to closed
  • Summary changed from Plugin activation links need to contain plugin name to Plugin activation links need to contain plugin name and the "Plugin" column should be marked as row header

comment:11 @bramd17 months ago

  • Status changed from closed to reopened

Whoops, didn't want to close the ticket...

comment:12 @nacin15 months ago

  • Component changed from Accessibility to Plugins
  • Focuses accessibility added

@bramd12 months ago

New patch that uses aria-label for an extended label for activate/deactivate/delete links.

comment:13 @GrahamArmfield9 months ago

I think this patch may need refreshing, I don't seem to be able to apply it.

@rianrietveld6 months ago

@rianrietveld6 months ago

refresh of 26167.2 also removing the title attributes from the link

comment:14 @rianrietveld6 months ago

  • Keywords needs-testing added

Added 2 patches:

26167.3: refresh of 26167.2 by @bramd
26167.4: refresh of 26167.2 also removing the title attributes from the link

comment:15 @slackbot6 months ago

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

comment:16 @arush6 months ago

I've tested this with Jaws latest release, and NVDA latest release and this patch works perfectly, even when tabbing through the screen. Browsers tested, IE10, IE11 FF latest release.

comment:17 @slackbot6 months ago

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

comment:18 @arush6 months ago

  • Keywords needs-testing removed

I think this patch is the perfect way to handle the plugin activation/edit/delete links on the installed plugins screen.

comment:19 @afercia5 months ago

Tested patch 26167.4, please check. Seems it breaks the color highlighting (background and left border) for active plugins and updates, inverting a th with a td. Also, tested with Firefox + NVDA, since the table cell with the plugin name and row actions is not a th, it will report wrong information about the plugin. Basically, the table semantics is broken.
The cell with the checkbox should still be a th and the one with plugin name and row actions still be a td.

comment:20 follow-up: @afercia5 months ago

Double checked and yes, I definitely agree with @bramd:

I've also sneaked in another little change that marks the column (cell) containing the plugin name as row header (<th>) instead of the checkbox which is more appropriate.

Patch n. 1 gets it right, the issue is in the following patches: the attribute scope="row" was moved back from the th to the td which is semantically incorrect and also not valid HTML. But I definitely agree: the relevant information here is the plugin name, so that should be used as row header. I guess the CSS issue about row colors can be fixed.

One more thing worries me here and in all the other similar tables: the "Select All" checkbox used as column header. This way, for each checkbox in each row, you will hear something like:

row 2 Select All column 1
Select {plugin name} check box not checked

which provides a wrong information, you're not going to "Select All" but to select just that single plugin.
I would propose to don't use the "Select All" as column header.

At this point, I'm wondering if moving the whole "checkboxes" column as last column in the table would be beneficial. Any thoughts more than welcome.

comment:21 in reply to: ↑ 20 @arush5 months ago

Replying to afercia:

Double checked and yes, I definitely agree with @bramd:

I've also sneaked in another little change that marks the column (cell) containing the plugin name as row header (<th>) instead of the checkbox which is more appropriate.

Patch n. 1 gets it right, the issue is in the following patches: the attribute scope="row" was moved back from the th to the td which is semantically incorrect and also not valid HTML. But I definitely agree: the relevant information here is the plugin name, so that should be used as row header. I guess the CSS issue about row colors can be fixed.

One more thing worries me here and in all the other similar tables: the "Select All" checkbox used as column header. This way, for each checkbox in each row, you will hear something like:

row 2 Select All column 1
Select {plugin name} check box not checked

which provides a wrong information, you're not going to "Select All" but to select just that single plugin.
I would propose to don't use the "Select All" as column header.

At this point, I'm wondering if moving the whole "checkboxes" column as last column in the table would be beneficial. Any thoughts more than welcome.

I think moving the checkboxes to the last column would be beneficial.

Note: See TracTickets for help on using tickets.