WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 weeks ago

#30738 accepted enhancement

JS content templates for base WP_Customize_Control

Reported by: celloexpressions Owned by: westonruter
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.1
Component: Customize Keywords: needs-unit-tests needs-patch
Focuses: accessibility, javascript, rest-api Cc:

Description

This will make it possible to create all of the base Customizer control types directly in JS, and will allow them to share a single template passed from the server on Customizer init for all controls with the basic types (following up on #28709).

I think we may need to explore adjusting how register_content_type works since this control handles multiple types and all fallback types. But more likely we'll just do something a bit different, probably adding fallback handling on the other side and hard-coding these types into the Customizer Manager somewhere.

Follow-up to #29572. Related to several tickets I'm currently creating.

Attachments (3)

30738.wip-2.diff (9.0 KB) - added by celloexpressions 2 years ago.
Mostly complete but needs debugging.
30738.diff (9.0 KB) - added by obenland 2 years ago.
export-setting-defaults.diff (1.4 KB) - added by westonruter 4 weeks ago.

Download all attachments as: .zip

Change History (43)

#1 @celloexpressions
3 years ago

Basically need to convert WP_Customize_Control::render_content() into a JS/Underscore template, then hook up the necessary pieces to address this control applying to multiple types.

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


2 years ago

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


2 years ago

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


2 years ago

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


2 years ago

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


2 years ago

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


2 years ago

@celloexpressions
2 years ago

Mostly complete but needs debugging.

#8 @celloexpressions
2 years ago

  • Focuses javascript added
  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 4.3

@westonruter 30738.wip-2.diff should be most of what we need but there's a syntax error in the big template, and I'm not finding it, do you see it? I think it should be a quick fix and that this should be a relatively easy/painless improvement, but we'll have to see how it goes.

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


2 years ago

@obenland
2 years ago

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


2 years ago

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


2 years ago

#12 @ocean90
2 years ago

  • Keywords needs-unit-tests added

#13 follow-up: @celloexpressions
2 years ago

Patch is not yet functional - we need someone to pick this up or we're going to have to punt probably.

Note that Menus in the Customizer are currently using several custom controls that really shouldn't be custom controls to step around this and use a common setting, which I believe the settings control param can handle.

#14 in reply to: ↑ 13 @obenland
2 years ago

Replying to celloexpressions:

Patch is not yet functional - we need someone to pick this up or we're going to have to punt probably.

Yes, unless you and @westonruter find time to make a decision on it this weekend, this is probably going to get punted.

#15 @celloexpressions
2 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 4.3 to Future Release

This should definitely happen, but no longer a priority for 4.3. Need a functional patch first.

#16 @westonruter
10 months ago

There is a partial implementation of this in the Customize Posts plugin (see also #37275):

https://github.com/xwp/wp-customize-posts/blob/develop/php/class-wp-customize-dynamic-control.php
https://github.com/xwp/wp-customize-posts/blob/develop/js/customize-dynamic-control.js

Something implemented here, which I think has been discussed previously, is that the label element doesn't wrap the title and description anymore (for non-checkboxes at least), by introducing a random id for the input to be used with the label[for] attribute. This is turning out to be important now with the introduction of the new notifications in 4.6 which have a container that gets injected right after the site title. This means that if there is a button inside of a notification, it won't work as expected because it is wrapped in a label. Also related here is that notifications should support a template allowing them to allow arbitrary markup as opposed to an escaped message. This was noted by @celloexpressions in https://wordpress.slack.com/archives/core-customize/p1469851496000101

#17 @westonruter
10 months ago

This might go in another ticket, but we also should allow for a content_template param to be passed in when instantiating a widget in JS. When this is supplied, it should use that template instead of the template selected via the type param as output from the PHP WP_Customize_Control::content_template() method. This would allow for custom control templates to be easily used without having to define an entirely new WP_Customize_Control or hack around with the type parameter to get it to use something else.

#18 @westonruter
10 months ago

Here's an implementation of content_template param which can be incorporated into Control#renderContent(): https://github.com/xwp/wp-customize-posts/pull/225/commits/495b1fcb0c2286305b9e38f3c868f6ae3e970c83

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


10 months ago

#20 @westonruter
10 months ago

  • Milestone changed from Future Release to 4.7
  • Owner set to westonruter
  • Status changed from new to accepted

Adding to 4.7 due to the existing code in Customize Posts which can be lifted.

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


10 months ago

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


9 months ago

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


9 months ago

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


9 months ago

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


8 months ago

#26 @jorbin
8 months ago

  • Milestone changed from 4.7 to Future Release

Removing from the 4.7 milestone due to lack of traction.

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


8 months ago

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


8 months ago

#29 @westonruter
5 months ago

  • Milestone changed from Future Release to 4.8

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


5 months ago

#31 @westonruter
4 months ago

The dropdown-pages control type actually _should_ be included among the types that have content template, and the list of pages that appears in the select field should be fetched via the REST API and populate it dynamically. See also #39908.

#32 @westonruter
4 months ago

  • Focuses rest-api added

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


3 months ago

#34 @afercia
7 weeks ago

  • Focuses accessibility added

Couple of considerations:

Labels
As per the Accessibility coding standards, all new code must use an explicitly associated label, that is the ones that don't wrap the associated element and use for/id attributes.

get_input_attrs()
It would be great to make this a general utility to be re-used in other places too, with an option to get or echo or two separate functions for that. See for example something very similar planned to be used for the Settings API revamping ongoing effort on GitHub: https://github.com/wpaccessibility/settings-api-enhanced/blob/50a91ffba9ef857f9b40010b1d8b3684ef645ae4/wp-admin/includes/template.php#L635

/cc @flixos90

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


7 weeks ago

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


6 weeks ago

#37 @jbpaul17
6 weeks ago

  • Milestone changed from 4.8 to 4.8.1

Punting to 4.8.1 per today's bug scrub.

#38 @westonruter
4 weeks ago

Just realized that the default param from WP_Customize_Setting is not getting exported from PHP to JS. Sometimes, as in the case of the color control, the default value from the setting is needed by the control. The color control currently copies the setting's default value into the control when the control is exported to the client, but this isn't going to facilitate controls that are added dynamically on the client. So I think something like export-setting-defaults.diff should also be done.

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


3 weeks ago

#40 @westonruter
2 weeks ago

  • Milestone changed from 4.8.1 to 4.9
Note: See TracTickets for help on using tickets.