WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 11 months ago

#37727 assigned enhancement

Allow for customize control notifications to have extensible templates

Reported by: westonruter Owned by:
Milestone: Future Release 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)

37727.0.diff (2.0 KB) - added by westonruter 12 months ago.

Download all attachments as: .zip

Change History (9)

#1 @westonruter
12 months ago

@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.

#2 @westonruter
12 months 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.


11 months ago

#4 @celloexpressions
11 months 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.


11 months ago

#6 @celloexpressions
11 months ago

  • Owner set to celloexpressions
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.


11 months ago

#8 @celloexpressions
11 months 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.

Note: See TracTickets for help on using tickets.