Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 8 years ago

#19910 closed task (blessed) (fixed)

Appearance Improvements: Theme Customization Frame

Reported by: koopersmith's profile koopersmith Owned by: koopersmith's profile koopersmith
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.3.1
Component: Customize Keywords: dev-feedback needs-docs
Focuses: Cc:

Description

For context, see the appearance umbrella ticket: #19909

Building a frame to house the customization UI and theme preview is the first step to improving the theme customization process. This will be the first iteration for my team (myself and ocean90). Once the frame is in place, we'll add components to the UI one by one (e.g. custom headers, backgrounds, etc).

  1. Add an iframe that can preview any theme (call this the target theme).
  2. Add a sidebar to house future UI.
    1. Temporarily activate the target theme when building the sidebar UI.
  3. Add an API to communicate with the iframe.
    1. Pass data to the iframe.
    2. Refresh the iframe (likely via post).
    3. Refresh the iframe's CSS (via postMessage).
  4. Add a temporary UI to demonstrate the API.
    1. Buttons that refresh and make trivial changes to the iframe.
  5. Add a save action (that, by default, will do nothing).

Attachments (8)

19910.replace-link.patch (4.9 KB) - added by ocean90 12 years ago.
19910.rtl-support.patch (9.4 KB) - added by ocean90 12 years ago.
19910.rtl-support.2.patch (9.4 KB) - added by ocean90 12 years ago.
Refresh
twentyeleven-customize.diff (1.5 KB) - added by Otto42 12 years ago.
Patch for twentyeleven to support postMessage updating of title, description, and header text-color
19910.customizeAfterInstall.diff (1.1 KB) - added by kirasong 12 years ago.
Adds quote to fix "Customize" link after new theme install on Chrome
default_option.diff (1.7 KB) - added by Otto42 12 years ago.
add and use a default_option_ filter
19910.customize-sidebar.diff (1.5 KB) - added by helenyhou 12 years ago.
19910.declare-text-color-as-header-text.patch (483 bytes) - added by ocean90 12 years ago.

Download all attachments as: .zip

Change History (236)

#1 @koopersmith
12 years ago

  • Summary changed from Project Gandalf: Theme Customization Frame to Appearance Improvements: Theme Customization Frame

#2 @ocean90
12 years ago

  • Cc ocean90 added

#3 @koopersmith
12 years ago

  • Status changed from new to accepted

Development is currently taking place via plugin. Check it out here: https://plugins.svn.wordpress.org/gandalf/branches/dev/

#4 @Mamaduka
12 years ago

  • Cc georgemamadashvili@… added

#5 follow-ups: @anonymized_4048828
12 years ago

  • Cc Myatu added

Please ensure to comment the code more, as this addition will affect several plugins (including mine). Think about remove_custom_background() as well.

#6 follow-ups: @nacin
12 years ago

In [19992]:

Add jQuery postMessage v 0.2. Dual licensed MIT/GPL. see #19910.

#7 @koopersmith
12 years ago

In [19995]:

Introduce new theme customizer to replace theme preview. Rough first pass. props koopersmith, ocean90. see #19910.

Merges in http://plugins.svn.wordpress.org/gandalf/branches/dev/ rev 510148.

#8 follow-ups: @azaozz
12 years ago

Some initial thoughts about [19995]:

  • This code can't run on the front-end, wouldn't it be better to be in wp-admin, not in wp-content.
  • Why do we need another JS framework on top of jQuery to handle this? It only makes the code harder to read/understand, brings a lot of complexity and a lot of unneeded features (for example how many instances of the classes and subclasses would we need to show a preview?).
  • Wouldn't it be better to reduce the number of files? As far as I see customize-loader can be added to an existing JS file and customize-base, customize-controls and customize-preview can be merged into one file. Same with the four new php files and two css files.
Last edited 12 years ago by azaozz (previous) (diff)

#9 follow-ups: @ocean90
12 years ago

With the customizer some functions aren't necessary anymore.

  • preview_theme()
  • _preview_theme_template_filter()
  • _preview_theme_stylesheet_filter()
  • preview_theme_ob_filter()
  • preview_theme_ob_filter_callback()

What should we do we with these?

I did a quick search through the plugin repo and there is only one plugin, which is using preview_theme_ob_filter directly:

$ ack 'preview_theme' --type=php

