Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#42406 closed defect (bug) (fixed)

Customize: Can't switch back to active theme once changeset is saved

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

Description

Steps to demonstrate:

  1. Open the Customizer, make a change, and save your changeset as a draft.
  1. Open the Themes panel, discover that you can't preview themes while the changeset is a draft, and switch the changeset status back to "publish."
  1. Go back to the Themes panel, discover that you now can preview themes again, and preview a new theme.
  1. Decide you don't like the new theme, and go back to the Themes panel to preview another theme or go back to the active theme.
  1. Get the "Sorry, you can’t preview new themes..." error.

Attachments (5)

42406.0.diff (697 bytes) - added by westonruter 7 years ago.
42406.diff (1.0 KB) - added by dlh 7 years ago.
42406.3.diff (3.3 KB) - added by westonruter 7 years ago.
42406.4.diff (3.2 KB) - added by westonruter 7 years ago.
Remove unused var to fix jshint error
42406.5.diff (5.3 KB) - added by westonruter 7 years ago.

Download all attachments as: .zip

Change History (21)

@westonruter
7 years ago

#1 @westonruter
7 years ago

  • Keywords has-patch needs-testing added
  • Milestone changed from Awaiting Review to 4.9

@dlh The part in your steps is 2:

Open the Themes panel, discover that you can't preview themes while the changeset is a draft, and switch the changeset status back to "publish."

Note in particular that if you had hit the Publish button, then it would have worked as expected. The issue is that you switched the status but then didn't action on the status change. So I think it's just a matter of it neglecting to look at the saved changesetStatus when currently is looking just at selectedChangesetStatus. It needs to account for both.

Let me know if 42406.0.diff fixes that issue for you.

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


7 years ago

@dlh
7 years ago

#3 @dlh
7 years ago

The patch fixes the issue as described, but I found that the themes panel was still inaccessible after publishing the changeset. In 42406.diff I've added a binding for toggleDisabledNotifications() on the changesetStatus state to try to address that.

@westonruter
7 years ago

#4 @westonruter
7 years ago

@dlh I found there were some additional problems where even though I had saved draft and I saw the notice, I could still click on the Live Preview buttons. This was due to the logic for determining the theme switch availability not being copied out to each place where it was needed. So 42406.3.diff factors out the logic into a common canSwitchTheme method to be re-used in each instance.

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


7 years ago

#6 @westonruter
7 years ago

  • Owner set to westonruter
  • Status changed from new to accepted

@westonruter
7 years ago

Remove unused var to fix jshint error

#7 @dlh
7 years ago

42406.3.diff works as expected with respect to the original issue. I did notice that if you're unable to switch themes, but you click "Theme Details" and then use the next/previous buttons to browse themes, the "Live Preview" button becomes clickable, although it doesn't actually do anything.

@westonruter
7 years ago

#8 @westonruter
7 years ago

@dlh Good catch. 42406.5.diff (delta) moves the logic for disabling the buttons to the showDetails method itself, so the buttons will be disabled regardless of how the user gets to the details overlay. Does that wrap things up?

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


7 years ago

#10 @jbpaul17
7 years ago

  • Keywords reporter-feedback added

#11 @dlh
7 years ago

  • Keywords reporter-feedback removed

42406.5.diff works as expected for me.

#12 @westonruter
7 years ago

  • Keywords dev-feedback added; needs-testing removed

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


7 years ago

#14 @obenland
7 years ago

  • Keywords commit dev-reviewed added; dev-feedback removed

42406.5.diff LGTM as well

#15 @westonruter
7 years ago

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

In 42113:

Customize: Make sure theme switch blocking in the Customizer is consistently applied when changeset is drafted/scheduled.

  • Consider both selectedChangesetStatus and changesetStatus states when deciding to disable.
  • Factor out common logic into canSwitchTheme function on ThemesPanel.
  • Keep Live Preview and Install buttons disabled in Themes controls and detail overlays when appropriate.

Props westonruter, dlh.
Amends [41788].
See #42126, #37661, #39896.
Fixes #42406.

#16 @westonruter
7 years ago

In 42122:

Customize: Fix logic inversion in [42113] which prevented themes from being installed in Customizer.

Also fix PHP notice related to parent themes and WordPress.org theme query results.

Props dd32, obenland, celloexpressions, westonruter, atachibana for testing.
See #42406, #37661.
Fixes #42442.

Note: See TracTickets for help on using tickets.