WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#42411 closed defect (bug) (wontfix)

Theme admin screen allows Live Preview while changeset is drafted/scheduled

Reported by: westonruter Owned by: westonruter
Milestone: Priority: normal
Severity: normal Version: 4.9
Component: Customize Keywords: has-patch has-screenshots ux-feedback
Focuses: Cc:

Description (last modified by westonruter)

In the Customizer, once you save a draft or schedule changes, you are not able to switch themes. A notice is displayed:

Sorry, you can’t preview new themes when you have changes scheduled or saved as a draft. Please publish your changes, or wait until they publish to preview new themes.

The install and preview buttons in the Themes sections are then disabled to prevent a user from switching themes while changes are drafted. However, if you go to the Themes admin screen the preview buttons are still enabled there, resulting in a situation where a user could open the Customizer for a theme switch when there are already drafted/scheduled changes.

See #42126 for the reasons for why are Customizer theme switches disabled when there are drafted/scheduled changes.

Two things I think we can do here:

  • Disable the live preview buttons while also showing an admin notice informing why the Live Preview buttons are disabled.
  • Short-circuit customize.php if attempting to load a non-active theme with a drafted/scheduled changeset.

Attachments (10)

Change History (33)

#1 @westonruter
3 years ago

  • Description modified (diff)

#2 @westonruter
3 years ago

  • Keywords has-patch needs-testing added
  • Owner set to westonruter
  • Status changed from new to accepted

#3 @westonruter
3 years ago

42411.0.diff also removes the WP_Customize_Manager arg from the customize_changeset_branching filter so that it can be used outside the context of the Customizer.

Also need to determine if we need to update customize.php to block loading of Customizer if requested theme is not active.

The query params used in the get_posts() calls can probably be cleaned up.

#4 follow-up: @melchoyce
3 years ago

Patch works for me. 👍

Let's update the copy to:

Because you have drafted or scheduled changes, live previewing other themes is currently disabled. Please [publish your changes], or wait until they publish to preview new themes.

"Publish your changes" should link to the open Publish Settings panel, focused on the Publish radio button.

Though, while testing, I noticed that you can still activate a new theme — and your drafted changeset still exists, even though your changes might not be possible. Maybe we should either disable the activate buttons as well, or wipe drafted/scheduled changes on activate (and pop up an alert to confirm before allowing activation).

#5 @jbpaul17
3 years ago

  • Keywords needs-refresh added

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


3 years ago

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

Replying to melchoyce:

Though, while testing, I noticed that you can still activate a new theme — and your drafted changeset still exists, even though your changes might not be possible. Maybe we should either disable the activate buttons as well, or wipe drafted/scheduled changes on activate (and pop up an alert to confirm before allowing activation).

I don't think we we need to go that far. It's true that some of the settings in the pending changeset will be ignored if you switch the theme, but that's also the case if you switch a theme in the Customizer after having made changes to theme mods in the first theme.

#8 @melchoyce
3 years ago

I feel like we're still going to encounter some confusion around it. Let's keep an ear to the ground and if it seems to be coming up, we can address it in 4.9.1.

For now, let's still update the notice copy.

#9 @westonruter
3 years ago

  • Keywords needs-refresh removed

@melchoyce I've implemented the notice change in 42411.1.diff, and also short-circuited access to Customizer upon load if user accessed via direct URL. This also eliminates the need to later check if theme is active when establishing an auto-loaded changeset. See notice-on-customizer-if-directly-accessed.png.

#10 @melchoyce
3 years ago

Can we avoid using "Cheatin', uh?" here? (I'd honestly like to get rid of that string entirely — see #38332 and #14530.)

#11 @westonruter
3 years ago

In 42107:

Customize: Prevent re-importing starter content when changeset is saved as draft or scheduled.

Themes cannot currently be switched in Customizer after changeset is saved anyway.

Props dlh, westonruter.
See #40146, #42411, #42126.
Fixes #42395.

@westonruter
3 years ago

#12 @westonruter
3 years ago

  • Keywords has-screenshots added

@melchoyce cheating heading message removed in 42411.2.diff. See remove-cheating-message.png.

#13 @melchoyce
3 years ago

Thanks 👍 Signing off on the design.

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


3 years ago

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


3 years ago

#16 @westonruter
3 years ago

  • Keywords dev-feedback added; needs-testing removed

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


3 years ago

#18 @obenland
3 years ago

@petya @SergeyBiryukov 42411.2.diff introduces a new translation string. How do you feel about that given we're in RC?

#19 @petya
3 years ago

@obenland Only 6 teams are at 100% atm so we should be fine with that.

#20 @westonruter
3 years ago

  • Keywords ux-feedback added; dev-feedback removed
  • Milestone changed from 4.9 to 4.9.1

Thinking about this some more, and I think actually that we can revisit this later. It turns out that actually if you have drafted changes in the Customizer, and then leave to go to the WP Admin's Themes screen… when clicking Live Preview from there the Customizer actually won't open up with the previously-drafted changeset autoloaded because of this key line in WP_Customize_Manager::establish_loaded_changeset():

<?php
if ( ! $this->branching() && $this->is_theme_active() ) { 

So no drafted/scheduled changeset will be autoloaded when previewing a theme switch anyway. After switching the theme, you can then exit the Customizer and come back in to continue editing the previous changeset. Still not ideal, but I think @obenland is right that this needs some more thought in terms of UX. (After activating the theme, should a notification then appear prompting them to switch to the previously-drafted changeset?)

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


3 years ago

#22 @johnbillion
3 years ago

  • Milestone changed from 4.9.1 to 4.9.2

#23 @westonruter
3 years ago

  • Milestone 4.9.2 deleted
  • Resolution set to wontfix
  • Status changed from accepted to closed

I don't think that we need to do anything here after all. When you open the Customizer to preview a theme switch, a new changeset will be started anyway.

Note: See TracTickets for help on using tickets.