ktai-style/inc/theme.php
530:public static function preview_theme() {
550:	ob_start( 'preview_theme_ob_filter' );

http://wordpress.org/extend/plugins/ktai-style/

#10 in reply to: ↑ 8 ; follow-up: @ocean90
12 years ago

Replying to azaozz:

Some initial thoughts about [19995]:

  • This code can't run on the front-end, wouldn't it be better to be in wp-admin, not in wp-content.

You mean wp-includes? Also the preview iframe is more or less front end.

Wouldn't it be better to reduce the number of files?

For the beginning I think it's fine, we still can combine them later, if needed.

Why do we need another JS framework on top of jQuery to handle this?

You mean postMessage?

---

"jQuery on front end" – we should keep #19979 in mind.

#11 in reply to: ↑ 8 ; follow-ups: @nacin
12 years ago

Replying to azaozz:

Some initial thoughts about [19995]:

I handled the merge of [19995] into core (koopersmith advised, then committed), so I can speak to these decisions.

This code can't run on the front-end, wouldn't it be better to be in wp-admin, not in wp-content.

Yes it can. And in fact, it does, due to how the preview iframe works. There is no actual harm to placing JS in wp-includes over wp-admin. The rule of thumb I have used is that if it is a library of sorts, or decoupled nicely from the admin, or something that could be employed on the frontend (like pointers), it should go into wp-includes. The only things that should go into wp-admin is code intrinsically tied to the admin interface.

Why do we need another JS framework on top of jQuery to handle this? It only makes the code harder to read/understand, brings a lot of complexity and a lot of unneeded features (for example how many instances of the classes and subclasses would we need to show a preview?).

There is not another framework here, nor is there a whole lot of complexity. (I was able to understand what was going on.) I cover the breakdown of JS files.

Wouldn't it be better to reduce the number of files? As far as I see customize-loader can be added to an existing JS file and customize-base, customize-controls and customize-preview can be merged into one file. Same with the four new php files and two css files.

For the JS files, no. They are loaded in different places. customize-loader.js handles the ability to load the customizer. customize-controls gets loaded in the frame that handles the controls. customize-preview gets loaded in the preview iframe. customize-base is a shared base class of both customize-preview and customize-controls. As they are nicely decoupled, they should remain separate.

For the PHP files, no. Individual classes belong in separate files. The fourth file, customize-controls.php, is a special file that is included to handle the controls after WP is hijacked.

Last edited 12 years ago by nacin (previous) (diff)

#12 in reply to: ↑ 9 @nacin
12 years ago

Replying to ocean90:

With the customizer some functions aren't necessary anymore.

  • preview_theme()
  • _preview_theme_template_filter()
  • _preview_theme_stylesheet_filter()
  • preview_theme_ob_filter()
  • preview_theme_ob_filter_callback()

We should probably leave these alone. While you've found no one referencing most of these functions directly, I would not be surprised if many of them are relying on ?template= &stylesheet= for their own particular needs. It's one of those under-the-hood pieces that we probably shouldn't break.

#13 in reply to: ↑ 10 ; follow-ups: @azaozz
12 years ago

Replying to ocean90:

You mean wp-includes? Also the preview iframe is more or less front end.

Yes, sry, meant wp-includes. No, a preview of the front-end only makes sense when the user is in the admin. So it's logical for the code to be in wp-admin.

... You mean postMessage?

No, I meant customize-base.js. It is build as a JS framework which may be useful if the previews were a stand-alone JS application but don't make sense in the current (WP admin only) scope.

#14 in reply to: ↑ 11 ; follow-up: @azaozz
12 years ago

Replying to nacin:

Yes it can. And in fact, it does, due to how the preview iframe works. There is no actual harm to placing JS in wp-includes over wp-admin. The rule of thumb I have used is that if it is a library of sorts, or decoupled nicely from the admin, or something that could be employed on the frontend (like pointers), it should go into wp-includes. The only things that should go into wp-admin is code intrinsically tied to the admin interface.

Yes, in general there's no harm. I was talking about the php and css code too, not only the JS. Imho it doesn't make sense all these files to be in wp-includes as they all are only used when the user is in the admin and are useless on the front-end.

There is not another framework here, nor is there a whole lot of complexity.

Not exactly. The JS is build as a framework on top of jQuery with the actual functions that "do the work" being several levels "deep". Don't see a point in having JS classes for a few jQuery based single-use lines of JS.

For the JS files, no. They are loaded in different places. customize-loader.js handles the ability to load the customizer. customize-controls gets loaded in the frame that handles the controls. customize-preview gets loaded in the preview iframe. customize-base is a shared base class of both customize-preview and customize-controls. As they are nicely decoupled, they should remain separate.

Agreed. Generally loading one larger JS file is better than segmenting it into several small files as after the first load the browsers keep all of it in memory cache. The same is true for the css. However when using JS concatenation the file that is loaded would probably be already different so the browsers would have to load it again.

For the PHP files, no. Individual classes belong in separate files. The fourth file, customize-controls.php, is a special file that is included to handle the controls after WP is hijacked.

I'm not sure it's a good idea to use OOP structures to output HTML in a non-OOP app. There is only one screen so there is only one "instance" of the HTML. What's the point of building specific OOP structure for only this part of the code that is used only once? But that discussion is not for here.

#15 @ocean90
12 years ago

19910.replace-link.patch

  • Removes thickbox enqueue
    • Fixes an issue with theme.js, it doesn't require thickbox (see r15656)
  • Uses data attributes to pass template/stylesheet value

#16 @koopersmith
12 years ago

Before anything, I'd like to note that this is a first pass and certain portions are underdeveloped or unfinished. Please take everything with a grain of salt.

Andrew, I hope this addresses a few of your questions.

Why use a class for WP_Customize?

The WP_Customize class does not just include customize-controls.php. It also handles correctly previewing a theme (in both the admin and front-end), registering and maintaining sections and settings, and properly switching the theme and saving settings.

Why is this in wp-includes?

To preview a theme setting, we need to be able to override it on the front end; the code has to run there. On top of that, it would be fairly easy to alter the WP_Customize class to run completely independent of the admin (if we removed the final keyword, that is).

What's going on in customize-base.js?

There is some underlying sugar there for several reasons. The most prominent is not fully operational yet, which is that there will be an option for the preview to apply changes without completely refreshing the page. This option will be facilitated by postMessage and synchronized values. Instead of writing code to individually watch, bind and synchronize each input, the sugar provides a system to keep all of them in sync with only a few lines of code.

It's important to note that this is JS-only UI. One of the main goals here is to make it easy for plugins and themes to add to the customize controls and preview without having to unbind large portions of core code.

#17 follow-up: @anonymized_4048828
12 years ago

Just some food for thought...

Looking at how sections and settings are added in the WP_Customize class, wouldn't there be a benefit to add a filter to add_setting and add_section. This provides ample opportunity to filter (modify) the respective setting or sections, without having to resort to a later remove_setting?

And this simply nitpicking: Sections and settings are rendered in the order it is saved in their respective arrays (in WP_Customize). Since these are protected, no outside reference is given so the order cannot be changed (nor is there the ability to do so in add_setting or add_section). This would be helpful to move certain sections or settings up and down the list for specific theme customizations (ie., a "Footer" right after the "Header" section, as "Static Front Page" and "Set Title & Tagline" would have little to do with it - makes it a bit more intuitive for the end-user.

#18 @koopersmith
12 years ago

In [20028]:

Theme Customizer: Strip slashes when sanitizing previewed values. see #19910.

#19 follow-up: @koopersmith
12 years ago

In [20030]:

Update customize-controls.php to use new WP_Theme API. see #19910, [20029].

#20 follow-up: @koopersmith
12 years ago

In [20031]:

Improve page loads in the theme customizer by layering loading iframes. Automate refreshing, but debounce multiple refresh events to prevent hammering the server with requests. see #19910.

#21 in reply to: ↑ 17 @koopersmith
12 years ago

Replying to Myatu:

Looking at how sections and settings are added in the WP_Customize class, wouldn't there be a benefit to add a filter to add_setting and add_section. This provides ample opportunity to filter (modify) the respective setting or sections, without having to resort to a later remove_setting?

Settings and sections are objects for just that reason — so that they can be modified without having to be entirely replaced. That said, the add_setting and add_section methods aren't set in stone yet. We'll be moving away from a strict $customize global in favor of dependency injection. I'm not convinced that we'll need a filter there, but let's revisit the idea once things settle a bit.

Replying to Myatu:

And this simply nitpicking: Sections and settings are rendered in the order it is saved in their respective arrays (in WP_Customize).

Settings and sections both contain priority parameters (you can specify these in the constructor or alter them afterwards) which serve as the primary means of sorting sections before they're rendered. The order settings/sections are added serves as a secondary sorting mechanism (tiebreaker) when multiple items share the same priority.

Thanks for the feedback!

#22 follow-up: @koopersmith
12 years ago

In [20035]:

Theme Customizer: Don't switch themes and update settings when the enter key is pressed. see #19910.

#23 follow-ups: @koopersmith
12 years ago

In [20034]:

Theme Customizer: Trigger UI updates only when necessary.

#24 follow-up: @cais
12 years ago

Replying to nacin:

It's not being executed, only defined, and it is quite small.

Right. It just adds a tiny bit of memory on every hit to the front-end and a tiny bit on every function_exists() call. There already are quite a few functions in wp-includes that are defined but cannot be used, was only hoping we won't be adding more.

Probably because the handlers for Plupload is not exactly a well-formed API.

Plupload is a relatively "young" JS library and the APIs are changing quite a bit from release to release. Proxying the settings seems less desirable than simply including the settings object as outputted from wp_plupload_default_settings() and letting plugins add the hook names/IDs for their HTML.

Also the current integration for the "main" uploader is mostly hard-coded. Was hoping to move it to the new API but that can wait for 3.5 :)

Last edited 12 years ago by azaozz (previous) (diff)

#25 in reply to: ↑ 24 ; follow-up: @koopersmith
12 years ago

Replying to cais:

This may be more observation and be a result of coder error but I found the following to cause the Customization > Navigation to not produce the results you would expect...

I'll look into this. If I'm reading this correctly, is the only difference that the nav menu registration is attached to init instead of after_setup_theme?

#26 @koopersmith
12 years ago

In [20049]:

Theme Customizer: Bind the preview iframe load event when triggering refresh instead of when creating the iframe. see #19910.

The loaded event should only fire when we've actually triggered a refresh. If loaded is bound when the iframe is created, sometimes the blank iframe will fire a 'load' event before a refresh is triggered (this occurred with relative frequency in firefox). By binding the loaded event in the refresh function, we prevent this from occurring.

#27 follow-ups: @cais
12 years ago

Replying to koopersmith:

Replying to cais:

This may be more observation and be a result of coder error but I found the following to cause the Customization > Navigation to not produce the results you would expect...

I'll look into this. If I'm reading this correctly, is the only difference that the nav menu registration is attached to init instead of after_setup_theme?

No, not quite ... here is a link to the actual functions.php file for Desk Mess Mirrored. It may be easier to see the code in its unadulterated form: https://github.com/Cais/desk-mess-mirrored/blob/master/functions.php

Starting at line 248 is the desk_mess_mirrored_setup function that is used with the 'after_theme_setup' hook, while inside that function starting at line 365 is the 'register_nav_menu' function call which uses the 'init' hook ...

... the more I look at this the more I think using the 'init' hook this way is not correct and could just be awkward code catching up to me. Although I have not seen any issues with this "menu code" since I implemented it back around WP 3.1.

#28 @koopersmith
12 years ago

In [20051]:

Theme Customizer: Load the preview iframe with the same scheme as the admin to prevent security warnings. props ocean90. fixes #20137, see #19910.

#29 @koopersmith
12 years ago

In [20055]:

Theme Customizer: Use a late priority when registering customize settings to ensure we catch any theme settings added earlier on init (such as nav menus). see #19910.

#30 in reply to: ↑ 27 ; follow-ups: @nacin
12 years ago

Replying to cais:

Starting at line 248 is the desk_mess_mirrored_setup function that is used with the 'after_theme_setup' hook, while inside that function starting at line 365 is the 'register_nav_menu' function call which uses the 'init' hook ...

Nesting function calls in PHP is not very intuitive or easy to read, and since they all end up in global scope, it's functionally the same. Koopersmith is correct when he says "If I'm reading this correctly, is the only difference that the nav menu registration is attached to init instead of after_setup_theme?" That is exactly what ends up happening here.

... the more I look at this the more I think using the 'init' hook this way is not correct and could just be awkward code catching up to me. Although I have not seen any issues with this "menu code" since I implemented it back around WP 3.1.

Correct. init is the wrong hook. after_setup_theme is the only valid hook to use for custom menus, custom backgrounds, custom headers, post thumbnails, etc. The init hook does work in those cases, nonetheless.

[20055] should consider wp_loaded — init should probably be given a chance to finish.

#31 in reply to: ↑ 30 ; follow-up: @koopersmith
12 years ago

Replying to nacin:

[20055] should consider wp_loaded — init should probably be given a chance to finish.

This got me thinking... since the customize settings won't be initialized during init, anything that grabs a theme value at that point won't receive the overridden value. That's bad. We need to split up the initialization and make things as JIT as possible to ensure accurately preview-able values.

#32 @koopersmith
12 years ago

In [20057]:

Theme Customizer: Use home_url instead of get_home_url. props nacin, fixes #20137, see #19910.

#33 @koopersmith
12 years ago

In [20058]:

Theme Customizer: Use wp_loaded for customize setting registration. see #19910.

#34 in reply to: ↑ 31 @koopersmith
12 years ago

Replying to koopersmith:

Replying to nacin:

[20055] should consider wp_loaded — init should probably be given a chance to finish.

This got me thinking... since the customize settings won't be initialized during init, anything that grabs a theme value at that point won't receive the overridden value. That's bad. We need to split up the initialization and make things as JIT as possible to ensure accurately preview-able values.

Updated to use wp_loaded in [20058]. Nacin, Ryan, and I will revisit this, add_setting, and the like early next week.

#35 @Mamaduka
12 years ago

When SCRIPT_DEBUG is fasle, we get wrong url for arrows.png

#36 in reply to: ↑ 30 @cais
12 years ago

Replying to nacin:

Replying to cais:
Correct. init is the wrong hook. after_setup_theme is the only valid hook to use for custom menus, custom backgrounds, custom headers, post thumbnails, etc. The init hook does work in those cases, nonetheless.

[20055] should consider wp_loaded — init should probably be given a chance to finish.

Thanks, I'll correct the theme code ... glad this was able to help sort out an issue that had not been addressed, yet.

#37 follow-up: @soulseekah
12 years ago

Back button should close Theme Customizer rather than return to a previous page.

#38 @koopersmith
12 years ago

In [20106]:

Theme Customizer: Improve form markup. Make the theme title/author a section. Prevent section arrows from blocking clicks. see #19910.

#39 @koopersmith
12 years ago

In [20108]:

Theme Customizer: Add mini screenshot and description to theme info section. see #19910.

#40 @koopersmith
12 years ago

In [20110]:

Theme Customizer: Improve markup and CSS for the return/collapse actions and the action bars within the customize controls form. see #19910.

#41 @koopersmith
12 years ago

In [20120]:

Theme Customizer: Add working color pickers. First pass. see #19910.

#42 @koopersmith
12 years ago

In [20121]:

Theme Customizer: Ensure color pickers initialize to the correct color. see #19910.

#43 @koopersmith
12 years ago

In [20123]:

Theme Customizer: First pass at using postMessage for background color. Fix instance where preview.targetWindow would become inaccurate. Initialize setting values in customize-preview.js. see #19910.

#44 @WraithKenny
12 years ago

  • Cc Ken@… added

#45 @koopersmith
12 years ago

In [20128]:

Theme Customizer: Add a Control object to better encapsulate different UI elements and make it easy to switch between hard refreshes and postMessage. see #19910.

#46 @koopersmith
12 years ago

In [20130]:

Theme Customizer: Color picker markup/CSS improvements. Part 1. see #19910.

#47 @koopersmith
12 years ago

In [20133]:

Theme Customizer: Move contents of customize.loader.css to wp-admin.css. Make markup/CSS for the full-screen overlay modular. see #19910.

#48 @koopersmith
12 years ago

In [20138]:

Theme Customizer: Begin integration into the install process. Combine previews, details, and install into a single workflow. see #19910.

#49 @koopersmith
12 years ago

Whoops, props chexee and hugobaeta on [20138].

#50 @koopersmith
12 years ago

In [20140]:

Theme Customizer: Make theme installer no-js compatible. Fix JS click handler that caught clicks on the 'Activate' link. Relocate display_theme() to WP_Theme_Install_List_Table->single_row(). see #19910.

#51 @nacin
12 years ago

In [20141]:

Deprecate display_theme(). see #19910.

#52 @hd-J
12 years ago

  • Cc jeremy@… added

#53 @drecodeam
12 years ago

  • Cc drecodeam added

Reported as a new ticket here: #20452

Seeing a strange issue in Opera 11.62 (Opera/9.80 (Macintosh; Intel Mac OS X 10.7.3; U; en) Presto/2.10.229 Version/11.62 ), Clicking links within the preview frame is causing the Customizer to mostly disappear, working fine in Safari however.

The Close button remains, and the frame is still loaded in the background somewhere I think..

Example of what I'm seeing:

http://cl.ly/062H3g102l1j3W0s0j1g/content

Last edited 12 years ago by dd32 (previous) (diff)

#54 @koopersmith
12 years ago

In [20179]:

Theme Customizer: First pass for upload controls, using background image as an example. Add a wrapper for Plupload that allows for custom upload UIs. see #19910.

wp.Uploader is a wrapper that provides a simple way to upload an attachment (using the wp_ajax_upload_attachment handler). It is intentionally decoupled from the UI. When an upload succeeds, it will receive the attachment information (id, url, meta, etc) as a JSON response. If the upload fails, the wrapper handles both WordPress and plupload errors through a single handler.

As todos, we should add drag classes for the uploader dropzone and account for the rough 100mb filesize limit in most browsers. The UI for the customizer upload controls could be improved as well.

#55 follow-ups: @koopersmith
12 years ago

Note: I reviewed [20179] with Nacin, Ryan, and Mark in person at SXSW. Feedback welcome.

#56 @koopersmith
12 years ago

In [20181]:

Theme Customizer: Improve sidebar CSS. Display most elements inline, refine padding/line-heights. see #19910.

#57 @koopersmith
12 years ago

In [20182]:

Theme Customizer: Move section descriptions to the title attribute of the section name. see #19910.

#58 @koopersmith
12 years ago

In [20184]:

Theme Customizer: Tweak radio control padding. see #19910.

#59 @koopersmith
12 years ago

In [20185]:

Theme Customizer: More sidebar CSS tweaks. Padding and margins. see #19910.

#60 @nacin
12 years ago

In [20187]:

Ensure no nonce or multipart_params get passed to the plupload_default_settings filter. see #19910.

#61 @nacin
12 years ago

In [20188]:

URL to admin-ajax.php should be relative. see #19910. see #18952.

#15 @nacin
12 years ago

In [20189]:

Move wp_plupload_default_settings() to the admin_enqueue_scripts hook to prevent fatal errors when admin_init is run outside of normal admin-header execution, like admin-ajax. see #19910. fixes #20240.

#16 @koopersmith
12 years ago

In [20197]:

Theme Customizer: Bind wp_plupload_default_settings() to the customize_controls_enqueue_scripts hook. Fixes JS errors. see #19910, [20189].

#17 follow-up: @nacin
12 years ago

In [20210]:

Theme Customizer: In customize_control_dropdown_pages(), remove bad translation. Replace with proper customize-control- markup. see #19910.

#18 @nacin
12 years ago

In [20230]:

Remove enqueue of wp_plupload_default_settings() from admin_enqueue_scripts(). For now, it needs to be enqueued selectively when needed. see #19910.

#19 follow-up: @nacin
12 years ago

In [20231]:

Callbacks for custom headers and custom backgrounds registered through add_theme_support() are now wp-head-callback, admin-head-callback, and admin-preview-callback. see #19910.

#20 in reply to: ↑ 19 ; follow-up: @nacin
12 years ago

Replying to nacin:

In [20231]

I was in auto-pilot. That belonged on #20249.

#21 @koopersmith
12 years ago

In [20232]:

Theme Customizer: Improve WP_Customize_Setting->check_capabilities(). Add support for arrays in WP_Customize_Setting->capability and WP_Customize_Setting->theme_supports. Don't bail when WP_Customize_Setting->capability is empty. see #19910.

#22 in reply to: ↑ 55 ; follow-up: @azaozz
12 years ago

Replying to koopersmith:

Note: I reviewed [20179] with Nacin, Ryan, and Mark in person at SXSW. Feedback welcome.

Some quick feedback:

  • Moving the settings for plupload to a separate function is nice, well done.
  • Having that function defined every time a visitor or a bot hits the front-end is not (this is a part of a bigger problem that eventually will have to be solved: there are a lot of functions in wp-includes that cannot be used on the front-end unless the user is logged in and have certain privileges. 99%+ of the hits on the front-end fall into this category).
  • Not sure there is a need for wp-plupload.js at all, at least not in it's current form. Why is it needed to proxy the settings for an external library? Perhaps it would be clearer if wp_plupload_default_settings() actually contained the simple function needed to initialize plupload instead of needing yet another JS file.

#23 follow-ups: @nacin
12 years ago

Having that function defined every time a visitor or a bot hits the front-end is not

It's not being executed, only defined, and it is quite small.

Not sure there is a need for wp-plupload.js at all, at least not in it's current form. Why is it needed to proxy the settings for an external library?

Probably because the handlers for Plupload is not exactly a well-formed API.

#24 in reply to: ↑ 23 ; follow-up: @azaozz
12 years ago

Replying to nacin:

It's not being executed, only defined, and it is quite small.

Right. It just adds a tiny bit of memory on every hit to the front-end and a tiny bit on every function_exists() call. There already are quite a few functions in wp-includes that are defined but cannot be used, was only hoping we won't be adding more.

Probably because the handlers for Plupload is not exactly a well-formed API.

Plupload is a relatively "young" JS library and the APIs are changing quite a bit from release to release. Proxying the settings seems less desirable than simply including the settings object as defined in wp_plupload_default_settings() and letting plugins add the hook names/IDs for their HTML.

Also the current integration for the "main" uploader is mostly hard-coded. Was hoping to move it to the new API but that can wait for 3.5 :)

