#31303 closed task (blessed) (fixed)
Add theme-browsing and theme-switching to the Customizer
Reported by: | celloexpressions | Owned by: | markjaquith |
---|---|---|---|
Milestone: | 4.2 | Priority: | normal |
Severity: | normal | Version: | 4.2 |
Component: | Customize | Keywords: | has-patch |
Focuses: | ui | Cc: |
Description
This ticket is for merging the Customizer Theme Switcher feature-plugin and addressing related issues that require core patches.
See https://make.wordpress.org/core/2015/02/11/customizer-theme-switcher-feature-plugin-merge-proposal/.
Plugin for testing (4.1-compatible): https://wordpress.org/plugins/customizer-theme-switcher/
Attachments (4)
Change History (16)
#1
@
10 years ago
- Component changed from Themes to Customize
Now everyone watching both components has been notified :)
All of the code changes here are on the Customizer side, and are UI-level.
#2
@
10 years ago
31303.2.diff addresses all of the known issues in the plugin that required core patches:
- Remove #customize-info for theme previews.
- Change front-end admin bar Themes link to point to themes in the Customizer (deep-linked).
- When a new theme is activated, go to the home page (front end), not the themes admin.
- If user doesn't confirm that they want to leave unsaved changes, remove customize-loading body class.
Further enhancements, primarily changing the way the themes header displays, should probably happen after a first-pass is in (see also #31336 and #31289).
#5
@
10 years ago
- Owner set to markjaquith
- Resolution set to fixed
- Status changed from new to closed
In 31533:
#6
follow-up:
↓ 7
@
10 years ago
Attached 31303.3.diff with some fixes for jshint and some improvements to escaping.
#7
in reply to:
↑ 6
;
follow-up:
↓ 8
@
10 years ago
Replying to westonruter:
Attached 31303.3.diff with some fixes for jshint and some improvements to escaping.
We don't escape translations like that. I also can't see our build tools producing JSHint errors. Where did you get them from?
#8
in reply to:
↑ 7
;
follow-ups:
↓ 9
↓ 10
@
10 years ago
Replying to ocean90:
Replying to westonruter:
Attached 31303.3.diff with some fixes for jshint and some improvements to escaping.
We don't escape translations like that.
Why not? Isn't it generally a good idea in the case of malicious POT files? And it is being used once here, in class-wp-customize-section.php:
<h2><?php esc_html_e( 'Themes' ); ?>
I also can't see our build tools producing JSHint errors. Where did you get them from?
Regarding the JSHint errors, see my phpstorm-jshint-customize-controls.png above. Specifically the issue is reported in JSHint within PhpStorm, although it is not reported from the command line JSHint. But actually, I saw that my PhpStrom was using JSHint 2.2. I just updated it to 2.5.10, and it is now not complaining about these lines. Nevertheless, the mixing of tabs and spaces here does not seem ideal.
#9
in reply to:
↑ 8
@
10 years ago
Replying to westonruter:
Replying to ocean90:
Replying to westonruter:
Attached 31303.3.diff with some fixes for jshint and some improvements to escaping.
We don't escape translations like that.
Why not? Isn't it generally a good idea in the case of malicious POT files?
Basically we generally trust the translations, and don't waste the extra time in processing their contents.
There's a lot of strings in WordPress, and unless we're going to change them all, there's little point in changing a few, when you combine that with the fact a string may legitimately have HTML in it, you suddenly get to a point where if a malicious translation wants to affect the screen, it just means it has to target a specific string on the page, rather than the other 20.
#10
in reply to:
↑ 8
;
follow-up:
↓ 11
@
10 years ago
Replying to westonruter:
And it is being used once here, in class-wp-customize-section.php:
<h2><?php esc_html_e( 'Themes' ); ?>
This should be removed, see also #30724.
#11
in reply to:
↑ 10
@
10 years ago
Replying to ocean90:
Replying to westonruter:
And it is being used once here, in class-wp-customize-section.php:
<h2><?php esc_html_e( 'Themes' ); ?>This should be removed, see also #30724.
Handled in #31289 (which is still waiting for feedback/commit).
Direct merge of the Customizer Theme Switcher plugin into core, without addressing other issues that couldn't be handled in the plugin.