-
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
Introduce ExperimentalBlockEditorProvider #47319
Conversation
packages/editor/src/index.js
Outdated
import { ExperimentalEditorProvider } from './components/provider'; | ||
import { lock } from './experiments'; | ||
|
||
export const experiments = {}; |
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 export is in here and not in experiments.js to avoid a circular dependency between experiments.js
and ./components/provider/index.ts
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.
I'd prefer this to be consistent across packages so maybe we should export experiments
from index.js
in all the other packages too?
Or split experiments.js
into experiments.js
and lock-unlock.js
.
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, let's make it consistent across packages. That's out of scope here, though – let's do it in a follow up PR
Size Change: +894 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
2513840
to
2c0f39a
Compare
Flaky tests detected in 9e1fa36. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4016530130
|
I added unit tests. There's still a problem with the native tests. |
This PR is blocked by #47421 |
718cc49
to
f50bb1f
Compare
This PR introduces `ExperimentalBlockEditorProvider` that prevents mixing experimental editor settings and the public `BlockEditorProvider` component. The actual filtering is handled in a private `__experimentalUpdateSettings` selector in the block-editor store. Experimental settings may still be set via a PHP plugin – limiting this vector would require a separate PR.
f50bb1f
to
cc28354
Compare
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 LGTM! We'll wait for mobile folks to test the native
files' changes and we can 🚢 .
Thanks Adam!
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.
Changes look good. I didn't notice any issues running the changes in the mobile demo app.
Description
This PR introduces
ExperimentalBlockEditorProvider
that prevents mixing experimental editor settings and the publicBlockEditorProvider
component. The actual filtering is handled in a private__experimentalUpdateSettings
selector in the block-editor store.Experimental settings may still be set via a PHP plugin – limiting this vector would require a separate PR.
Rationale
WordPress extenders cannot update the experimental block settings on their own. The
updateSettings()
actions of the@wordpress/block-editor
store will filter out all the settings that are not a part of the public API. The only way to actually store them is via private action.__experimentalUpdateSettings()
.To privatize a block editor setting, add it to the
privateSettings
list in/packages/block-editor/src/store/actions.js:
Mobile apps
This PR updates a few
.native.js
files. Mobile apps choose to import the.native.js
files over their.js
counterparts whenever possible. At the same time, this PR introduced the expectation of having two extra named exports:ExperimentalEditorProvider
andExperimentalBlockEditorProvider
. I updated the native files to export the regular Provider component under the experimental name. This works because the settings filtering is restricted to the web – it doesn't make sense in context of mobile apps anyway.Testing instructions
cc @ntsekouras @youknowriad