#25 follow-up: @nacin
12 years ago

Right. It just adds a tiny bit of memory on every hit to the front-end and a tiny bit on every function_exists() call. There already are quite a few functions in wp-includes that are defined but cannot be used, was only hoping we won't be adding more.

I don't see a function_exists() call here. But this function *can* be used on the front-end — see also the ajax upload-attachment action.

#26 in reply to: ↑ 25 @azaozz
12 years ago

Replying to nacin:

I don't see a function_exists() call here.

What I mean is that every function_exists() call that runs on the front end will have to check one more function name which in PHP is always global.

#27 in reply to: ↑ 37 ; follow-ups: @kirasong
12 years ago

  • Cc mike.schroder@… added

Replying to soulseekah:

Back button should close Theme Customizer rather than return to a previous page.

Unsure if this is being worked on, but that has been bothering me as well.
I find myself clicking to preview or customize themes, then clicking the back button in the browser, and ending up on a different page than expected.

#28 in reply to: ↑ 27 @anonymized_4048828
12 years ago

Replying to DH-Shredder:

Replying to soulseekah:

Back button should close Theme Customizer rather than return to a previous page.

Unsure if this is being worked on, but that has been bothering me as well.
I find myself clicking to preview or customize themes, then clicking the back button in the browser, and ending up on a different page than expected.

