Edit: Pass editor features dynamically #25795
Conversation
jorgefilipecosta
added
[Type] Enhancement
|
Size Change: +574 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
jorgefilipecosta
requested review from
ajitbohra and
chrisvanpatten
as code owners
jorgefilipecosta
force-pushed
the
update/pass-editor-features-dynamically
branch
2 times, most recently
from
94668b5 to
d9622aa
Compare
|
What's the advantage of doing this over using the existing settings already processed and stored in |
Hi @nosolosw, The user can change the settings eg: change the color palette so we need to merge the user settings with theme settings and pass these settings to the blockeditor. select('core/block-editor').getSettings().__experimentalSettings just returns the settings at the time the server processed them it is not aware of the changes the user did to the settings. |
|
Ah, just reviewed the other PR and now I understand how this whole thing is tied together 👍 |
|
So, as I understand this, we want to update the |
jorgefilipecosta
force-pushed
the
update/pass-editor-features-dynamically
branch
from
d9622aa to
72dbefe
Compare
jorgefilipecosta
force-pushed
the
update/pass-editor-features-dynamically
branch
from
72dbefe to
aa89774
Compare
Hi @nosolosw, |
packages/edit-site/src/components/editor/global-styles-provider.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/editor/global-styles-provider.js
Outdated
Show resolved
Hide resolved
| // Deep clone from base data. | ||
| // | ||
| // We don't use cloneDeep from lodash here | ||
| // because we know the data is JSON compatible, | ||
| // see https://github.com/lodash/lodash/issues/1984 | ||
| const mergedTree = JSON.parse( JSON.stringify( baseData ) ); | ||
|
|
||
| const styleKeys = [ 'typography', 'color' ]; | ||
| const styleKeys = [ 'typography', 'color', 'custom', 'spacing' ]; |
There was a problem hiding this comment.
custom and spacing are not valid style keys, they are only relevant for settings. We probably need to port the normalize block schema (and its function) from the (server).
There was a problem hiding this comment.
I renamed the variable to validMergeKeys so it can contain keys valid for style and settings. I think ideally we should update the function to no even need to rely on this hardcoded data.
There was a problem hiding this comment.
But for now, it seems in an acceptable solution following a similar approach to what we had.
There was a problem hiding this comment.
Hey, I hope you don't mind, but I submitted a commit at 6652d28 with a simple fix for this (it can be improved in subsequent PRs).
There was a problem hiding this comment.
Thank you for the enhancements :)
…r.js Co-authored-by: O André <[email protected]>
…r.js Co-authored-by: O André <[email protected]>
jorgefilipecosta
force-pushed
the
update/pass-editor-features-dynamically
branch
from
8a6b1b5 to
8d1789a
Compare
|
Pushed 701a6a2 to protect against a WSOD when baseStyles are undefined. |
| const settings = useSelect( ( select ) => | ||
| select( 'core/edit-site' ).getSettings() | ||
| ); |
There was a problem hiding this comment.
This useSelect call is missing a dependency array.
This PR applies a change that dynamically passes the block editor features to the edit-site.
The user can change these features now, so we need to compute them dynamically.
This PR is a follow-up to #25711, and with it, after the user changes the color palette, the change is immediately reflected on the block editor, and we can see the colors available changed.