Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Updating the BlockEditorProvider settings prop should reset the store's settings entirely #51904

Merged
merged 5 commits into from
Jun 27, 2023
Merged

Updating the BlockEditorProvider settings prop should reset the store's settings entirely #51904

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b75e138
Updating the BlockEditorProvider settings prop should reset the store…
youknowriad Jun 26, 2023
a666e86
Add tests for action
getdave Jun 27, 2023
8d1d984
Small refactor per review
youknowriad Jun 27, 2023
5155e3b
Fix tests
youknowriad Jun 27, 2023
fd15da0
Small fix
youknowriad Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions packages/block-editor/src/components/provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,14 @@ export const ExperimentalBlockEditorProvider = withRegistryProvider(
...settings,
__internalIsInitialized: true,
},
stripExperimentalSettings
stripExperimentalSettings,
true
);
}, [ settings ] );
}, [
settings,
stripExperimentalSettings,
__experimentalUpdateSettings,
] );

// Syncs the entity provider with changes in the block-editor store.
useBlockSync( props );
Expand Down
5 changes: 4 additions & 1 deletion packages/block-editor/src/store/private-actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ const privateSettings = [
*
* @param {Object} settings Updated settings
* @param {boolean} stripExperimentalSettings Whether to strip experimental settings.
* @param {boolean} reset Whether to reset the settings.
* @return {Object} Action object
*/
export function __experimentalUpdateSettings(
settings,
stripExperimentalSettings = false
stripExperimentalSettings = false,
reset = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
stripExperimentalSettings = false,
reset = false
{
stripExperimentalSettings = false,
reset = false
}

Nit: these positional arguments are growing and require reading the source of the function in order to understand what a given call is doing.

For example what does true do here (I know the answer but only because I'm reviewing this PR)?

__experimentalUpdateSettings(
	{
		...settings,
		__internalIsInitialized: true,
	},
	stripExperimentalSettings
	stripExperimentalSettings,
	true
);

Perhaps we can consider rewriting to use an object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it's a private action though, we can change our mind at any time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't need to happen now of course.

) {
let cleanSettings = settings;
// There are no plugins in the mobile apps, so there is no
Expand All @@ -50,6 +52,7 @@ export function __experimentalUpdateSettings(
return {
type: 'UPDATE_SETTINGS',
settings: cleanSettings,
reset,
};
}

Expand Down
6 changes: 6 additions & 0 deletions packages/block-editor/src/store/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,12 @@ export function template( state = { isValid: true }, action ) {
export function settings( state = SETTINGS_DEFAULTS, action ) {
switch ( action.type ) {
case 'UPDATE_SETTINGS':
if ( action.reset ) {
return {
...SETTINGS_DEFAULTS,
...action.settings,
};
}
return {
...state,
...action.settings,
Expand Down
8 changes: 0 additions & 8 deletions packages/edit-site/src/components/block-editor/use-site-editor-settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,6 @@ export default function useSiteEditorSettings( templateType ) {
inserterMediaCategories,
__experimentalBlockPatterns: blockPatterns,
__experimentalBlockPatternCategories: blockPatternCategories,
// Temporary fix for bug in Block Editor Provider:
// see: https://github.com/WordPress/gutenberg/issues/51489.
// Some Site Editor entities (e.g. `wp_navigation`) may utilise
// template locking in their settings. Therefore this must be
// explicitly "unset" to avoid the template locking UI remaining
// active for all entities.
templateLock: false,
template: false,
};
}, [ storedSettings, blockPatterns, blockPatternCategories ] );
}