Indeed, I've caught myself doing that too. However, I think that the UI should be changed a little so it is clear to the end user (s)he is viewing a modal window over top the Themes screen, akin the existing Theme Preview.

This would also give it more consistency with the current UI, which it is currently lacking. And if screen real estate becomes an issue with a slightly smaller window, provide the option for the end user to expand the modal window to "full screen".

It should be fairly straight forward to implement, given the customization is already inside a div container. Modifying the behaviour of the 'Back' button on the other hand is a technical mess.

#29 @koopersmith
12 years ago

In [20248]:

Theme Customizer: Numerous API refinements and bugfixes. Add a theme_supports check for header_textcolor. see #19910.

  • prepare_controls() now removes any settings and sections that return false for check_capabilities().
  • Added maybe_render() methods to both settings and sections that call the protected render() methods.
  • Stop firing front-end preview functionality when rendering the controls.
  • Merged the WP_Customize_Setting->_render_type() method into WP_Customize_Setting->render().
  • Removed the 'customize_render_control-' hook; use 'customize_render_setting' instead.
  • Added a property to sections and settings so they no longer rely on the global. Hooray for dependency injection.
  • Shifted calls to WP_Customize_Setting->enqueue() to the 'customize_controls_enqueue_scripts' action.
  • Added a theme_supports check for the header_textcolor setting.

#30 follow-ups: @koopersmith
12 years ago

In [20250]:

Theme Customizer: Do not attempt to grab the contents of the preview iframe to populate the window title. Reverts plugins repo revision 508947. see #19910.

#31 follow-up: @koopersmith
12 years ago

In [20251]:

Theme Customizer: If a section has no settings, prevent it from being rendered. see #19910.

#32 @koopersmith
12 years ago

In [20253]:

Theme Customizer: Plupload does not support objects in its multipart_params setting (which causes exceptions in Firefox). Use multidimensional keys to circumnavigate this issue. see #19910.

#33 @koopersmith
12 years ago

In [20254]:

Theme Customizer: Make dropdown-pages a native customize control. see #19910.

While the customize_render_control- action has been removed, we could still accomplish this with the customize_render_setting action. That said, in this case, avoiding native integration was a matter of minor semantics that ended in the same result.

#34 @koopersmith
12 years ago

In [20257]:

Theme Customizer: In JavaScript, add the ability to execute a callback once a set of values have been initialized. see #19910.

#35 @koopersmith
12 years ago

In [20259]:

Theme Customizer: Ensure background_color and background_image exist instead of blindly attempting to access their controls. see #19910.

#36 @koopersmith
12 years ago

In [20260]:

Theme Customizer: Add a WP_Customize_Setting->visibility parameter to show/hide a control based upon the value of another control. Also shifts rendering the setting wrapper element into WP_Customize_Setting->render() and adds WP_Customize_Setting->render_content(). see #19910.

#37 follow-up: @koopersmith
12 years ago

In [20261]:

Theme Customizer: Only show uploader 'remove' links when there is an image to remove. see #19910.

#38 @koopersmith
12 years ago

In [20263]:

Theme Customizer: Add background repeat, position, and attachment settings. Change visibility parameter to accept a string or array( , ). see #19910.

#39 @koopersmith
12 years ago

In [20266]:

Theme Customizer: Use a more complex name for the save parameter to prevent the customizer from clashing with plugins. see #19910.

#40 @andyadams
12 years ago

  • Cc aadams@… added

#41 @koopersmith
12 years ago

In [20274]:

Theme Customizer: Change 'Random Image' control to be a proper 'Header Image' control. First step to integrating header images. see #19910.

#42 @koopersmith
12 years ago

In [20276]:

Theme Customizer: Add WP_Customizer_Setting->control_params, and wp.customize.Control.params to allow settings to pass arbitrary parameters to controls. Add the 'context' parameter to wp.customize.UploadControl. see #19910.

#43 @koopersmith
12 years ago

In [20278]:

Theme Customizer: First pass at image controls. Use header_image as the initial case. Add a 'removed' control param for upload/image controls to map 'removed' to a value other than the empty string. see #19910.

#44 @koopersmith
12 years ago

In [20290]:

Theme Customizer: Add 'choose image' functionality to image controls. Rough first pass, using header images as an example. see #19910.

#45 @koopersmith
12 years ago

In [20295]:

Create WP_Customize_Control to separate the process of rendering a control from fetching, previewing, and saving its values. see #19910.

Many-to-many mapping between settings and controls.

  • Settings and controls have been separated in both the PHP (WP_Customize_Setting, WP_Customize_Control) and the JS (wp.customize.Setting, wp.customize.Control).
  • While most settings are tied to a single control, some require multiple controls. The 'header_textcolor' control is a good example: to hide the header text, header_textcolor is set to 'blank'.

Add 'Display Header Text' control.

A handful of miscellaneous bugfixes along the way.

Notes:

  • Controls should be separated out a bit more; juggling type-specific arguments in the switch statement is rather inelegant.
  • Page dropdowns are currently inactive and need to be re-linked.

#46 @koopersmith
12 years ago

In [20297]:

Theme Customizer: Fix postMessage for background_color. The method parameter affects settings, not controls. see #19910.

#47 @koopersmith
12 years ago

In [20299]:

Theme Customizer: Sanitize selectors by transforming square brackets into dashes. see #19910.

This both better conforms to the CSS spec and prevents bugs from occurring in JavaScript selector engines.

#48 @koopersmith
12 years ago

In [20300]:

Theme Customizer: Properly pass arguments by reference to WP_Customize_Setting->multidimensional(). see #19910, [20136], #20163.

This is necessary for WP_Customize_Setting->multidimensional_replace() to work properly on multidimensional arrays (which was, as the name indicates, the point of the function in the first place).

#49 @koopersmith
12 years ago

In [20301]:

Theme Customizer: Properly bind the 'upload new' and 'remove image' actions in the image picker. see #19910.

This occurred because we weren't calling UploadControl.ready (where the actions are declared and the uploader is initialized) in ImageControl.ready.

#50 @koopersmith
12 years ago

In [20302]:

Theme Customizer: Add data binding to page dropdown controls. Add WP_Customize_Control->get_link() to return the data attribute string. see #19910.

#51 @koopersmith
12 years ago

In [20303]:

Theme Customizer: Move 'Site Title/Tagline' section to the top of the list. Add specific priorities to all sections. see #19910.

#52 @koopersmith
12 years ago

In [20306]:

Theme Customizer: Fire WP_Customize->customize_preview_init on wp_loaded instead of template_redirect. props nacin, see #19910.

Certain customize settings (such as show_on_front and other static front page features) need to be overridden before template_redirect.

#53 @dd32
12 years ago

Seeing a strange issue in Opera 11.62 (Opera/9.80 (Macintosh; Intel Mac OS X 10.7.3; U; en) Presto/2.10.229 Version/11.62 ), Clicking links within the preview frame is causing the Customizer to mostly disappear, working fine in Safari however.

The Close button remains, and the frame is still loaded in the background somewhere I think..

Example of what I'm seeing:

http://cl.ly/062H3g102l1j3W0s0j1g/content

#54 @koopersmith
12 years ago

In [20319]:

Theme Customizer: Move upload and image controls to subclasses. see #19910.

Instead of grouping together every control parameter into a single array and passing them all to the JS, use subclasses with custom parameters to implement additional PHP functionality and the parameter to send only the necessary variables to the JavaScript control.

Replaces WP_Customize_Control->control_params with WP_Customize_Control->json and WP_Customize_Control->to_json(). The to_json() method refreshes the json array passed to the JavaScript control (set to control.param by default).

Creates WP_Customize_Upload_Control and WP_Customize_Image_Control.

#55 follow-ups: @soulseekah
12 years ago

Should browser back button close Theme Customizer rather than return to a previous page?

#56 in reply to: ↑ 55 @jane
12 years ago

Replying to soulseekah:

Should browser back button close Theme Customizer rather than return to a previous page?

It should return to Install Themes. See #20337.

#57 @koopersmith
12 years ago

In [20344]:

