Opened 4 years ago
Last modified 5 weeks ago
#50442 accepted enhancement
Add prefixes to all admin notices (Warning, Error, Success, Info)
Reported by: | kebbet | Owned by: | joedolson |
---|---|---|---|
Milestone: | 6.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Administration | Keywords: | needs-design needs-patch early |
Focuses: | ui, accessibility, ui-copy | Cc: |
Description (last modified by )
This is a continuation #47656
From @afercia comment (ticket:47656#comment:47)
- Make sure all the Error and Warning notices use respectively an "Error:" and "Warning:" text prefix
- Further discuss whether also the "Success" and "Info" notices should use a similar text prefix
- Update documentation related to admin notices, for example: https://codex.wordpress.org/Plugin_API/Action_Reference/admin_notices and https://developer.wordpress.org/reference/hooks/admin_notices/
- Create a new issue on the Gutenberg GitHub to add text prefixes for the editor notices: right now, none of them uses a prefix (see screenshot below)
I would like to add
- Combine strings constructed like
__('Error:') . __('any string')
into one string for better context for translators.
Change History (22)
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
4 years ago
#4
@
11 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.
#6
@
11 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
@
11 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.
#9
@
11 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
@
10 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?
#12
@
8 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.
7 months ago
#14
@
7 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.
4 months ago
#17
@
4 months ago
Yes, I was looking at it last Friday, but I'm still on the fence on implementation.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
4 months ago
#19
@
4 months ago
- Keywords early added
- Milestone changed from 6.6 to 6.7
We have three options: 1) text and an icon; 2) icon with screen reader text or 3) icon without screen reader text.
The current notices only use color to indicate type: info, error, or success.
The goal is to make the type of notice more evident.
Adding visible text creates a large amount of labor for plugin authors and for core, as some existing notices will already contain the relevant text, e.g. "Error: your error message".
Adding screen reader text is less evident to authors, so it creates the same duplication only for screen reader users, and is less likely to be observed or fixed by extenders.
So the question is whether only adding an icon is acceptable. It improves on the existing model, which only uses color. In theory, the text of the message should always be clear enough to communicate the type of message, in which case adding screen reader text is redundant.
Do we consider this visual information to be supplemental, or informative? If it's supplemental, then we don't need screen reader text. If it's informative, we do.
From the accessibility bug scrub, we agreed that the icon should be considered informative, for consistency. This means that along with making this change, we'll also need to review the text of all core notices to potentially rewrite and remove any existing prefixes, and also provide sufficient time to extenders to be able to rewrite their own messages.
Punting to 6.7 and marking as early.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
2 months ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
5 weeks ago
#22
@
5 weeks ago
I certainly agree this would be informative for all users. I think while a dev note could be added to instruct authors to remove the prefix, I assume it probably isn't super important. Couldn't we just do something like this?
<?php $error_prefix = __( 'Error' ); $check_error_prefix = preg_match( '/^Error/', $admin_notice_content ); if ( $check_error_prefix === 1 ) { return sprintf( '<span class="screen-reader-text">%s: </span>%s', $error_prefix, $admin_notice_content ); }
My PHP is a bit rusty but I think the meaning is understood.
Thanks.
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.