#37727 closed enhancement (fixed)
Allow for customize control notifications to have extensible templates
Reported by: | westonruter | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 4.6 |
Component: | Customize | Keywords: | has-patch |
Focuses: | javascript | Cc: |
Description
It was noted by @celloexpressions toward the end of 4.6:
FYI I just noticed an issue with the JS template for control notifications introduced in [37476]. We need to display the raw message rather than escaping it
{{{
instead of{{
so that html is allowed. Which it should be for links, etc., and it's needed for #34923. I'm making the change in the next patch there and don't think we need to change it for 4.6 but wanted to point it out.
It is useful to embed non-message content into a notification to allow for buttons to action on the notification. However, I don't feel that just allowing the raw message
to be injected into the notification template is necessarily the best way to go. To me it would seem better if we allowed for Notification
subclasses that had custom templates to handle the various notification codes differently. Then the server wouldn't have to pass markup back as part of error messages.
Allowing custom templates for notifications would be highly useful for the global notification area in the Customizer (#35210).
See also https://core.trac.wordpress.org/ticket/30738#comment:16
Attachments (1)
Change History (12)
#2
@
4 years ago
- Keywords has-patch added; needs-patch removed
One approach is in 37727.0.diff where the Notification
object can have a template
argument which is then used to render the content of the notification (the containing li
is not mutable here). Nevertheless, the entire template for rendering out all of the notifications can be overridden via the notificationsTemplate
property on the wp.customize.Control
instance itself. @celloexpressions any thoughts on this patch?
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
3 years ago
#4
@
3 years ago
- Milestone changed from Awaiting Review to 4.7
It feels a bit odd to call it (and make it) a template since in most cases there won't/shouldn't be a need for any logic or even accessing the notification object. However, this should work well and also fixes an issue I'd seen with the message also going to screen readers. To avoid needing to make a separate message and template that are virtually the same, I wonder if the message could default to being the template with tags stripped if the message isn't provided but a template is?
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
3 years ago
#8
@
3 years ago
- Milestone changed from 4.7 to Future Release
- Owner celloexpressions deleted
Per today's customize chat, we can safely switch to unescaped notification output, eliminating the immediate need for this. However, extensible templates might still be useful in some cases, so we'll reconsider in a future release.
#9
@
2 years ago
- Milestone changed from Future Release to 4.9
- Owner set to westonruter
- Status changed from assigned to accepted
I'm going to resolve this with #35210.
@celloexpressions I just found a problem with adding markup to the
message
: screen readers could potentially read the markup. At least, that's what I just experienced with the wp-a11y-speech-synthesis plugin enabled.