Shiny Updates: consider to improve the bulk update failure notices

[afercia]

When bulk-updating plugins and there's some failure in the update process, Shiny Updates displays a notice on top of the screen:

Users can use bulk update also to update just one plugin. In this case, the error message could be improved a bit:

To reproduce a failure message:

• select one or more plugins to update
• disconnect from the internet
• select "Update" from the Bulk Actions
• press "Apply" and wait for a few seconds

Also, since the "1 failure" is a button that expands a panel with the error details, it should use an aria-expanded attribute to be dynamically toggled to give assistive technologies users some feedback about what's happening on the screen.

[swissspidy]

Bulk updates were improved in Shiny Updates V2, so clearly a regression in trunk. The second screenshot indeed looks odd.

Unfortunately I don't have much time at the moment, but it would be great if we could fix this in 4.6

[juhise]

Replying to swissspidy:

Unfortunately I don't have much time at the moment, but it would be great if we could fix this in 4.6

I tried to fix this. As per @afercia suggestion, added aria-expanded attribute.
I am tester so not much good in coding. May be this needs improvement.
I have added toggle arrow, so by this user can know what's happening on the screen.

[Ankit K Gupta]

@juhise
Seems you missed the case when more than one failure appears. I have added this case in your code.
Thanks

[Ankit K Gupta]

Updated patch file

[afercia]

@juhise @Ankit K Gupta thanks very much the approach looks good. I'd recommend a few things:

• the %s failure (singular and plural) string should be improved a bit; to keep it simple, maybe there's no need to distinguish between plugins and themes
• this should be translatable: <span class="screen-reader-text">Toggle panel: Failure Notice</span>
• the patch should use some ​coding standards, mainly about braces and spaces; also, WordPress uses tabs for indentation and not spaces, this has probbly something to do with some settings in the editor you're using
• the if/else to toggle the aria-expanded value maybe could be avoided and use just the value of $bulkActionNotice.find( 'ul' ).hasClass( 'hidden' ) which already returns true or false
• jQuery optimisation: instead of getting the same elements multiple times, I'd get them once and store them in a variable for later reuse

[juhise]

updated patch

[juhise]

Tried to update the patch according to the suggestion.

[juhise]

Replying to swissspidy:

Unfortunately I don't have much time at the moment, but it would be great if we could fix this in 4.6

@swissspidy
Are we not going to fix this in 4.6?

[afercia]

Refreshed a bit the patch, looks good to me.

• changed the "failure/s" strings
• added a couple CSS classes to make things easier
• removed a few unnecessary things

In updates.js the patch removes also some tab characters (my editor does that automatically) probably previous developments leftovers, so there will be 5 lines with unrelated changes.

If it's going to make it in 4.6, then it should have the i18n-change keyword.

[jorbin]

I've updated the patch to use a class and target it for the css. I kept the 'Toggle panel: Failure Notice' string as we use a similar string already in https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/template.php#L1039

[ocean90]

37510.diff​ doesn't work because there is no such CSS class, 37510.3.patch​ works as expected.

I kept the 'Toggle panel: Failure Notice' string as we use a similar string already

Yep, but "toggle" and "panel" are words which are not easy translatable in some languages. Can we use "Show more details"?

[afercia]

Replying to ocean90:

37510.diff​ doesn't work because there is no such CSS class

Looking, I guess it just needs {{ data.className }} to be added in the template.

"toggle" and "panel" are words which are not easy translatable in some languages. Can we use "Show more details"?

"Toggle" is handy because, for example, you don't need to dynamically update "Show details" in "Hide details" and vice-versa but yeah it's difficult to translate in many languages, Italian too. Considering the whole string, and how it gets announced by screen readers thanks to the aria-expanded attribute, I'd say it's OK. See screenshots below:

[afercia]

Refreshed patch to address the last 2 issues.

[ocean90]

37510.2.diff​ looks good to me. @jorbin, what you you think?

[jorbin]

In 38185:

Updates: Improve bulk update failure notice

When doing a bulk update, if there are failures the user needs to know about that. This makes it clearer that you can click on the notification to see more details, especially for screen reader users.

Fixes #37510.
Props juhise, Ankit K Gupta, afercia, jorbin, ocean90. 

[jorbin]

In 38187:

Updates: Improve bulk update failure notice

Merge of [38185] to the 4.6 branch

When doing a bulk update, if there are failures the user needs to know about that. This makes it clearer that you can click on the notification to see more details, especially for screen reader users.

Fixes #37510.
Props juhise, Ankit K Gupta, afercia, jorbin, ocean90.