Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#42049 closed task (blessed) (fixed)

Customize Themes: extensibility improvements

Reported by: celloexpressions's profile celloexpressions Owned by: westonruter's profile westonruter
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.9
Component: Customize Keywords: has-patch
Focuses: Cc:

Description

Extensiblility was a goal of #37661. Now that the initial framework is in core, let's audit the potential to extend the core functionality.

In particular, WP_Customize_Themes_Section should make it as easy as possible for developers to add theme sections for browsing themes from external sources. One initial improvement would be an instance parameter that determines whether searching and filtering is done locally on the loaded themes (like the installed section) or remotely through a theme directory API (like the wporg section) rather than using hardcoded checks for installed and wporg. We could also use filters in the load themes action to minimize the need for changes in themes section subclasses.

Attachments (2)

42049.diff (18.6 KB) - added by celloexpressions 7 years ago.
First pass at extensibility improvements and improved filter-searching.
42049.2.diff (18.0 KB) - added by celloexpressions 7 years ago.
Refresh for minor conflicts in trunk.

Download all attachments as: .zip

Change History (10)

@celloexpressions
7 years ago

First pass at extensibility improvements and improved filter-searching.

#1 @celloexpressions
7 years ago

  • Keywords has-patch added; needs-patch removed

In 42049.diff:

  • Introduce WP_Customize_Themes_Section::filter_type, which has built-in functionality for local and remote filtering. When this set to local, all themes are assumed to be loaded from Ajax when the section is first loaded, and subsequent searching/filtering is applied to the loaded collection of themes within the section. This is how the core "Installed" section behaves - third-party sources with limited numbers of themes may consider leveraging this implementation. When this is set to remote, searching and filtering always triggers a new remote query via Ajax. The core "WordPress.org" section uses this approach, as it has over 5000 themes to search.
  • Refactor filterSearch() to accept a raw term string as input. This enables a feature filter to be used on a section where filter_type is local.
  • Refactor filter() on a theme control to check for an array of terms. Also sort the results by the number of matches. Rather than searching for an exact match, this will now search for each word in a search distinctly, allowing things like tags to rank in search results more accurately.
  • Split loadControls() into two functions for themes section JS: loadThemes() to initiate and manage an Ajax request and loadControls() to create theme controls based on the results of the Ajax call. If third-party sections need to change the way controls are loaded, such as by using a custom control subclass of WP_Customize_Theme_Control, this allows them to use the core logic for managing the Ajax call and only override the actual control-creation process.
  • Introduce customize_load_themes_ajax filter to facilitate loading themes from third-party sources (or modifying the results of the core sections).

In addition to making it easier for third-party sources to leverage the core functionality here, this now brings significant improvements to the installed themes search filter. I don't have anything else planned for this ticket - the next step is to engage with theme marketplaces and encourage them to start building plugins to integrate here (Jetpack is probably a good starting point?), reporting any issues they find.

#2 @celloexpressions
7 years ago

  • Keywords commit added
  • Owner set to westonruter
  • Status changed from new to reviewing

Should be ready for final review/commit.

It would be nice to get this in for beta 1 so that the dev-note can be finished and include the API improvements here. Otherwise, this should transition to a task for 4.9, as we need to ensure that the new API meets the project's extensibility goals and doesn't cause future-compatibility issues. It would also be good to get the (installed) searching improvements tested more widely sooner rather than later.

#3 follow-up: @westonruter
7 years ago

  • Keywords needs-refresh added; commit removed
  • Milestone changed from 4.9 to Future Release
  • Owner changed from westonruter to celloexpressions

In extensibility, I don't think we should just yet add a bunch to extend when we haven't yet solidified the foundation. I'd rather make sure we have the core themes experience in place, and then in a subsequent release add extensibility. That way we'd be able to ensure that extensions are building off of a solid base. We can add the extensibility in 4.9.x even, as part of a v3 of the themes experience.

@celloexpressions also, the patch isn't applying on trunk as of r41723 at least (this is before #42083):

$ grunt patch:42049
Running "patch:42049" (patch) task
(Stripping trailing CRs from patch.)
patching file src/wp-admin/js/customize-controls.js
Hunk #1 succeeded at 1649 with fuzz 1 (offset 56 lines).
Hunk #2 succeeded at 1673 (offset 56 lines).
Hunk #3 succeeded at 1739 (offset 56 lines).
Hunk #4 succeeded at 1754 (offset 56 lines).
Hunk #5 succeeded at 1813 (offset 56 lines).
Hunk #6 succeeded at 1830 (offset 56 lines).
Hunk #7 succeeded at 1842 with fuzz 2 (offset 56 lines).
Hunk #8 succeeded at 1855 (offset 56 lines).
Hunk #9 succeeded at 1873 (offset 56 lines).
Hunk #10 succeeded at 1891 (offset 56 lines).
Hunk #11 succeeded at 1917 (offset 56 lines).
Hunk #12 succeeded at 1968 (offset 56 lines).
Hunk #13 succeeded at 1978 (offset 56 lines).
Hunk #14 succeeded at 1992 (offset 56 lines).
Hunk #15 succeeded at 2010 (offset 56 lines).
Hunk #16 FAILED at 2026.
Hunk #17 succeeded at 2066 (offset 56 lines).
Hunk #18 succeeded at 2096 (offset 56 lines).
Hunk #19 succeeded at 4662 (offset 27 lines).
1 out of 19 hunks FAILED -- saving rejects to file src/wp-admin/js/customize-controls.js.rej
(Stripping trailing CRs from patch.)
patching file src/wp-includes/class-wp-customize-manager.php
Hunk #1 succeeded at 4275 (offset 71 lines).
Hunk #2 succeeded at 4807 (offset 71 lines).
Hunk #3 succeeded at 4837 (offset 71 lines).
Hunk #4 succeeded at 4845 (offset 71 lines).
Hunk #5 succeeded at 4862 (offset 71 lines).
Hunk #6 succeeded at 4933 (offset 71 lines).
(Stripping trailing CRs from patch.)
patching file src/wp-includes/customize/class-wp-customize-themes-section.php

