-
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
Stabilize isPreviewMode flag #66149
base: trunk
Are you sure you want to change the base?
Stabilize isPreviewMode flag #66149
Conversation
Size Change: +15 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
…vate symbol within RichText block
c30246d
to
72f81d3
Compare
Flaky tests detected in 72f81d3. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11355847467
|
@@ -60,6 +60,7 @@ export function BlockPreview( { | |||
...originalSettings, | |||
focusMode: false, // Disable "Spotlight mode". | |||
__unstableIsPreviewMode: true, |
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.
Can we make this a getter and deprecate it or something?
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.
Maybe we can avoid passing all these __unstableIsPreviewMode configs and instead have a deprecated "property getter" in the default settings that fallbacks to isPreviewMode
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.
As much as I love that idea, it seems incredibly complex to do it right.
We're using the SETTINGS_DEFAULTS
object in many places (defaults.native
, the reducer, the Editor store), and the spread operator does not include the prototype, meaning the getter is not copied. Also, overriding the settings in this file, for example, would also result in the getter not being copied.
So, I think this duplication is better. Unfortunately, we cannot display the deprecation notice because we're not using the getter function. However, a possible workaround would be to add the getter function exclusively to the sub-registries (in this file and the others that override the original settings) and keep the reducers (block editor store, editor store) intact. I believe we can even remove the __unstableIsPreviewMode
property from it directly if we're feeling adventurous.
Why?
It's been almost three years since we introduced
__unstableIsPreviewMode
as a flag to indicate if the block is being rendered as part of the blocks list, block preview, pattern preview, etc. It's time to make it stable.I was motivated after Automattic/jetpack#39768 (comment).
How?
This PR adds
isPreviewMode
as a new default setting key and alongside__unstableIsPreviewMode
whenever it's used as a sub-registry value.Finally, I'm rewriting all internal usages of
__unstableIsPreviewMode
to the newisPreviewMode
flag.