WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#28709 closed task (blessed) (fixed)

Improve/introduce Customizer JS models for Controls, Sections, and Panels

Reported by: westonruter Owned by: ocean90
Milestone: 4.1 Priority: high
Severity: normal Version:
Component: Customize Keywords: has-patch
Focuses: javascript, docs Cc:
PR Number:

Description

The Customizer currently has a Control model which facilitates interacting with a control that was registered in the initial request to open the Customizer. As explored in the Widget Customizer (#27112), controls may also be dynamically instantiated and destroyed via JavaScript (see customize-widgets.js). However, the current Control model was not specifically designed for this since the constructor does not allow you to pass a template for the control to then get added to a given section's container element automatically. Instead, you're currently required to first construct the control's elements and insert them into the page, and then create a new Control model instance that connects to this element that was inserted into the document.

In addition to improving the Control model to facilitate dynamic instantiation via JavaScript, there also need to be models for managing Sections and (with #27406) Panels. Just as you can create controls dynamically, you should be able to create sections and panels. The models should allow the section/panel to have state for whether they are open or closed, and they should have methods for expanding/collapsing. They also should have collections which represent which controls a section has, and which sections a panel has. There should likewise be methods for adding controls to sections and adding methods to controls, and the DOM should automatically update to move the container elements to their appropriate locations when such changes are made.

Attachments (15)

28709.wip.diff (17.0 KB) - added by westonruter 5 years ago.
https://github.com/x-team/wordpress-develop/pull/29
28709.wip.2.diff (21.3 KB) - added by westonruter 5 years ago.
https://github.com/xwpco/wordpress-develop/compare/01050a4d20849a9c92505bab535a6aad2ce201ca...da6a3db755dbfc5d12876b35c80771e7aaa75453 https://github.com/xwpco/wordpress-develop/pull/29
28709.wip.3.diff (21.3 KB) - added by westonruter 5 years ago.
Refresh patch 28709.wip.2.diff
28709.wip.4.diff (30.5 KB) - added by westonruter 5 years ago.
https://github.com/xwpco/wordpress-develop/pull/29
28709.wip.5.diff (39.8 KB) - added by westonruter 5 years ago.
https://github.com/xwpco/wordpress-develop/compare/cdf7b96c9ea88f962b707434cebca993d9f56bba...c7816e1e777f25e0b52a3935b0b765455a11a8cc
28709.wip.6.diff (52.5 KB) - added by westonruter 5 years ago.
https://github.com/xwpco/wordpress-develop/compare/c7816e1e777f25e0b52a3935b0b765455a11a8cc...2a5c56abda35b47cb946a11d36b31940ca030b69
28709.wip.7.diff (58.7 KB) - added by westonruter 5 years ago.
https://github.com/xwpco/wordpress-develop/compare/2a5c56abda35b47cb946a11d36b31940ca030b69...5169fd3c115a8af92891e819180cd7ebcb64177c
28709.wip.8.diff (64.1 KB) - added by westonruter 5 years ago.
28709.9.diff (66.8 KB) - added by westonruter 5 years ago.
https://github.com/xwpco/wordpress-develop/pull/40
28709.10.diff (72.6 KB) - added by westonruter 5 years ago.
https://github.com/xwpco/wordpress-develop/compare/a8b2a02...2e773a5
28709.11.diff (73.7 KB) - added by westonruter 5 years ago.
https://github.com/xwpco/wordpress-develop/compare/2e773a5...f714ff4f
28709.12.diff (74.1 KB) - added by westonruter 5 years ago.
https://github.com/xwpco/wordpress-develop/compare/f714ff4...619799f4
28709.13.diff (4.6 KB) - added by westonruter 5 years ago.
https://github.com/xwp/wordpress-develop/pull/56
28709.14.diff (29.8 KB) - added by westonruter 5 years ago.
https://github.com/xwp/wordpress-develop/pull/56
28709.15.diff (2.8 KB) - added by ocean90 5 years ago.

Download all attachments as: .zip

Change History (61)

#1 @celloexpressions
5 years ago

I'm also creating controls and sections in the Menu Customizer, so this would be a big help. Should also be able to use it internally for #28580.

I'd be okay with moving the section and panel expanding/collapsing functionalities from accordion.js to something more structured and customizer-specific.

Last edited 5 years ago by celloexpressions (previous) (diff)

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


5 years ago

#3 @westonruter
5 years ago

  • Summary changed from Improve/introduce Customizer models for Controls, Sections, and Panels to Improve/introduce Customizer JS models for Controls, Sections, and Panels

#4 @westonruter
5 years ago

  • Milestone changed from Future Release to 4.1

#5 @Aniruddh
5 years ago

A Customizer JS model for settings will be very useful.

#6 @celloexpressions
5 years ago

  • Priority changed from normal to high

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


5 years ago

#8 @westonruter
5 years ago

There's some good synergy between this ticket and #29572. Ideally the DOM elements for the panels, sections, and controls would be created via JS when the page loads instead of having them rendered via PHP, since this would eliminate there from being two ways to add such structures to the page. In other words, a control shouldn't be constructing a selector and then using it to find its container DOM element. Instead, the control should be supplied the HTML (template) to be used for the container and then insert this control into the associated section.

In terms of the API, I suggest that we havewp.customize.panel and wp.customize.section to go along with the existing wp.customize.control, all of which would be Values instances. There would then be a wp.customize.Panel and wp.customize.Section to also accompany the wp.customize.Control, although there wouldn't need to be a panel/section construct to be analogous to controlConstructor, since there aren't explicit types of panels and sections, although they can be subclassed.

#9 @westonruter
5 years ago

In 28709.wip.diff, first stab at JS-driven panels, sections, and controls.

#10 @celloexpressions
5 years ago

This is a great start, Weston!

I think we should make sure that we support custom sections and panels as fully as we support custom controls. Whether that's through something similar to how controls do it, or something a little more intuitive, that's an important component of the API as a whole, even if it's not used as commonly.

As much as I'd love for everything to be rendered from JS, I don't think we're quite there yet. I see #29572 and this ticket as the first steps, then in 4.2 we could introduce a more comprehensive API for dynamically creating controls, then maybe in 4.3 render everything from JS by default. If we could build out some structure for sections and panels in JS, and maybe the ability to dynamically create sections, at least for now, I think that would be best. We're going to have to do a lot for back-compatibility once we get to that stage, so I think we'll need to move carefully with the more radical changes.

Some of the things I'd like to see done in the first-pass here:

  • Remove all customizer-specific logic from accordion.js, in favor of open/closing/state functions internally (ie, stop using accordion.js in the Customizer). Also with the thought that we may move away from using an accordion for sections sooner rather than later. This would also facilitate deep-linking.
  • Support for deep-linking, or a logical way that that would be implemented once this initial API is in, coordinating with the expanding/collapsing of sections.
  • JS-side support for contextual sections and panels (or a logical way to add that), I already did the PHP side on #29758.

Things that should probably wait, but that we should keep in mind:

  • Rendering all sections and panels from JS by default.
  • Changes to how custom sections and panels work in PHP.
  • I'm thinking it might be best to avoid significant changes to the control models for now, other than the ability to associate them with sections in some way, so that we can scope this appropriately for 4.1 and start building off of it.

#11 @westonruter
5 years ago

In 28709.wip.2.diff:

Watch changes to controls/sections/panels and reflow

  • Add active state to panels and sections, as with controls
  • Add toggle() method to panels and sections, as with controls
  • Invoke toggle() when a panel/section active state changes
  • Allow WP_Customize_Panel and WP_Customize_Section subclasses to specify a distinct type, and be able to associate them with custom constructors in JS, just as with WP_Customize_Control.

Examples:

Automatically collapse the header_image section when its only control is inactive

wp.customize.control( 'header_image' ).active.set( false )

Move the header_image control into the background_image section:

wp.customize.control('header_image').section( 'background_image' )

Move the background_image section to the end of the Customizer pane:

wp.customize.section('background_image').priority( 10000 ) 

This should result in the colors section and wigets panel being hidden, but it's not sticking right now:

wp.customize.section( 'colors' ).active( false )
wp.customize.panel( 'widgets' ).active( false )

Need to implement Panel.focus(), Section.focus(), and Control.focus() which can be used by deep-linking. Widgets should be updated to use this as well. Widgets should perhaps use the priority() state to specify the ordering of widget form controls when dragging them into a new order.

Still need to merge accordion logic into the Customizer controls.

This implements the JS API for the active_callback in Panels and Sections (#29758). The PHP side needs to be hooked up.

Additionally, the Customizer preview should postMessage the panels, sections, controls that it expects up to the Customizer pane and they can be created on demand. This would be lazy-loading (#28580).

celloexpressions, regarding your first-pass feedback:

Remove all customizer-specific logic from accordion.js, in favor of open/closing/state functions internally (ie, stop using accordion.js in the Customizer). Also with the thought that we may move away from using an accordion for sections sooner rather than later. This would also facilitate deep-linking.

Still need to do this, but we can nicely organize the logic into the Panel and Section models now.

Support for deep-linking, or a logical way that that would be implemented once this initial API is in, coordinating with the expanding/collapsing of sections.

Stubbed out. The focus() methods I think will serve well here, and then in the Customizer initializer/bootstrap we can look for a query param for a panel/section/control to focus on, and then call the focus method on that item.

JS-side support for contextual sections and panels (or a logical way to add that), I already did the PHP side on #29758.

Added.

celloexpressions: Regarding your feedback for things which should wait for a future release:

Rendering all sections and panels from JS by default.

Take a look. They're all getting rendered with JS now.

Changes to how custom sections and panels work in PHP.

I followed the same pattern for custom sections/panels as was already done for controls. It's actually a pretty low footprint.

I'm thinking it might be best to avoid significant changes to the control models for now, other than the ability to associate them with sections in some way, so that we can scope this appropriately for 4.1 and start building off of it.

There aren't too many changes to the control model, aside from the addition of an embed method and the container, priority, and section properties. And of course that the controls are all added via JS now.

Let me know what you think! Feel free to submit tweaks to my PR on GitHub: https://github.com/xwpco/wordpress-develop/pull/29

This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.


5 years ago

#13 @westonruter
5 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

@westonruter
5 years ago

Refresh patch 28709.wip.2.diff

This ticket was mentioned in IRC in #wordpress-dev by ocean90. View the logs.


5 years ago

#15 @westonruter
5 years ago

In 28709.wip.4.diff attached latest WIP. I believe the core JS models for Panels, Sections, and Controls are now in place. What remains is making sure that the Widgets in the Customizer make use of the new API instead of manually manipulating the DOM as it is right now: namely, reordering widgets by drag/drop should now make sure the corresponding property values get updated so that any JS sorting doesn't result in an undefined order.

#16 @westonruter
5 years ago

For an example of what this new API now facilitates:

if you are inside of an expanded panel, and you inactivate all sections within it, for example:

_( wp.customize.panel( 'widgets' ).sections() ).each( function ( section ) {
    section.active( false );
} );

Then it will automatically collapse the panel and slide up the accordion section title so that it can no longer be accessed.

Also, if you re-assign all controls from one section to another, the former section will automatically appear as deactivated (it will hide) because it is contextually inactive (it has no controls that are active). Moving all controls from a "colors" section to a "styles" section, also resulting in the colors section being collapsed and hidden:

_( wp.customize.section( 'colors' ).controls() ).each( function ( control ) {
    control.section( 'styles' );
} );

#17 @westonruter
5 years ago

Oh, and the patch needs so me love from celloexpressions in terms of how the accordions and panels should be handled. Anyone who wants can submit a PR to amend https://github.com/xwpco/wordpress-develop/pull/29 or I can add you to push commits directly to this develop.git.wordpress.org clone on GitHub.

#18 @celloexpressions
5 years ago

I'm going to figure out the accordion/section/panel stuff as soon as I can, most likely tomorrow evening. In the meantime, I encourage everyone who can to leave feedback either here or on GitHub.

We particularly need dev feedback as this is a massive patch that we need to expedite completion of.

Weston, could you put together some more usage examples that cover different aspects of the API? We'll probably want to do a 4.1 Customizer improvements post like we did for 4.0, although this could probably get its own post as well.

#19 @westonruter
5 years ago

In 28709.wip.5.diff I tried to align the Sections and Panels more closely with Controls and the Widgets controls. Instead of the toggle( active ) actually changing the active value (state), now this method is aligned with widgets where it is a handler to respond to an active state change. The name has also been renamed to toggleActivewith toggle now being a deprecated alias. Sections and Panels now have toggleExpanded as well.

I'm pretty happy with the active and expanded states, the shortcut methods that change them (activate()/deactivate(), expand()/collapse()), but what I'm not really happy about is the API for how controls respond to changes in state, and how the behavior is defined. Right now, the models have a toggleActive( active ) and a toggleExpanded( expanded ) methods. You might think that this would change the underlying active and expanded states, but this is not their purpose. Instead of changing their associated state, they are called when their associated state is changed. So they are event handlers. The naming toggle* implies that a state is going to be changed though. Perhaps it would be better to say onToggleActive() and onToggleExpanded(), and that these are the methods that subclasses would override to provide custom behaviors.

Thoughts?

#20 @ryankienstra
5 years ago

Weston's onToggleActive() and onToggleExpanded() function names are more accurate than only toggleActive() and toggleExpanded(), in my opinion. The "on" is appropriate because he binds it to the wp.customize.Control (self) object in line 69 of customize-control.js:

self.active.bind( function ( active ) { 
  self.toggleActive( active && self.isContextuallyActive() );

Also, the code looks cleaner with the same method names used for sections, panels, and controls.

#21 @westonruter
5 years ago

In 28709.wip.6.diff (new changes):

  • Add initial unit tests for wp.customize.Class in customize-base.js
  • From #29758, add PHP API for contextual panels/sections (props @celloexpressions) along with JS one.
  • Eliminate SidebarControl.toggleActive in favor of setting the containing section's active state, using the API from #29758.

#22 @westonruter
5 years ago

In 28709.wip.7.diff (new changes):

  • Clean up accordion.js, removing all Customizer-specific logic
  • Prevent accordion.js from acting on the Customizer if a plugin enqueues it for another purpose (Menu Customizer is an example)
  • toggleExpanded() and toggleActive() changed to onToggleExpanded() and onToggleActive(). Also, expandedArgumentsQueue introduced to hold arguments for animation duration and (soon) allowMultiple
  • Normalize API for passing parameters to activate/deactivate, expand/collapse
  • Add defaultActiveArguments and defaultExpandedArgument
  • Pass onToggleActive args.duration to slideDown/slideUp
  • Add activeArgumentsQueue to Control
  • Use onToggleExpanded and onToggleActive naming on Widget controls
  • Close top-level sections immediately when opening panel
  • Fix Widget Customizer _applyCardinalOrderClassNames to use API instead of DOM traversal

Also props @kienstra @celloxpressions

See related PRs for discussions:

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. View the logs.


5 years ago

#24 @westonruter
5 years ago

In 28709.wip.7.diff:

  • Rename onToggle* to onChange*
  • Move WidgetControl duration to defaultExpandedArguments
  • Short-circuit expand/collapse, activate/deactivate if already set
  • Make sure Control.activate/Control.deactivate take params
  • Update WidgetControl.collapse/WidgetControl.expand to accept params
  • api.Section.focus() and api.Control.focus()
  • Support allowMultiple option on Section.expand()
  • Factor out duplicate logic from expand/collapse, activate/deactivate; always call completeCallback
  • Expand contained panel first before expanding section
  • Update Section.focus to build upon Section.expand
  • Consolidate focus into one shared method reused by Panels, Sections, Controls

Also props @kienstra @celloxpressions

#25 @westonruter
5 years ago

In 28709.9.diff, make necessary changes to support deep-linking of Customizer panels/sections/controls. See #28650.

@celloexpressions: The panels still need some love. And this may be related, but now when dragging a widget from one section to another, it often takes a few times of dragging a widget over another section until jQuery UI recognizes the widget control as being dragged over it. The negative margin may be the culprit.

#26 @westonruter
5 years ago

  • Owner set to ocean90
  • Status changed from new to reviewing

#27 @westonruter
5 years ago

In 28709.10.diff:

  • Allow controls and sections to be registered without parents
  • Add ready() abstract to Section/Panel
  • Remove needless callbacks
  • Factor out keydown enter key event detection logic
  • Eliminate hard-coded 150 number for meta section toggle speed
  • Fix collapsing of other open sections in the accordion
  • Invoke completeCallbacks even when unchanged
  • Construct widget controls properly using new API
  • Fix rendering multiple panels.

HEAD: 2e773a5

#28 @westonruter
5 years ago

In 28709.11.diff:

  • Use event instead of e for variable names
  • Use non-debounced reflowPaneContents when Customizer ready fires. Let subsequent calls be debounced when changes happen to the models. Props @ocean90
  • Skip doing any DOM manipulation if the ordering is already correct

Commit HEAD: f714ff4

Code review comment welcomed on the PR: https://github.com/xwpco/wordpress-develop/pull/29/files

#29 @westonruter
5 years ago

In 28709.12.diff (HEAD 619799f):

  • Simplify logic for areElementListsEqual and annotate
  • Fix checking of indexOf return value

Known remaining issues/todos:

  • Animation of panels needs work. The margin-top seems to be causing sporadic problems. Dragging widgets between sections seems to be a side-effect of this.
  • The duration argument should be replaced with an instant flag.
  • Move private functions into a wp.customize.utils namespace to expose them for unit testing.
  • Add more unit tests.
  • Evaluate merit of remaining @todo comments.

This patch includes fixes for:

Last edited 5 years ago by westonruter (previous) (diff)

#30 @ocean90
5 years ago

In 30102:

Improve/introduce Customizer JavaScript models for Controls, Sections, and Panels.

  • Introduce models for panels and sections.
  • Introduce API to expand and focus a control, section or panel.
  • Allow deep-linking to panels, sections, and controls inside of the Customizer.
  • Clean up accordion.js, removing all Customizer-specific logic.
  • Add initial unit tests for wp.customize.Class in customize-base.js.

https://make.wordpress.org/core/2014/10/27/toward-a-complete-javascript-api-for-the-customizer/ provides an overview of how to use the JavaScript API.

props westonruter, celloexpressions, ryankienstra.
see #28032, #28579, #28580, #28650, #28709, #29758.
fixes #29529.

#31 @ocean90
5 years ago

  • Focuses docs added
  • Keywords needs-patch added; has-patch dev-feedback removed

Another todo: Some more docs.

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


5 years ago

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


5 years ago

#34 @ocean90
5 years ago

In 30307:

Customizer: Use jQuery.fn.toggle() instead of slideUp/slideDown if panel/section/control is not inserted into DOM yet.

jQuery does nothing when calling slideUp on elements that are not inserted into the DOM yet, which can now be the case now when first loading the Customizer as the panels, sections and controls get dynamically inserted, see #28709.

props westonruter.
fixes #30251.

#35 @ocean90
5 years ago

In 30308:

Customizer: Trigger widget-added event when initializing.

Widget controls are now added to the pane dynamically via JavaScript, see #28709.
Remove the event trigger from SidebarControl.addWidget() as it's covered by WidgetControl.ready().

props westonruter.
fixes #30236.

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


5 years ago

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


5 years ago

#38 @ocean90
5 years ago

  • Type changed from enhancement to task (blessed)

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


5 years ago

#40 @westonruter
5 years ago

In 28709.13.diff:

  • Use embedded instead of ready to name Deferred
  • Remove needless todo related to deeplinking
  • Remove todos for events which we can add later if requested

#41 @westonruter
5 years ago

In 28709.14.diff amend the previous patch with ryankienstra's refactoring of private functions into wp.customize.utils with new QUnit tests.

#42 @westonruter
5 years ago

  • Keywords has-patch added; needs-patch removed

@ocean90
5 years ago

#43 @DrewAPicture
5 years ago

@ocean90, anything left here other than 28709.15.diff?

#44 @ocean90
5 years ago

In 30714:

Customizer: Add panel/section type as CSS class to the HTML container.

see #28709.

#45 @ocean90
5 years ago

In 30716:

Customizer: Move private helper functions to wp.customize.utils so they can be unit tested.

Includes unit tests.

props ryankienstra, westonruter.
see #28709.

#46 @ocean90
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In [30716] I have removed the changes to containerRootElement from 28709.14.diff. I would put this rather to wp.customize than wp.customize.utils.


Seems like we're done here.
New tickets please.

Note: See TracTickets for help on using tickets.