Theme Customizer: Improve data binding in wp.customize.Value and wp.customize.Values. see #19910.

  • Replace the convoluted wp.customize.Value.link method with a simple shortcut for direct binding.
  • Add wp.customize.Value.sync for bidirectional linking.
  • Add wp.customize.Value.setter for handling compound values (instead of using wp.customize.Value.link).

#58 @koopersmith
12 years ago

In [20352]:

Theme Customizer: Improve the customize loader and themes list table markup. see #19910.

  • Use event delegation for triggering the customize loader (so we play nicely with infinite scroll).
  • Use data attributes on .load-customize links instead of parsing the href.
  • Properly translate the 'Customize' string instead of injecting the replacement with JS.

#59 @koopersmith
12 years ago

In [20354]:

Theme Customizer: Add a link to customize the current theme. see #19910.

#60 @koopersmith
12 years ago

In [20391]:

Theme Customizer: Move 'collapse' button to footer and add a visible text label. see #19910.

#61 @koopersmith
12 years ago

In [20392]:

Theme Customizer: String change to clarify choosing a static page (remove 'select below'). see #19910.

#62 @koopersmith
12 years ago

In [20393]:

Theme Customizer: Remove small screenshot from theme title bar. see #19910.

#63 follow-up: @ocean90
12 years ago

I think we should prevent a direct access to /wp-admin/admin.php?template=twentyten&stylesheet=twentyten&customize=on

#64 @ocean90
12 years ago

19910.rtl-support.patch does add RTL support for the customizer.

#65 @koopersmith
12 years ago

In [20404]:

Theme Customizer: Add a 'previewing' notice by the theme name. Reduce padding and size on section titles, add a border between section titles and content to better indicate the title as a click target. see #19910.

#66 @anonymized_4048828
12 years ago

In class WP_Customize_Setting, the preview() function does not pass back the post_value() in the customize_preview_ action, although action in the update() function.

Although I can still obtain the post_value() for the setting by jumping through a few hoops, it doesn't make much sense not to return the value in the customize_preview_ action instead.

#67 follow-up: @anonymized_4048828
12 years ago

Looking at the update() function, it uses $this->type opposed to $this->id. So action customize_save_ should be used instead - my bad. Still doesn't pass back the value though :(

#68 in reply to: ↑ 67 @koopersmith
12 years ago

Replying to Myatu:

Looking at the update() function, it uses $this->type opposed to $this->id. So action customize_save_ should be used instead - my bad. Still doesn't pass back the value though :(

You're correct — after looking through the actions and filters, a few names/arguments are inconsistent. I'll try to simplify things a bit.

#69 in reply to: ↑ 63 @koopersmith
12 years ago

Replying to ocean90:

I think we should prevent a direct access to /wp-admin/admin.php?template=twentyten&stylesheet=twentyten&customize=on

I disagree — having the frame accessible by URL is very useful for debugging and does not significantly restrict or alter the experience.

@ocean90
12 years ago

Refresh

#70 @koopersmith
12 years ago

In [20456]:

Theme Customizer: RTL styles. props ocean90. see #19910.

#71 @koopersmith
12 years ago

In [20476]:

Theme Customizer: Allow the customize iframe to be accessed directly (with full feature support). see #19910.

  • Move the 'Return to Manage Themes' and 'Collapse Sidebar' actions from themes.php to customize-controls.php.
  • Create a postMessage connection between themes.php and customize-controls.php.
  • Allow the theme customizer to be accessed directly (independent of themes.php and the customize loader).
  • Add wp_customize_href() and wp_customize_url().
  • Remove wp_customize_loader(). To include the loader, use wp_enqueue_script( 'customize-loader' ).
  • The theme customizer now requires postMessage browser support.
  • Add .hide-if-customize and .hide-if-no-customize CSS classes.
  • Clean up customize-preview.js.

#72 @nacin
12 years ago

In [20477]:

Theme Customizer: Stick to wp_customize_url() instead of wp_customize_href(). Switch argument order so it is stylesheet-template. (Template is hypothetically optional, but the function will not support that.) see #19910.

Move to ->display('Name'), as ->get('Name') is a raw, untranslated version of the header.

#73 @koopersmith
12 years ago

In [20479]:

Theme Customizer: If the customize loader is enqueued, then add the 'customize-support' class early. This prevents a flash of unstyled content. see #19910.

#74 @Otto42
12 years ago

  • Cc Otto42 added

#75 @koopersmith
12 years ago

In [20488]:

Theme Customizer: Integrate with browser history. Use window.history by default, with window.onhashchange as a fallback. fixes #20337, see #19910.

#76 @koopersmith
12 years ago

In [20495]:

Theme Customizer: Display 'Save and Activate' when switching themes. Add and properties to the WP_Customize class. see #19910.

#77 @nacin
12 years ago

In [20496]:

Move to admin.php?customize=on&theme=$stylesheet, rather than juggling both template and stylesheet values. see #19910.

Combine the setup_theme() and customize_previewing() methods. Remove the set_template() and set_stylesheet() methods. Add set_theme() method to WP_Customize to store the working WP_Theme object. We will use this for the stylesheet and template.

Use the WP_Theme display() method when preparing headers for display, not get() or the deprecate properties.

#78 @koopersmith
12 years ago

In [20497]:

Theme Customizer: Improve color picker toggle styles and markup. see #19910.

#79 @koopersmith
12 years ago

In [20504]:

Theme Customizer: Improve markup/styles for opened color picker. see #19910.

#80 @koopersmith
12 years ago

In [20506]:

Remove 'visibility' parameter from WP_Customize_Control. Handle control visibility in JS instead. see #19910.

  • Have the header text color picker display only when header_textcolor != 'blank'

#81 @koopersmith
12 years ago

In [20507]:

Theme Customizer: Remove redundant call to wp_enqueue_script. see #19910.

#82 @koopersmith
12 years ago

In [20510]:

Theme Customizer: Color control hover styles. see #19910.

#83 @koopersmith
12 years ago

In [20517]:

Theme Customizer: Use native postMessage for wp.customize.Messenger. see #19910.

  • Removes use of jquery.postmessage.js
  • Fixes bug where Opera would attempt to use the hash transport and redirect to the dashboard.
  • Fixes bug where multiple postMessage connections could not coexist in a single frame.

#84 @koopersmith
12 years ago

In [20518]:

Remove jQuery postMessage plugin, as it is no longer used (as of [20517]). see #19910.

#85 @koopersmith
12 years ago

In [20520]:

Theme Customizer: Remove unused 'template' argument from wp_customize_url. see #19910.

#86 @koopersmith
12 years ago

In [20530]:

Theme Customizer: Box model fixes for image picker thumbnails. see #19910.

#87 @ciobi
12 years ago

  • Cc alex.ciobica@… added

#88 @koopersmith
12 years ago

In [20545]:

Theme Customizer: Improve image picker control. see #19910.

Overhauled image pickers:

  • Add support for drag and drop uploads to image controls.
  • Improve the 'uploaded' tab in image controls: automatically add images uploaded during the current session, hide the tab when no uploaded images exist.
  • Move the header image control to the WP_Customize_Header_Image_Control class. Remove wp_customize_print_uploaded_headers() and wp_customize_print_uploaded_headers() functions.
  • Abstract the dropdown functionality from the color picker to the .dropdown class.
  • In wp.Uploader, unset element keys if passed an empty jQuery collection.

#89 @koopersmith
12 years ago

In [20547]:

Theme Customizer: Firefox CSS love. props helenyhou, see #19910.

#90 @koopersmith
12 years ago

In [20572]:

Theme Customizer: Simplify logic when preparing menu controls. Include instructions to edit menu content in section description. Add string for default state. see #19910.

#91 @ryan
12 years ago

  • Component changed from Themes to Customizer

#92 @koopersmith
12 years ago

In [20584]:

Theme Customizer: Preview the target theme during save to ensure all settings are properly registered. Temporarily revert to the active theme when running switch_theme. fixes #20508, see #19910.

Introduces WP_Customize->start_previewing_theme() and WP_Customize->stop_previewing_theme() to easily enable/disable the theme filters as needed.

#4 @koopersmith
12 years ago

In [20585]:

Theme Customizer: For clarity, wp.customize.Setting.method to wp.customize.Setting.transport. Add WP_Customize_Setting->transport to allow setting the transport method via PHP. see #19910.

#5 follow-ups: @koopersmith
12 years ago

In [20598]:

Theme Customizer: Add statuses to the color and image controls. see #19910.

Move the color control from the switch statement to WP_Customize_Color_Control.
Markup improvements.

#6 follow-ups: @anonymized_4048828
12 years ago

CSS: Controls in the content column, ie .customize-control-select select make use of fixed widths, which do not account for a scroll-bar that may appear if the expanded content exceeds the height of the browser window.

Also, the scroll bar looks wonky in Chrome (not tested in others as of yet), as shown below (scrolled all the way to the top):

http://i48.tinypic.com/1z5156v.jpg

#7 @koopersmith
12 years ago

In [20645]:

