Opened 2 weeks ago
Last modified 40 hours ago
#29988 new enhancement
Twenty Fifteen: Use JS/postMessage to update the color scheme instead of triggering a page refresh
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | 4.1 | Priority: | normal |
| Severity: | normal | Version: | trunk |
| Component: | Bundled Theme | Keywords: | has-patch dev-feedback |
| Focuses: | javascript | Cc: |
Description
It takes a really long time... :(
Attachments (1)
Change History (13)
comment:2
avryl
— 2 weeks ago
Even if you replace and generate the *whole* style tag with JS, it'd be a lot faster. :)
comment:3
celloexpressions
— 2 weeks ago
We really shouldn't do that. The only good way to implement postMessage support here would be for core to support partial preview refreshes: #27355. Otherwise we have to have the colorscheme logic (mostly CSS) duplicated in JS, which makes it even harder to keep in sync with the stylesheet. The general rule of thumb for colorscheme-type options in the Customizer right now is that you have two options:
- Use color pickers, generate css from that in the <head>, and use only refresh previewing.
- Offer a pre-defined list of colorshemes (via a select or radio control), and trigger the different schemes with a body class, building the actual styles into a stylesheet that grabs one by class. Then, you can implement postMessage pretty cleanly by just changing that body class. This is also the best way to implement layout options such as left/right sidebar, and is the fastest way to live-preview leveraging browser capabilities.
Twenty Fifteen does both UI-wise, which could work, although I don't think the UX is very good right now. I'll most likely be making a ticket to clean up the approach for the colorschemes more generally, as it has some major issues based on my initial quick look at it.
But for this, unless we want to significantly change how the colorschemes work, getting #27355 patched and into core in 4.1 is probably our only option here. I had planned on doing it for 4.1, but it got pushed out of priority so that we could focus on things like media in the Customizer, since that was an advertised feature of Twenty Fifteen that we wanted to highlight with revamped background images and the removal of the old admin screens. We can probably make that happen only if we get some breathing room with beta on that ticket (ie, it probably needs a bit more than 2 weeks).
comment:4
follow-up:
↓ 5
avryl
— 2 weeks ago
Why would you take a trip to the server just for this? Even a partial refresh would be slow. If duplicating the CSS is the problem, then add it with PHP and parse it as a template?
comment:5
in reply to:
↑ 4
celloexpressions
— 2 weeks ago
Replying to avryl:
Why would you take a trip to the server just for this? Even a partial refresh would be slow. If duplicating the CSS is the problem, then add it with PHP and parse it as a template?
Try justifying doing something like that in a bundled theme to nacin :)
One of the things with bundled themes is that they should be reasonably straightforward, and as simple as possible for new "developers" to dig into the code. And we should also be doing it in the best-practice way. When it comes down to it, duplicating code in PHP and JS is not a good idea. Parsing the php as a template might be good, but if we were to do that we would want to have some sort of a core API to facilitate it.
In terms of a core API that helps developers leverage postMessage in the Customizer more easily/without duplicating code, partial preview refreshes are the best solution we've come up with that should work for most theme/plugin options that have a chance of working without doing a full preview refresh. We've been tossing that idea around for nearly a year now, and have yet to come up with anything better or as accessible and compatible with different scenarios. It comes down to that trip up to the server to grab a bit of output being significantly faster than a complete loading of the front-end of the site as well as images, etc., even if it isn't as instant as a full-JS solution.
For Twenty Fifteen, this also goes back to the two different UIs for colorschemes. When working with color pickers, the delay is fine, and maybe even better as it can be overly distracting (see the background color control). When working with a selector there's a straightforward best-practice way to implement postMessage support. With the combination, we don't have as many options there. Also, looking at the way the CSS is currently implemented, I doubt that it would convert well to JS, considering that it's pretty messy in PHP. I also have some suggestions for accessibility improvements that would increase the code complexity there, thinking along the lines of how Fourteen Colors works; that would almost certainly prevent a php-template-type solution from working.
comment:6
westonruter
— 2 weeks ago
- Keywords has-patch dev-feedback added; needs-patch removed
In 29988.patch I've implemented partial preview refreshes for updating the inline styles when one of their constituent settings is changed. Changing a setting in the Customizer pane causes the setting to get copied to the preview via postMessage and then the preview does an Ajax request to regenerate the CSS and then swaps it out. This is much much faster than refreshing the entire page.
Other changes/fixes made in the course of this patchL
- Core: style elements generated by wp_add_inline_style now include an ID attribute. This allows them to be selected with JS.
- Core: The Customizer Color Picker control was failing to update its state when its underlying setting was changed (sync fail). This has been fixed.
- TwentyFifteen custom backgrounds have been moved to a separate includes file, similar to the custom header includes file.
- TwentyFifteen functions.php: This is a bit unsightly, but the custom header and custom background callbacks get removed from being added to the wp_head and instead get added to wp_enqueue_scripts. This eliminates a priority problem, and it works better since wp_add_inline_style() is now being used uniformly for all inline style output.
- TwentyFifteen colorScheme control: The color picker element was being found by traversing the DOM, but there are no guarantees that the control is going to remain where the theme expects. So now it is using the proper wp.customize.control(…).container API to grab the control's element.
Also on GitHub: https://github.com/xwpco/wordpress-develop/pull/31/files
Regarding a general framework partial-preview refreshes (#27355), I think more use cases need to be fleshed out and patterns need to emerge. Implementing a refresh for CSS in a bundled theme is a nice way to get this idea out there.
comment:7
ircbot
— 13 days ago
This ticket was mentioned in IRC in #wordpress-dev by westonruter. View the logs.
comment:8
westonruter
— 13 days ago
We've come up with a plan to add partial preview refreshes to core (#27355). See IRC log. Once patch is added to #27355, then the patch attached here will need to be updated to make use of the new API. This will eliminate all the complexity currently in the patch, though there are a couple things in the patch that should still be added to twentyfifteen and core. I should to open separate tickets for them, I reckon.
comment:9
westonruter
— 12 days ago
I've extracted the general Customizer improvements, including the core ColorControl change and the fixes to the Twenty Fifteen colorScheme control, from the above patch into a separate ticket #30031.
comment:10
westonruter
— 12 days ago
Also extracted the addition of an id attribute on inline style elements to a separate ticket #30032.
comment:11
slackbot
— 40 hours ago
This ticket was mentioned in Slack in #core-themes by iandstewart. View the logs.
comment:12
slackbot
— 40 hours ago
This ticket was mentioned in Slack in #core-themes by iandstewart. View the logs.
+1.
I started to look at this with @iamtakashi while he was developing the theme. We were thinking about breaking up $css in twentyfifteen_color_scheme_css() into selectors for each color to pass to 'js/customizer.js' with wp_localize_scripts() to avoid duplicating all of those selectors.