Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#31722 closed defect (bug) (fixed)

Shiny Updates: the aria-label should be updated to reflect the current status

Reported by: afercia's profile afercia Owned by: afercia's profile afercia
Milestone: 4.2 Priority: normal
Severity: major Version: 4.2
Component: Upgrade/Install Keywords: has-patch, 4.2-strings
Focuses: ui, accessibility Cc:


Please refer to attached screenshots. Not sure this is the only place to check, basically all the aria information should be updated together with the "visual" information to always reflect the current upgrade/install status.

In this example, where it still says "Update xx plugin now" or "Install xx plugin now", the value should be updated at least with the final result "Updated" or "Installed". Worth checking also what happens in case of failures.

Attachments (4)

updates.js.diff (1.8 KB) - added by mehulkaklotar 9 years ago.
31722.patch (1.4 KB) - added by afercia 9 years ago.
shiny-updates.diff (4.2 KB) - added by mehulkaklotar 9 years ago.
31722.2.patch (6.5 KB) - added by afercia 9 years ago.

Download all attachments as: .zip

Change History (21)

#1 @DrewAPicture
9 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.2

#2 @mehulkaklotar
9 years ago

Here we have a 1 problem here while updating the aria-label for a particular plugin. Because the aria-label has a plugin name in it. So will it be the the same message as the message showed to user without the plugin name in it? Attaching same changes here. But need more clarification over plugin name thing.

#3 @afercia
9 years ago

Hi @mehulkaklotar, thanks very much for your patch. You made a good point about the plugin name. I'd say to keep it simple and have in the aria-label the same message displayed to users, without the plugin name.
After all, users already know the name of the plugin they're updating because they've just pressed the button, for example their screen reader just read out:
"Update RTL Tester 1.0.5 now"

A couple of things:

  • patch should be refreshed after r31897 that removed the installs part
  • more importantly, the button with aria-label is used only in the "plugin-install.php" page so the patch should do nothing when updating from "plugins.php"

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

9 years ago

#5 @DrewAPicture
9 years ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to afercia
  • Priority changed from normal to high
  • Severity changed from normal to major
  • Status changed from new to assigned

Since this related to the accessibility of new functionality, we really need to come up with a good solution here sooner than later. @afercia: If you don't mind, I'm going to assign this ticket to you.

#6 @DrewAPicture
9 years ago

  • Status changed from assigned to reviewing

#7 @afercia
9 years ago

Following @mehulkaklotar approach, I would propose just a couple of tweaks, see previous comment 3. Results in the screenshot below. Proposed patch:

  • update the "Update" button aria-label attribute to reflect the current update status
  • do that just where there's a button, in the 'plugin-install' screen

9 years ago

#8 @mehulkaklotar
9 years ago

@afercia, can we extract the plugin names with some jQuery Code and then do some tweaks about the name appearing in the aria-label?

#9 @afercia
9 years ago

@mehulkaklotar hi, I agree it would be nice to have the plugin name added to the messages, but I see two issues there:

  • translation issues: for example "Updating {pluginname}" and "{pluginname} updated!" not sure how to handle that in different languages
  • the {pluginname} should be returned by the API response instead of trying to get it with some jQuery which is maybe a bit unreliable

maybe worth it to create a new ticket to propose the API to return the plugin name?

#10 @mehulkaklotar
9 years ago

Hello @afercia,

Yes. We can have a data-title attribute to the each plugin anchor links and then take that as input for updating the message in aria-label.

#11 follow-up: @afercia
9 years ago

Hello @mehulkaklotar
you're right, thanks very much :) we need some new strings and a placeholder to allow translators to shift the plugin name if needed. Agreed with @ocean90 to introduce a few new strings. To summarize:

New patch by @mehulkaklotar uses data attribute to pass the plugin name to JavaScript.

In the refreshed proposed patch:

  • since this is the plugin name, I'd propose to change "title" and "plugin_title" in "name"
  • new strings for translation, use a placeholder
  • tweaking existing strings (e.g. removed dot from 'Update Failed.', casing)
  • updates.js: removed some mixed space/tabs or double spaces
  • updates.js: moved 1 var declaration at the top
  • updates.js: jshinted

9 years ago

#12 in reply to: ↑ 11 @ocean90
9 years ago

Replying to afercia:

  • new strings for translation, use a placeholder

Please add a translator comment to each new string, which explains what the placeholder is used for: /* translator comment: %: plugin name */.

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

9 years ago

#14 @DrewAPicture
9 years ago

  • Keywords 4.2-strings added

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

9 years ago

#16 @jorbin
9 years ago

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

In 31982:

Update aria-label when doing a shiny plugin update

Also updates it again when the shiny plugin update is finished.
Also updates it if the shiny update fails

props mehulkaklotar, afercia
Fixes #31722

#17 @DrewAPicture
9 years ago

  • Priority changed from high to normal
Note: See TracTickets for help on using tickets.