-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Site Editor: Don't navigate to the patterns in Template Parts mode #52884
Conversation
Size Change: -11 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
Flaky tests detected in 897458a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5646144200
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
I have found cases where the hybrid theme is still able to access the Pattern page after applying this PR. That is access from the Command Palette. This issue is reported in #52154.
I've removed changes from #52656 as the user won't be able to access the patterns screen and create a template part. What do you think?
Until #52154 can be resolved, I think it might be a good idea to leave the #52656 change in place for now.
0488d98dccbbcb8cba0516e029206618.mp4
Good point. I'll restore the check but remove the e2e test since you can't access the screen using navigation. What do you think? |
Yes, I think that's fine 👍 |
Done ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for work in fixing this for hybrid themes @Mamaduka 👍
✅ Didn't spot any issues with block theme navigation in site editor
✅ Navigating back from the template part list view for hybrid themes goes back to wp-admin
❓ Refreshing the template part list page for hybrid themes results in the "The theme you are currently using is not compatible with the site editor" message.
❓ Refreshing the edit template part page for hybrid themes results in a blank black page
The issues noted above might be outside the scope of this PR so I don't think they need to block landing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm this doesn't fix the issue for me. I tested by installing Emptyhybrid theme and navigating to Appearance > Template Parts. Reloading the page causes the message to appear: "The theme you are currently using is not compatible with the Site Editor."
Sorry for the confusion. This mainly fixes the client-side navigation issues and correctly sets The other half of the fix needs to happen in core - WordPress/wordpress-develop#4893. |
I just cherry-picked this PR to the update/further-bugfixes-rc2 branch to get it included in the next release: 5a9a8e3 |
What?
Fixes #52847.
PR prevents themes with template parts mode enabled from navigating to the patterns screen.
Why?
Currently, this mode only allows access to the template parts list view and template parts editing. This results in an error when users try to access patterns.
How?
Relocate where we set
isRoot
based on theisTemplatePartsMode
setting.Testing Instructions
Smoke test sidebar navigation with block themes; confirm it works as before.
emptyhybrid
for Gutenberg env.Screenshots or screencast