Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#54549 closed defect (bug) (fixed)

It's possible to switch to a block theme from within Customizer

Reported by: antonvlasenko's profile antonvlasenko Owned by: hellofromtonya's profile hellofromTonya
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.9
Component: Customize Keywords: has-screenshots has-patch has-unit-tests has-testing-info commit needs-dev-note
Focuses: ui Cc:

Description

As reported by @hellofromTonya here:
Right now, it's possible to switch to a block theme from within Customizer.

Steps to reproduce the issue:

  1. Activate a non-block theme like Twenty Nineteen.
  2. Click on Customize button to open Customizer.
  3. Click on "Change" in the "Active theme" area.
  4. Notice Twenty Twenty-Two is displayed there. Click on "Live Preview" button.
  5. Click "Activate & Publish" button.

Attachments (6)

Screen Shot 2021-12-07 at 11.32.01 AM.png (1.6 MB) - added by shaunandrews 3 years ago.
Screen Shot 2021-12-07 at 11.32.06 AM.2.png (67.5 KB) - added by shaunandrews 3 years ago.
Screen Shot 2021-12-07 at 11.31.51 AM.png (301.0 KB) - added by shaunandrews 3 years ago.
notice.png (36.2 KB) - added by poena 3 years ago.
Utilize the existing space for displaying notifications:
Test-Report-pr1991-applied.png (5.1 MB) - added by hellofromTonya 3 years ago.
Test Report: with PR 1991 applied, the block theme has a notification over it. Works as expected ✅
54549-test-in-customizer.gif (9.1 MB) - added by hellofromTonya 3 years ago.
Test Report: shows interaction between block and non-block themes in Customizer.

Change History (31)

#1 @antonvlasenko
3 years ago

I'm working on a fix for this issue.

#2 @joyously
3 years ago

It seems like this should not be a bug or problem.
It also seems that a Customize link should be available when a block theme is active, so that other themes can be previewed for switching, and so that other things plugins add to Customizer can still be reached.

#3 @antonvlasenko
3 years ago

It seems like this should not be a bug or problem.

As far as I know, block-based themes are not supposed to be edited with Customizer.

It also seems that a Customize link should be available when a block theme is active, so that other themes can be previewed for switching, and so that other things plugins add to Customizer can still be reached.

In this case other themes can be previewed using the Themes page.

This ticket was mentioned in PR #1991 on WordPress/wordpress-develop by anton-vlasenko.


3 years ago
#4

  • Keywords has-patch added; needs-patch removed

#5 @antonvlasenko
3 years ago

  • Keywords has-unit-tests added

Steps to test this issue:

  1. Activate a non-block theme like Twenty Nineteen.
  2. Click on Customize button to open Customizer.
  3. Click on "Change" in the "Active theme" area.
  4. Block themes like Twenty Twenty-Two should not be displayed on that page.

Before:
https://cldup.com/jvmprOUkNP.png

After:
https://cldup.com/qUvTfjmJyD.png

#6 @antonvlasenko
3 years ago

  • Keywords needs-testing has-screenshots added

anton-vlasenko commented on PR #1991:


3 years ago
#7

This PR has a lot of changes because it has been rebased on top of https://github.com/WordPress/wordpress-develop/pull/2014 branch (because we've renamed some methods).

hellofromtonya commented on PR #1991:


3 years ago
#8

Hey @anton-vlasenko, I think you need to revert some of the commits in this PR such as 450c03d39b504f4687b2458bcb9eff3b962a60c1, ed79860f0582e0b971afb15b5522c7b7cbaf2c11, and 09d406230dde56eade1c2f27879e73f8cd7ac006. All of these were committed today. Then rebase on top of trunk to pull the latest into this PR. Thanks!

anton-vlasenko commented on PR #1991:


3 years ago
#9

@hellofromtonya Thank you for the review!
Yes, that's good point. I've made the requested changes.

#10 @hellofromTonya
3 years ago

  • Keywords dev-feedback added

Need consensus on this one before moving forward. Here come some pings:

@noisysocks @jffng What do you think about removing block themes from within Customizer's previewer?

#11 @jffng
3 years ago

Thanks for the ping and work here!

I'm not sure about this. In general I don't think we should be preventing or removing pathways for folks to switch to block themes, regardless of where they're coming from — in this case, the Customizer. But this particular flow could use more design consideration.

Last edited 3 years ago by jffng (previous) (diff)

#12 @shaunandrews
3 years ago

The first idea that comes to mind is to continue to show block themes in the Customizer, but separate them, either in the grid list or with some groupings in the sidebar.

We could simply disable the themes, with some explanatory label about not supporting the Customizer. This might be complicated.

Alternatively, we could allow activate and show a dialog about using the Site Editor.