Theme Customizer: Migrate to an ajax-based solution for refreshing the preview and saving. see #20507, #19910.

  • Use ajax-based saving, add saving indicator.
  • Use ajax-based refreshing instead of form targets.
  • Instead of using hidden inputs with prefixed names to track the canonical data, use the values stored in wp.customize. Encode the values as JSON before sending to avoid bugs with ids that contain square brackets (PHP mangles POST values with nested brackets).
  • Use wp.customize.Previewer solely for the purpose of previewing; move the postMessage connection with the parent frame and other unrelated code snippets into the 'ready' handler.

#8 in reply to: ↑ 6 ; follow-ups: @koopersmith
12 years ago

Replying to Myatu:

CSS: Controls in the content column, ie .customize-control-select select make use of fixed widths, which do not account for a scroll-bar that may appear if the expanded content exceeds the height of the browser window.

Also, the scroll bar looks wonky in Chrome (not tested in others as of yet), as shown below (scrolled all the way to the top)

Thanks for the report Myatu. I've noticed both of these issues, but haven't been able to address them just yet. Hopefully this'll be fixed up soon — patches welcome. :)

#9 follow-ups: @Otto42
12 years ago

Is there an easy way for code to change the transport value of an existing setting?

For example, I tried to modify a theme to call $sett = $wp_customize->get_setting('blogname'), and then to just set the transport on that object with $sett->transport = 'postMessage'.

However, it gives me back a 500 error. Presumably that's protected somewhere along the line. I can see the value there if I dump it, but attempting to change it = death.

Calling remove_setting and then readding it with the postMessage transport does work, but is obviously less than ideal.

I'm trying to do all this at the customize_register action hook.

#10 in reply to: ↑ 9 ; follow-up: @Otto42
12 years ago

Replying to Otto42:

Is there an easy way for code to change the transport value of an existing setting?

Never mind. I was being stupid. This code works fine when hooked to customize_register:

$sett = $wp_customize->get_setting('blogname');
$sett->transport='postMessage';

@Otto42
12 years ago

Patch for twentyeleven to support postMessage updating of title, description, and header text-color

#11 follow-ups: @Otto42
12 years ago

Not sure if that patch belongs in this ticket or not, but it adds some functionality to twentyeleven to make the blog title, description, and header-textcolor use postMessage updating instead of refresh updating. Looks nicer, faster, easier to use. Shows how a theme can use knowledge of itself to make things happen smoother. Might not necessarily be the "right" way.

#12 in reply to: ↑ 11 @nacin
12 years ago

Replying to Otto42:

Not sure if that patch belongs in this ticket or not, but it adds some functionality to twentyeleven to make the blog title, description, and header-textcolor use postMessage updating instead of refresh updating. Looks nicer, faster, easier to use. Shows how a theme can use knowledge of itself to make things happen smoother. Might not necessarily be the "right" way.

Nice. We have #20448 for Twenty Ten and Twenty Eleven.

#13 follow-ups: @koopersmith
12 years ago

In [20649]:

Theme Customizer: Pass the WP_Customize instance to all actions fired inside the class. Plugins/themes should not refer to the $wp_customize global. see #19910, #20448.

#14 in reply to: ↑ 13 ; follow-up: @jackmahoney
12 years ago

  • Keywords dev-feedback reporter-feedback needs-docs added

Replying to koopersmith:

In [20649]:

Theme Customizer: Pass the WP_Customize instance to all actions fired inside the class. Plugins/themes should not refer to the $wp_customize global. see #19910, #20448.

Could you please elaborate on how a theme developer can call functions on WP_Customize instance? How does one instantiate it? I am using a fork of _s' theme and the standard Gandalf features work fine but I cannot find how to add my own.

add_action( 'customize_register' , 'my_customization_function');
function my_customization_function(){
                global $wp_customize;
                //But $wp_customize is null!
		$wp_customize->add_section( 'layout', array(
			'title'          => __( 'Layout' ),
			'priority'       => 20,
		) );
}

Your help is greatly appreciated!

@kirasong
12 years ago

Adds quote to fix "Customize" link after new theme install on Chrome

#15 @jackmahoney
12 years ago

Actually don't worry figured it out, will post answer shortly.

#16 @kirasong
12 years ago

I'm sure you will likely want to make further improvements to the look/feel of the post-install options anyway, but 19910.customizeAfterInstall.diff adds a quote to make the Customize link appear properly after installing a theme.

#17 follow-up: @ocean90
12 years ago

Related: #20585, #20582

#18 in reply to: ↑ 14 @Otto42
12 years ago

Replying to jackmahoney:

Could you please elaborate on how a theme developer can call functions on WP_Customize instance? How does one instantiate it? I am using a fork of _s' theme and the standard Gandalf features work fine but I cannot find how to add my own.

Your help is greatly appreciated!

jack: I'll be writing a tutorial on this soon, but for the moment some of the implementation details are still being finalized. So anything we document now may be out of date by release.

For the moment, take a look at the patches I've made in Ticket #20448 for the twentyeleven theme. This shows how to implement custom stuff with the current code.

#19 follow-up: @koopersmith
12 years ago

In [20695]:

Add quote to fix 'Customize' link after new theme install on Chrome. props DH-Shredder, see #19910.

#20 follow-up: @ocean90
12 years ago

  • Keywords reporter-feedback removed

We should put all our customize-*.js files into one folder named wp-customizer or something like that. Only for a better overview.

#21 @Otto42
12 years ago

Question: Why is this code in the WP_Customize_Setting->preview() function?

case 'option' :
	if ( empty( $this->id_data[ 'keys' ] ) )
		add_filter( 'pre_option_' . $this->id_data[ 'base' ], array( $this, '_preview_filter' ) );
	else
		add_filter( 'option_' . $this->id_data[ 'base' ], array( $this, '_preview_filter' ) );
	break;

I mean, I get the case, but I'm not sure of the reason for the diff between the existence of the pre_ vs. non-pre_ filter. If the customizer is just overriding values, shouldn't all of them go through the pre_ filter? I mean, we're not filtering the existing data, we're replacing it. Since the option data is loaded into the customizer in advance, replacing "all" should work, right? Or am I missing a case? Perhaps default cases need to be considered?

#22 follow-up: @Otto42
12 years ago

I understand why the use of pre_option vs. option now (serialized data vs. non-serialized data), however the use of option_ in this case causes the settings preview to not work when the row of settings does not exist in the database at all.

Basically, if the entire option row doesn't exist, then get_option will return the $default value, without applying the option_whatever filter to it, so the preview doesn't reflect the changes made.

I considered a few alternatives, but I think that actually applying the option_$option filter to the default options will cause breakage in some plugins somewhere, and so it's not a viable patch. So instead, I've created a "default_option_$option" filter, which specifically filters only the default options returned. By applying the customizer preview filters to that as well, this problem is fixed.

Potential patch attached.

@Otto42
12 years ago

add and use a default_option_ filter

#23 follow-ups: @chipbennett
12 years ago

  • Cc chip@… added

For option type settings, that correspond to settings added as an array via register_setting(), is there any way to pass them through the register_setting() $sanitize_callback on-save? Seems that might be more flexible/scalable than requiring a sanitize_callback option for each $wp_customize->add_setting() call.

#24 in reply to: ↑ 23 ; follow-up: @Otto42
12 years ago

Replying to chipbennett:

For option type settings, that correspond to settings added as an array via register_setting(), is there any way to pass them through the register_setting() $sanitize_callback on-save? Seems that might be more flexible/scalable than requiring a sanitize_callback option for each $wp_customize->add_setting() call.

Looks like I gave you bad info before, Chip. Options that are added via register_setting *do* get their validation callback ran across them, even when being saved via the theme customizer.

This is because register_setting actually adds the validation function as a filter to sanitize_option_{$option_name}, so regardless of the method by which the option is updated, the sanitize function will still run on its contents.

#25 in reply to: ↑ 6 ; follow-up: @helenyhou
12 years ago

Replying to Myatu:

CSS: Controls in the content column, ie .customize-control-select select make use of fixed widths, which do not account for a scroll-bar that may appear if the expanded content exceeds the height of the browser window.

19910.customize-sidebar.diff is a first-pass at addressing that. May need a more thorough sweep through widths in case I missed something.

Also, the scroll bar looks wonky in Chrome (not tested in others as of yet), as shown below (scrolled all the way to the top)

This is because of the header and footer that are absolutely positioned. Not sure what to do (or can be done) about this just yet.

#26 @koopersmith
12 years ago

In [20737]:

Theme Customizer: Use a internal global _wpCustomizeSettings object for wp.customize.settings to prevent script race conditions in IE (which blocked interaction with the preview). see #20582, #19910.

#3 @koopersmith
12 years ago

In [20741]:

Theme Customizer: Add cross-domain handling for when the admin and front-end are different origins. Handles both ajax and postMessage calls. props rboren, mdawaffe, nacin. see #20507, #19910.

#18 @koopersmith
12 years ago

In [20743]:

Theme Customizer: Add basic loading indicator when loading the customizer inside an iframe. see #19910.

#19 follow-up: @koopersmith
12 years ago

In [20745]:

Theme Customizer: Maintain scrolled position when preview performs a full refresh. Allow wp.customize.Messenger to send/receive falsy values. see #19910.

#20 follow-up: @koopersmith
12 years ago

