-
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
Fix: Shadow/Font size preset panel crashes the editor #65765
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: +12 B (0%) Total Size: 1.77 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.
This looks good 👍 Thanks for the fix!
Long-term, I agree with @ciampo that ideally the component would handle this - errors like these are consequences of keeping an open API with regards to those params.
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.
Agreed. LGTM as an urgent fix. I'll see if I can create a slightly more sophisticated fix , still in app-land — it will help us to understand how to handle this better at the Navigator
-level.
The E2E failure doesn't seem related, and can be encountered across many other PRs.
Thanks for the review! I think the E2E test failure is related to the core, so I would like to merge it once the core is fixed. Core ticket: https://core.trac.wordpress.org/ticket/60666 |
Thank you! I've also created a PR with an alternative solution that could avoid the "flash of empty screen" while transitioning out #65790 |
Another critical error was found regarding font size presets: #65774 I believe that the issue is not related to #64777, but simply a problem with the font size preset logic itself, and it also occurs in WP 6.7. On the other hand, this PR does not need to be backported to WP6.7. Therefore, it might be better to merge this PR after backporting the fix for #65774 to WP 6.7. |
@@ -46,6 +46,10 @@ function FontSize() { | |||
|
|||
const [ globalFluid ] = useGlobalSetting( 'typography.fluid' ); | |||
|
|||
if ( ! origin || ! slug ) { |
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.
Oh nice! I promise I didn't steal this for #65811
😆
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.
Tested quickly in playground. Thanks for catching this so quickly.
I updated this PR as #65791 was merged. The following warning was raised, so I moved the location of
The issue reported in #65711 has been resolved as before in this PR, and #65774 should not have reoccurred: d3f08e437dc371600e5ed42fc6dcc687.mp4 |
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 🚀
As mentioned before, the real fix should be applied in Navigator
. Once that's done, we'll be able to rework the code, removing the early returns (and using goTo
only as a fail-safe for other use-cases)
Flaky tests detected in dd9380b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11142791584
|
* Fix: Shadow/Font size preset panel crashes the editor * Refactor logic Co-authored-by: t-hamano <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: ciampo <[email protected]> Co-authored-by: ramonjd <[email protected]>
…ess#65765)" This reverts commit f162973.
* Fix: Shadow/Font size preset panel crashes the editor * Refactor logic Co-authored-by: t-hamano <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: ciampo <[email protected]> Co-authored-by: ramonjd <[email protected]>
Temporary fix for #65711
Related to #64777
What?
This PR fixes a critical error that occurs when returning to the previous screen from the shadow presets panel or the font presets panel.
This fix isn't ideal, but we need to prevent the issue from shipping in the upcoming Gutenberg 19.4.
Why?
I found that the problem was caused by #64777.
From this comment:
How?
Perform an early return to avoid a critical error when the parameter is undefined. One ideal future solution is suggested in this comment.
Testing Instructions
Screenshots or screencast
93efe80e1db33057473dc1044f3d0ccc.mp4