@celloexpressions
7 years ago

Refresh for minor conflicts in trunk.

#4 in reply to: ↑ 3 @celloexpressions
7 years ago

  • Keywords needs-refresh removed
  • Milestone changed from Future Release to 4.9
  • Type changed from enhancement to task (blessed)

Replying to westonruter:

In extensibility, I don't think we should just yet add a bunch to extend when we haven't yet solidified the foundation. I'd rather make sure we have the core themes experience in place, and then in a subsequent release add extensibility. That way we'd be able to ensure that extensions are building off of a solid base. We can add the extensibility in 4.9.x even, as part of a v3 of the themes experience.

You're misinterpreting the point of this ticket, and the way extensibility is intended to work in conjunction with future improvements to the feature in terms of UI.

This has nothing to do with the "first pass" UI for browsing themes from WordPress.org. In fact, on #37661, we discussed the possibility of third-party implementations in the customizer inspiring future improvements to the WordPress.org side of things. However, in terms of UI, the foundation for extensibility is already in place and unlikely to change based on changes to WordPress.org theme browsing in the customizer.

The installed themes experience is similarly unlikely to change, and this is where the majority of the improvements here are made. In addition to third-party theme sources, the improvements here make it much more feasible to effectively use this feature on large multisite networks, and extend the installed themes section as required. For example, large networks like WordPress.com can largely make use of the default installed themes experience, but may want to add filtering or other features - in order to do that without duplicating core logic (and breaking things if we change later), we shouldn't be hardcoding checks for the installed section action and rather introduce a parameter for local remote filtering. The changes here are as much for improved future compatibility as they are for ease of use in the API, which is how we can set ourselves up with the ability to make changes to the overall experience in the future without breaking things, if we ever decide to do that.

Regardless of when we want to encourage third-party implementations, there are a number of improvements in 42049.2.diff (a refresh) that should be made to improve the initial experience and UI (such as the searching changes, included here to avoid conflicting patches). And if we don't want people to extend the experience, we should explicitly mark certain elements as private (in a distinct ticket) rather than leaving an annoying-but-functionally-extensible API for public use. Punting is not the right approach here due to the nature of these changes.

#5 @celloexpressions
7 years ago

@westonruter please let me know if you have any specific concerns with the changes proposed in 1 and 42049.2.diff, which looks like it should apply now. Would be nice to get this in before beta 2.

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


7 years ago

#7 @westonruter
7 years ago

  • Owner changed from celloexpressions to westonruter
  • Status changed from reviewing to accepted

#8 @westonruter
7 years ago

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

In 41807:

Customize: Improve behavior and extensibility of theme loading and searching.

  • Introduce WP_Customize_Themes_Section::$filter_type, which has built-in functionality for local and remote filtering. When this set to local, all themes are assumed to be loaded from Ajax when the section is first loaded, and subsequent searching/filtering is applied to the loaded collection of themes within the section. This is how the core "Installed" section behaves - third-party sources with limited numbers of themes may consider leveraging this implementation. When this is set to remote, searching and filtering always triggers a new remote query via Ajax. The core "WordPress.org" section uses this approach, as it has over 5000 themes to search.
  • Refactor filterSearch() to accept a raw term string as input. This enables a feature filter to be used on a section where filter_type is local.
  • Refactor filter() on a theme control to check for an array of terms. Also sort the results by the number of matches. Rather than searching for an exact match, this will now search for each word in a search distinctly, allowing things like tags to rank in search results more accurately.
  • Split loadControls() into two functions for themes section JS: loadThemes() to initiate and manage an Ajax request and loadControls() to create theme controls based on the results of the Ajax call. If third-party sections need to change the way controls are loaded, such as by using a custom control subclass of WP_Customize_Theme_Control, this allows them to use the core logic for managing the Ajax call and only override the actual control-creation process.
  • Introduce customize_load_themes filter to facilitate loading themes from third-party sources (or modifying the results of the core sections).
  • Bring significant improvements to the installed themes search filter.

Props celloexpressions.
Amends [41648].
See #37661.
Fixes #42049.

Note: See TracTickets for help on using tickets.