In [20747]:

Theme Customizer: Link the current theme screenshot to the customizer. see #19910.

#21 @ryan
12 years ago

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

Closing this umbrella ticket. Report bugs in new tickets.

#22 follow-up: @koopersmith
12 years ago

In [20758]:

Uploader & Theme Customizer: Refresh uploader flash/silverlight shims when the mouse enters the browse button. Add class to body based on uploader's drag/drop support. Properly assign customize image uploader containers (so flash shims are positioned correctly when scrolled). see #19910, #20452, #20582.

#23 follow-ups: @koopersmith
12 years ago

In [20759]:

Theme Customizer: Remove !important declarations from hide-if-customize and hide-if-no-customize classes. Use a default no-customize-support class on the body so we can use an element's original display property when visible. fixes #20565, see #19910.

#24 follow-up: @koopersmith
12 years ago

In [20761]:

Theme Customizer: Add a fallback UI for browsers that do not support drag and drop upload. see #19910, #20452, #20582.

#24 follow-up: @koopersmith
12 years ago

In [20777]:

Theme Customizer: Flex-width friendly UI, improved typographical hierarchy. see #19910.

#25 follow-up: @koopersmith
12 years ago

In [20789]:

Theme Customizer: Fix Opera bug where, once the sidebar was collapsed, it could not be re-opened. props rosshanney. fixes #20452, see #19910.

#26 @koopersmith
12 years ago

In [20791]:

Theme Customizer: Prevent sidebar scrollbar from overlapping sidebar header or footer. fixes #20649, #20650, see #19910.

#27 follow-ups: @koopersmith
12 years ago

In [20795]:

Theme Customizer: Add an Events mixin to wp.customize, used by default in wp.customize.Messenger. see #19910.

#28 @koopersmith
12 years ago

In [20797]:

Theme Customizer: Make wp.customize.Values.when return a jQuery.Deferred.promise() object, to add additional flexibility. see #19910.

#29 @koopersmith
12 years ago

In [20798]:

Theme Customizer: Clarify wp.customize.Values API by removing Values.pass and all methods that passed through to the wp.customize.Value objects it contained. see #19910.

#30 follow-ups: @koopersmith
12 years ago

In [20799]:

Theme Customizer: Add the wp.customize.Events mixin to wp.customize.Values. Provide 'add', 'remove', and 'change' events by default. see #19910.

#31 follow-up: @koopersmith
12 years ago

In [20801]:

Theme Customizer: Make the wp.customize.Events methods chainable. see #19910.

#32 @koopersmith
12 years ago

In [20802]:

Theme Customizer: Properly change state when theme is switched. fixes #20610, see #19910.

  • Causes the Manage Themes page to refresh if the customizer is closed after the active theme is switched.
  • Changes the text of the 'Save and Activate' button once the theme has been activated.
  • Improves the naming of the customize control settings.
  • Add events to customize.Loader and make callbacks more flexible.
  • Makes the customize-loader l10n compatible with non-admin settings.
  • Adds WP_Customize->is_current_theme_active().
  • Minor style corrections, including jQuery.attr/prop changes.

#33 @koopersmith
12 years ago

In [20809]:

Theme Customizer: Properly escape customize settings when sending values to JS. Add WP_Customize_Setting->js_value(). fixes #20687, see #19910.

#34 @koopersmith
12 years ago

In [20810]:

Manage Themes: Show current theme screen shot when customize is not supported. props ocean90, fixes #20693, see #19910.

#3 @koopersmith
12 years ago

In [20816]:

Theme Customizer: Add 'ready' event to customize-controls. see #19910.

#4 @koopersmith
12 years ago

In [20820]:

Theme Customizer: Ensure image control tabs are full-width to prevent floating bugs. see #19910.

#5 follow-ups: @koopersmith
12 years ago

In [20822]:

Theme Customizer: Sidebar RTL fixes. see #19910.

#6 follow-ups: @koopersmith
12 years ago

In [20824]:

Theme Customizer: RTL fixes. props ocean90, fixes #20585, see #19910.

#7 @ryan
12 years ago

In [20827]:

'Live Preview' instead of 'Customize' for the customizer links in the theme browser. see #19910

#8 follow-ups: @koopersmith
12 years ago

In [20833]:

Theme Customizer: Reset scroll to top when the preview url is changed. props ocean90, fixes #20713, see #19910.

#4 @ryan
12 years ago

In [20845]:

Don't concatenate. see #19910

#5 follow-ups: @koopersmith
12 years ago

In [20861]:

Theme Customizer: Properly handle redirects in the preview by setting wp_redirect_status to 200. props nacin, see #20507, #19910.

#28 @koopersmith
12 years ago

In [20863]:

Theme Customizer: Add a base element to the preview's head element to allow relative links (root, hash, and query strings). see #20507, #19910.

#29 @koopersmith
12 years ago

In [20864]:

Theme Customizer: Change 'Return to...' link to 'Cancel' and move 'Save' button to header. props helenyhou, fixes #20692, see #19910.

#30 follow-ups: @ryan
12 years ago

In [20877]:

"Save & Activate", "Save & Publish" for customizer save button. Props jane. see #20692 #19910

#31 follow-up: @koopersmith
12 years ago

In [20882]:

Theme Customizer: Improve accuracy of identifying internal urls. see #20507, #19910.

The 'customize_preview_link' filter has been replaced by 'customize_allowed_urls'.
Improved accuracy when checking for wp-admin.
Improved accuracy when attempting to match the schemes of the control and preview frames.
Improved accuracy of internal link whitelist.

#31 follow-up: @koopersmith
12 years ago

In [20886]:

Theme Customizer: Check for CORS support when the preview and admin urls are cross-domain. Add a fallback to the customize control frame, and check support there as well. see #20582, #19910.

#32 @koopersmith
12 years ago

In [20890]:

Theme Customizer: Fix hash-based loader fallbacks. see #20736, #19910, [20886].

#33 @koopersmith
12 years ago

In [20893]:

Theme Customizer: Add CORS checks to the initial check for customize support. Prevents flash of customize links on large pages. see #20582, #19910.

Add wp_customize_support_script(), to quickly alter the body class based on whether customize is supported.

#33 @koopersmith
12 years ago

In [20897]:

Theme Customizer: Prevent messengers from binding to the current window when a parent doesn't exist. see #19910.

#34 @koopersmith
12 years ago

In [20899]:

Theme Customizer: Improve activate and publish flow, make customizer states easier to track. fixes #20743, see #19910.

#3 @koopersmith
12 years ago

In [20908]:

Theme Customizer: Improve default background property handling. see #20600, #19910.

If the custom background default wp-head-callback (_custom_background_cb) is used, we use postMessage for all custom background properties. Otherwise, we use full refreshes.

When using postMessage, the preview recalculates the custom background CSS block, allowing it to omit CSS values when they are not present and fall back on the original CSS.

#11 follow-ups: @koopersmith
12 years ago

In [20910]:

Theme Customizer: Improve hex color sanitization functions. fixes #20600, see #19910.

Instead of fetching default header_textcolor manually, return null to do so automatically.
Improve hex regex.

#12 @koopersmith
12 years ago

In [20912]:

Theme Customizer: Section reorganization. see #19910.

Now that sections are organized in accordions, group items by size and usage.
Move header/background controls into colors, header image, background image, and title/tagline sections.

#13 follow-ups: @azaozz
12 years ago

If the custom background default wp-head-callback (_custom_background_cb) is used, we use postMessage for all custom background properties. Otherwise, we use full refreshes.

Was looking at this yesterday, didn't suggest a patch since it's a big change. Perhaps something to consider for 3.5 or in the future.

Wouldn't it be simpler and more robust to always do a full refresh of the preview iframe when a user changes something? That would allow WP filters, actions and PHP callbacks to run on the changed value and filter/reject/tweak the value according to the theme and any plugins that may be hooked there or after that.

So instead of the current "flow":

  • User changes a setting
  • We send the changes back to the server (if they need processing in PHP)
  • We receive the filtered values (optional)
  • We push them to the preview iframe, applying the changes with JS

we could do:

  • User changes a setting
  • We send the changes back to the server triggering a refresh of the preview iframe.

In both cases the preview is updated to reflect the new setting. In the second case we don't need to push any changes with JS removing any complications there (cross-domain iframes, etc.).

#14 follow-up: @koopersmith
12 years ago

In [20913]:

Theme Customizer: Improve background image control, add correct meta key to custom headers and backgrounds uploaded using the customizer. see #19910.

#15 in reply to: ↑ 13 @Otto42
12 years ago

Replying to azaozz:

Wouldn't it be simpler and more robust to always do a full refresh of the preview iframe when a user changes something?

A refresh is very slow by comparison to using postMessage to make instant changes. Seconds instead of under a millisecond. It would be super slow and annoying to have a color change take a few seconds for me to see the effect, when I could just as easily cycle around and see the change in real-time.

As it is now, the theme has the choice of whether to use refresh (default, works for everything) or postMessage (added on, only works for things that the theme can re-implement in JS code).

#16 in reply to: ↑ 13 @Otto42
12 years ago

Replying to azaozz:

So instead of the current "flow":

  • User changes a setting
  • We send the changes back to the server (if they need processing in PHP)
  • We receive the filtered values (optional)
  • We push them to the preview iframe, applying the changes with JS

Also, as far as I know, your steps two and three there don't happen anywhere. postMessage sends the value directly into the preview frame's JS handler when it changes, it doesn't have the ability to pass it back into the server for filtering, unless I missed something somewhere.

#17 follow-up: @azaozz
12 years ago

Replying to Otto42:

A refresh is very slow by comparison to using postMessage to make instant changes...
Also, as far as I know, your steps two and three there don't happen anywhere...

That's another point: doing "instant" refresh bypasses the possibility to run any PHP hooks and callbacks that may be needed. I know we don't currently filter "background-color" for example but a plugin or theme may decide to filter it. Currently that's not possible to show in the preview.

Regarding speed: right, postMessage is faster than "full" refresh of the preview, however this has it's drawbacks (no IE < 10 when cross-domain, i.e. on most multisite installs current IE won't work). Also speed is somewhat relative, it's "nice to have" but shouldn't prevent running hooks on the user settings.

#18 @Otto42
12 years ago

If a theme or plugin is using filters on those relevant pieces in some way, then they have the ability to change the control to be using refresh instead of postMessage as well. It's one line of code:

$wp_customize->get_setting('background-color')->transport='refresh';

As it is, I figure most will be going the other way and changing default refresh settings to be postMessage ones.

Themes we don't need to worry about at all WRT their filters, since the theme author will also have to implement any JS to handle a postMessage method. The controls default to refresh, after all, they have to turn on postMessage.

And yes, I think it should totally prevent hooks running on the settings, for the case where the theme has explicitly said "hey, use postMessage". Not running those through the server is sort of the whole point.

Re: IE, just force it to refresh on all controls when IE and cross-domain is detected. Not much else to do about that. postMessage is an optional add-on that the theme has to both enable and add extra code to support, while refresh is the default.

#19 follow-up: @koopersmith
12 years ago

In [20915]:

Theme Customizer: Allow sanitize_hexcolor to accept the empty string. Fixes default assignment on save and bug where header textcolor would remain hidden if loaded hidden. see #19910.

#20 follow-up: @azaozz
12 years ago

Another (big?) advantage of always doing full refresh of the preview is that the code needed would be a lot simpler and more robust/compatible. For example currently the customizer won't work in FF with the NoScript addon if more strict rules are set for it.

But as I said it's too late to look into this now, perhaps in 3.5.

#21 in reply to: ↑ 20 @nacin
12 years ago

Replying to azaozz:

Another (big?) advantage of always doing full refresh of the preview is that the code needed would be a lot simpler and more robust/compatible. For example currently the customizer won't work in FF with the NoScript addon if more strict rules are set for it.

But as I said it's too late to look into this now, perhaps in 3.5.

I'm confused why this is bring brought up now, seeing that postMessage was part of the original scope and implementation — way back from January/February. Regardless, the user experience is far, far better with postMessage. I'm a backend API person and even then I don't see any kind of justification to do a round-trip. It works, it's simple enough, it's robust, it's compatible. Removing it is a non-starter in my book.

#22 follow-up: @azaozz
12 years ago

Replying to Otto42:

And yes, I think it should totally prevent hooks running on the settings, for the case where the theme has explicitly said "hey, use postMessage". Not running those through the server is sort of the whole point.

So if there's a plugin that runs hooks on a setting and the theme opts for postMessage, the preview would not reflect the filtering done by the plugin and the output will be different from the front-end? The main point of a preview is to be "truthful" :)

Denying PHP hooks on the new settings is a drawback, consider adding menus and widgets to the customizer in the future, "instant" JS driven previews would not be possible for them.

Replying to nacin:
As far as I remember the original scope was to have an API for front-end previews accessible from any page in the admin. Handling the preview from JS only was a "nice-to-have-if-it-works" kind of thing.

The underlying problem is that in some cases the preview won't be "real" without a trip to the server, whether it's an XHR or iframe refresh.

Yes, agree now it's not the time for such discussion, could we leave it for when adding widgets and menus support to the customizer.

#23 follow-ups: @koopersmith
12 years ago

In [20916]:

Twenty Eleven theme customizer integration. props lancewillett, Otto42. fixes #20448, see #19910.

#24 in reply to: ↑ 22 ; follow-up: @Otto42
12 years ago

Replying to azaozz:

So if there's a plugin that runs hooks on a setting and the theme opts for postMessage,

In that case the plugin is perfectly capable of overriding the theme's control and changing the transport mode for that control to "refresh" and thus forcing the customizer to refresh the theme.

add_action( 'customize_register', 'force_refresh_demo', 100 ); // late hook to ensure the control is there
function force_refresh_demo($wp_customize) {
  wp_customize->get_setting('some-example-setting')->transport='refresh';
}

Voila. Plugin can handle it, and it's the proper place for it to be handled because the plugin knows whether or not it's hooking onto some setting and modifying it in a way that the customizer will need to care about.

Denying PHP hooks on the new settings is a drawback, consider adding menus and widgets to the customizer in the future, "instant" JS driven previews would not be possible for them.
...
The underlying problem is that in some cases the preview won't be "real" without a trip to the server, whether it's an XHR or iframe refresh.

If it is necessary to add some new mechanism later that goes to the server to retrieve some kind of PHP generated data via AJAX, and then pass that into the preview frame, then that can be added later. It doesn't mean that we should dump postMessage and opt for full-refresh all the time, just because some cases don't work well with postMessage. There's nothing wrong with having more than one way to get the job done.

#25 follow-up: @koopersmith
12 years ago

In [20918]:

Hide 'Customize' toolbar item when customizer is not supported. fixes #20751, see #19910.

Removes script queue check for 'customize-loader' from wp_customize_support_script(), because we may want to check for customize-support on a page without the loader.

#2 @koopersmith
12 years ago

In [20919]:

Theme Customizer: Correctly use custom-background class in theme preview. See #19910.

#3 @koopersmith
12 years ago

In [20922]:

Theme Customizer: Reduce size of box shadow to minimize scrollbar overlap in Firefox. props azaozz, fixes #20753, see #19910.

#3 @koopersmith
12 years ago

In [20925]:

Theme Customizer: Add a signature to preview requests to be super-double-ultra-sure that the customizer generated the preview. Redirects can be sneaky. fixes #20507, see #19910.

#4 @koopersmith
12 years ago

In [20928]:

Theme Customizer: Fix IE8 drag/drop uploader incompatibility. see #20582, #19910.

Bind drag/drop support checks to individual plupload instances.
Add drag-over classes to wp.Uploader and corresponding styles to customize image controls.

#5 follow-ups: @koopersmith
12 years ago

In [20930]:

Theme Customizer: Fix preview background CSS creation to insert/remove style nodes (for IE8 compatibility). fixes #20582, see #19910.

#6 follow-ups: @ocean90
12 years ago

We should say that the text color is only for the header.

See [20912] and 19910.declare-text-color-as-header-text.patch.

#7 @nacin
12 years ago

In [20933]:

Theme Customizer: 'Header Text Color', not 'Text Color', when referring to the header text in the Colors section. props ocean90. see #19910.

#8 follow-ups: @nacin
12 years ago

In [20934]:

Theme Customizer: As customize.php without a theme parameter defaults to the current theme, update wp_customize_url() to make $stylesheet optional and update references for the current theme.

see #19910, #20751, #20575.

#3 @koopersmith
12 years ago

In [20936]:

Theme Customizer: Ensure that JS color controls always use real color values, even if the server-side value is a hex value without a hash. fixes #20448, see #19910.

Adds WP_Customize_Setting->sanitize_js_callback and 'customize_sanitize_js_$settingID' filter, to filter values before they're passed to JS using WP_Customize_Setting->js_value().

Adds support for regular hex colors to the color picker.

Changes color methods:

  • sanitize_hex_color() accepts 3 and 6 digit hex colors (with hashes) and the empty string.
  • sanitize_hex_color_no_hash() accepts 3 and 6 digit hex colors (without hashes) and the empty string.
  • maybe_hash_hex_color() ensures that a hex color has a hash, and otherwise leaves the value untouched.

#4 @koopersmith
12 years ago

In [20940]:

Theme Customizer: Don't expand theme title if there are no details to show. props nacin, fixes #20757, see #19910.

#5 follow-ups: @kwight
11 years ago

Anyone know why the section descriptions were moved out of view and into the title attribute in [20182]? I can't seem to find an explanation.

#6 in reply to: ↑ 5 ; follow-ups: @SergeyBiryukov
11 years ago

Replying to kwight:

Anyone know why the section descriptions were moved out of view and into the title attribute in [20182]? I can't seem to find an explanation.

Me neither, but feel free to create a new ticket if there's a problem.

#7 in reply to: ↑ 5 @SergeyBiryukov
11 years ago

Replying to kwight:

Anyone know why the section descriptions were moved out of view and into the title attribute in [20182]? I can't seem to find an explanation.

Follow-up: #24392

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


8 years ago

Note: See TracTickets for help on using tickets.