-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: Fix the patterns route on mobile #67467
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: -1 B (0%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
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.
Fixes the issue for me. Thanks @youknowriad 🚀
Can we also fix the browser console error on navigation? This also appears to be a result of #67199. 9ed79344ca0cdccfc020dfb57a67bbd8.mp4Additionally, it might be a good idea to explore #67505 as a follow-up to this PR (See this comment). |
I'm not sure that JS error is related to the refactoring, it might be related to the refactoring of the sidebar done a long time ago to avoid the Navigation component. cc @jsnajdr |
I think the JS error ( gutenberg/packages/edit-site/src/components/layout/index.js Lines 149 to 161 in 5c76815
which renders It seems that it was really #67199 that broke this, namely the changes in "home route". The old version rendered |
Feels like |
However, the It would be better if the I think that @oandregal is the main expert in this area 🙂 Another questions is: if the Maybe we should fix the mobile sidebar layout code. The part that renders |
I found similar yesterday, and tested rendering the area inside and outside SidebarContent in #67505 as a quick fix. The error pops up for most sidebar navigation in mobile, e.g., #67532 |
Co-authored-by: youknowriad <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: ramonjd <[email protected]>
Co-authored-by: youknowriad <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: ramonjd <[email protected]>
closes #67460
What?
Fix a small regression preventing patterns access on mobile.
Testing Instructions
1- Open the site editor on mobile
2- Click "patterns"
You should see the grid of patterns.