(I attached some _very_ quick mockups below. They're more like wireframes to help with the discussion.)

Last edited 3 years ago by shaunandrews (previous) (diff)

#13 @hellofromTonya
3 years ago

  • Keywords needs-design-feedback added; has-patch has-unit-tests removed

Updating the keywords as it needs design consideration (which is in progress) and then the patch and tests will need adjustment once there's consensus on design.

Tentatively moving into 5.9 as this is introduced in the 5.9 cycle. But could be considered an enhancement and moved to a later release for refinement.

#14 @noisysocks
3 years ago

Yeah I also don't think we should hide or remove block themes from this view. Showing a message with a prompt to "activate and switch to the site editor", as in Screen Shot 2021-12-07 at 11.31.51 AM.png seems reasonable to me.

#15 @poena
3 years ago

I think it would be better to utilize the existing space for displaying messages related to installation, instead of adding a new modal.

@poena
3 years ago

Utilize the existing space for displaying notifications:

#16 @antonvlasenko
3 years ago

Utilize the existing space for displaying notifications:

I like the idea of using the existing markup to display notifications, as it will require fewer changes (given the time left before the WordPress 5.9 release).
And it makes sense to me to reuse already existing functionality.
So, can we display a notification above the theme's screenshot (as suggested by @poena)?
https://cldup.com/Qv9Puv1brW.png
The text of the message can be refined later.

Last edited 3 years ago by antonvlasenko (previous) (diff)

#17 @antonvlasenko
3 years ago

The patch is ready to be reviewed.
I haven't got any new feedback, so I implemented the fixed as described in my message above.

Version 1, edited 3 years ago by antonvlasenko (previous) (next) (diff)

#18 @costdev
3 years ago

Test Report

Env

  • WordPress 5.9-beta2-52344-src
  • Chrome 96.0.4664.93
  • Windows 10
  • Theme: Twenty Nineteen
  • Gutenberg Editor
  • Plugin: None activated

Steps to test

  1. Activate a non-block theme like Twenty Nineteen.
  2. Click on Customize button to open Customizer.
  3. Click on "Change" in the "Active theme" area.
  4. Notice Twenty Twenty-Two is displayed there. Click on "Live Preview" button.
  5. Click "Activate & Publish" button.
  6. Apply PR 1991.
  7. Repeat steps 1-3.
  8. Twenty Twenty-Two should now have an error notice above it.

Results

  • Before the PR: There was no indication that a block theme uses the Site Editor instead of the Customizer.
  • After the PR: A notice now informs the user that a block theme uses the Site Editor instead of the Customizer.

Notes

  • The "activate this theme" in the grid view does not activate the theme. Like clicking anywhere in a theme's grid item, it opens a modal.
  • The "activate this theme" in the modal correctly activates the theme.

@hellofromTonya
3 years ago

Test Report: with PR 1991 applied, the block theme has a notification over it. Works as expected ✅

@hellofromTonya
3 years ago

Test Report: shows interaction between block and non-block themes in Customizer.

#19 @hellofromTonya
3 years ago

  • Keywords has-patch has-unit-tests has-testing-info commit needs-dev-note added; needs-testing dev-feedback needs-design-feedback removed
  • Milestone changed from Awaiting Review to 5.9

As this is introduced in 5.9, moving it into the milestone. PR 1991 is ready for commit.

#20 @antonvlasenko
3 years ago

P̶l̶e̶a̶s̶e̶ ̶d̶o̶n̶'̶t̶ ̶m̶e̶r̶g̶e̶ ̶i̶t̶.̶ ̶I̶'̶v̶e̶ ̶n̶o̶t̶i̶c̶e̶d̶ ̶a̶n̶ ̶i̶s̶s̶u̶e̶ ̶o̶n̶ ̶t̶h̶e̶ ̶T̶h̶e̶m̶e̶s̶ ̶p̶a̶g̶e̶ ̶t̶h̶a̶t̶ ̶m̶a̶y̶ ̶b̶e̶ ̶r̶e̶l̶a̶t̶e̶d̶ ̶t̶o̶ ̶t̶h̶i̶s̶ ̶p̶a̶t̶c̶h̶.̶
UPD: false alarm.

Last edited 3 years ago by antonvlasenko (previous) (diff)

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

#22 @hellofromTonya
3 years ago

  • Owner changed from antonvlasenko to hellofromTonya
  • Status changed from assigned to reviewing

Preparing the commit now.

#23 @hellofromTonya
3 years ago

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

In 52371:

Customize: Overlay incompatible banner for block themes.

Starting in 5.9, block themes are not compatible with (do not support) Customizer; rather, they use the Site Editor. Viewing installed themes in Customizer, this commit adds an overlay message to alert users and give them a way to activate the block theme. Clicking on the "Activate" button activates the block theme and redirects back to the Appearance > Themes interface, where the user can then enter the Site Editor for customization.

Non-block themes are not affected by this change and continue to work in Customizer.

Follow-up to [41648], [41893], [52279].

Props antonvlasenko, costdev, hellofromTonya, jffng, joyously, noisysocks, poena, shaunandrews.
Fixes #54549.

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


3 years ago

Note: See TracTickets for help on using tickets.