Make WordPress Core

Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#40254 closed defect (bug) (fixed)

Customize: Eliminate customize-loader fullscreen iframe method for opening customizer in core

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


Dependency from #28536, which comments:

Tentatively we're thinking to use the functionality as it exists in the feature plugin which only applies when customize-loader is not used, and also to remove customize-loader from being used in core. Currently the customize-loader is used on the themes admin screen and for the Customize button on the admin dashboard, but it is not technically needed and it adds a lot of complexity which makes browser history in the customizer challenging. The primary gain in using customize-loader is that exiting the customizer is very fast because it just means an iframe is removed from the document. On the themes admin screen, which uses JS for theme search, also manipulates the browser history so returning to the themes admin will already restore the previous search. We'll have to evaluate the pros and cons.

The customize-loader functionality would have to remain for sake of plugins that make use of it.

Change History (8)

#1 @celloexpressions
7 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

I think it's time to retire this functionality. For back-compat, and easy maintenance for any core uses, we can adjust the customize-loader script to navigate the browser to the customizer (acting as a link) rather than opening it in an iframe.

The more important long-term replacement will be a similar script that loads the customizer directly on the frontend.

#2 @westonruter
6 years ago

  • Milestone changed from Future Release to 4.9
  • Owner set to westonruter
  • Status changed from new to accepted
  • Type changed from enhancement to defect (bug)

Let's remove this now from Themes and the Dashboard, as some things just can't work right (like discarding unsaved changes) or wonkily when using customize-loader. It will improve consistency.

#3 @westonruter
6 years ago

  • Keywords has-patch added; needs-patch removed

#4 @celloexpressions
6 years ago

Should customize-loader.js also be refactored to navigate to the customizer automatically, for back compat? If it will no longer work properly, it can probably be reduced to naviating to the link on the element where the class is provided (it currently doesn't work without the link), rather than opening the customizer iframe. That way any third-party instances will continue to be functional without surfacing odd bugs in the customizer itself that result from being loaded via an iframe.

#5 @westonruter
6 years ago

@celloexpressions it will still work, but it won't have the full experience. For example, the logic for dismissing drafted changes upon clicking Close won't currently work with customize-loader (though we could add support into customize-loader itself for this); likewise, the work being done on changeset locking (#42024) won't be able to immediately unlock the changeset when clicking Close. It will still work, bit it wouldn't be 100% the same behavior as loading it directly.

But there are plugins that make use of customize-loader and would break without it being available. For example, Customize Posts currently uses customize-loader to open the Customizer on top of the edit post screen, and if it tried instead to navigate away then it would cause an AYS dialog from the edit post screen. So I intend to continue maintaining support for customize-loader in core for the time being, but potentially add a console.warn() noting that it is deprecated.

I just updated the PR with a couple more changes and todos. Let me know if anything looks amiss.

#6 @westonruter
6 years ago

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

In 41797:

Customize: Eliminate use of customize-loader in core so Customizer is opened consistently in top window.

  • Open the door for future browser history feature in #28536, which is currently not feasible when customize-loader is used.
  • Remove customizer-loader from being used on admin screens for Dashboard, Themes, non-shiny theme install/update.
  • Keep the customize-loader functionality available for plugins, for the time being. It may become deprecated.
  • Ensure return param in customizer links in Themes screen update to reflect search updated by pushState.
  • Persist return when reloading Customizer due to theme switch, autosave restoration, or changeset trashing.
  • Use location.replace() instead of changing location.href when trashing.
  • Hide theme browser while Themes screen is loading when there is a search to prevent flash of unfiltered themes.
  • Use throttling instead of debouncing when searching themes to ensure that screen is updated immediately on page load.
  • Fix encoding and decoding of search param between URL and search field.
  • Add support for dismissing autosaves when closing customize-loader, when it is used by plugins.
  • Skip sending changeset UUID to customize-loader for population in browser location if changeset branching is not enabled.

See #28536.
Fixes #40254.

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

6 years ago

#8 @westonruter
6 years ago

In 42029:

Themes: Switch back from throttling to debouncing in theme searches on admin screen.

Start debouncing after initial search performed when search query param is present to prevent initial "flash of unsearched themes".

Props afercia, westonruter.
Amends [41797].
See #40254.
Fixes #42348.

Note: See TracTickets for help on using tickets.