Make WordPress Core

Opened 4 years ago

Last modified 5 hours ago

#50442 accepted enhancement

Add prefixes to all admin notices (Warning, Error, Success, Info)

Reported by: kebbet's profile kebbet Owned by: joedolson's profile joedolson
Milestone: 6.6 Priority: normal
Severity: normal Version:
Component: Administration Keywords: needs-design needs-patch
Focuses: ui, accessibility, ui-copy Cc:

Description (last modified by sabernhardt)

This is a continuation #47656

From @afercia comment (ticket:47656#comment:47)

I would like to add

  • Combine strings constructed like __('Error:') . __('any string') into one string for better context for translators.

Change History (17)

#1 @apedog
4 years ago

I understand the reasoning behind having mandatory Error and Warning prefixes. I don't hold an opinion either way - but I understand why it can be considered beneficial. It might not be pretty. It might be redundant for some messages (eg. strings that already contain the word 'error' in them) but they're a good marker/label for those notices. Good for accessibility. They grab attention regardless of coloring scheme. And they're unambiguous.

However mandatory 'Info' and 'Success' prefixes would just be annoying.

Adding the word 'Info' to what is already an information string does not add any new information. It's already information. It's not only redundant it's stupid.

The only difference between Info and Success notices is that Success notices accompany an action while Info notices are passive notices. There is no real accessibility need (none that I know or can think of) to disambiguate them from Info notices. Not in the way that Error and Warning need to stand out.

-1 for adding mandatory 'Info' and 'Success' prefixes.

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

4 years ago

#3 @sabernhardt
3 years ago

  • Description modified (diff)

#4 @joedolson
7 months ago

  • Milestone changed from Awaiting Review to 6.5
  • Owner set to joedolson
  • Status changed from new to accepted

With the addition of the new admin notice functions, this can be handled much more easily. Slating to look at in 6.5. Pinging @costdev for awareness.

#5 @joedolson
7 months ago

  • Focuses accessibility added

#6 @costdev
7 months ago

Thanks for the ping @joedolson!

To what extent should we consider extenders who may make use of these functions in 6.4?

  • If they already prefix their notices with an appropriate or preferred string, forcing Core's own may be akin to a regression/BC break/inconvenience.
  • For extenders who transfer to the new functions, they may use the same (translated) strings they previously used with hardcoded HTML. If those strings already contain a prefix and the extender feels they should now remove their prefix as Core enforces one, their translations will also need to be updated.
  • Checking for existing prefixes is likely to be unreliable and an unnecessary (albeit, tiny) performance hit.
  • Core could instead enforce its own rule for prefixing $message parameters passed in Core.
  • The previous point is the reason why I didn't suggest adding a (string) $prefix key to $args.

To be clear though, I personally like the idea of abstracting this to wp_get_admin_notice(). The functions were created to benefit exactly this type of ticket. There are unlikely to be many notices requiring translation in the first place, and the markup is filterable if someone really doesn't like them.

I mentioned the above points because I think our thoughts about at least some of these should be documented on this ticket.

Regarding whether to prefix all notice types:

IMO, it's good for users to get the context of the notice right away - That's why we use colours. As these can't be utilized by all users, the prefixes should increase accessibility, and that's the main reason I think prefixing all notice types makes sense.

Combine strings constructed like __('Error:') . __('any string') into one string for better context for translators

This is where the use of the functions becomes problematic. The combined strings would be provided via the $message parameter, and therefore couldn't be standardized within the function. Thoughts @joedolson? How important do you think this addition is @kebbet?

#7 @joedolson
7 months ago

  • Keywords needs-design added

One reason for addressing this quickly is to reduce the impact of extender implementation. Most extenders won't take this up immediately, I suspect - for plugin authors, it's rare to only support the latest release, so the labor to start using this heavily won't be common right away. So I think we can make reasonable use of this if we implement it early and provide a good dev note - the changes any extender would need to make will be text only, so they're relatively easy changes.

One thing we can consider is making the text more decorative, e.g. actual text present in association with the border, but presented in a way that it doesn't simply append as text. More of a label or flag on the panel. That makes the information more of an accessible version of the existing border indication, rather than an extension of the error text.

Adding needs-design to reflect that.

#8 @costdev
7 months ago

Great points @joedolson!

#9 @joedolson
7 months ago

Reading through the history of this issue, as reflected in #47656, I can see that there are a number of problems that should be taken into consideration. Design-wise, there's some back-compat issues with handling this outside of core, and a lot of text churn that could happen with changing all errors. In many cases, the context of an error or warning is fairly clear, but right now the distinction is only conveyed using color. Adding this as text is reasonable, but this might be a case where we can more easily handle this issue by adding an icon to indicate each state with accessible text.

What this avoids is 1) the need to modify the text of existing notices. 2) Fixes the color-only indicator problem, by providing cues through color, icon, and text (for screen readers.) The gap is a lack of text indication for sighted users who don't immediately understand the icon, but this information is all enhancing comprehension; the color and icon should never be critical to the message. If we have a message in a notice that leaves ambiguity, then that message should be updated.

I suggest using dashicons-no for errors, dashicons-warning, for warnings, dashicons-info, for info, and dashicons-yes for success.

Open to arguments against this, however.

#10 @sabernhardt
6 months ago

The dashicons-no icon looks very similar to the dismiss icon in admin notices. Would a red flag be a good (international) choice for error messages?

#11 @joedolson
4 months ago

Good call; we don't want people to start thinking there are two dismiss buttons.

#12 @kebbet
4 months ago

And a sidenote on prefixes: the swedish locale tends to remove the translations of success in many messages as there is no good translation for it. Adding Success as a prefix sound to me really unnecessary.

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

3 months ago

#14 @joedolson
3 months ago

  • Milestone changed from 6.5 to 6.6

It probably makes more sense to try and complete this after we've finished extending usage of the admin notice functions globally. We didn't get that finished in 6.5, so I think this is best punted. However, I'm going to milestone it for 6.6, since I think that's pretty plausible.

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

5 hours ago

#16 @mukesh27
5 hours ago

  • Keywords needs-patch added

@joedolson Does this ticket in your radar for 6.6?

#17 @joedolson
5 hours ago

Yes, I was looking at it last Friday, but I'm still on the fence on implementation.

Note: See TracTickets for help on using tickets.