-
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
Combine useSetting functions #31782
Combine useSetting functions #31782
Conversation
Size Change: +11 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
@@ -62,17 +64,20 @@ const deprecatedFlags = { | |||
* const isEnabled = useSetting( 'typography.dropCap' ); | |||
* ``` | |||
*/ | |||
export default function useSetting( path ) { | |||
export default function useSetting( path, name = '', store ) { |
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.
Hi there! I think for me it doesn't make sense to pass a "store" here. (Why are we accessing the editSite store in the block-editor package, what happens if the shape of the edit-site store changes but not the block-editor) It feels like we're sharing code just for the sake of doing so. In these situations, I think it's probably better to leave the duplication in place. what may seem similar may not be in reality (edit-site allows editing these but not block-editor)
Description
After the refactor in #31587 we now have 2
useSetting
functions: one in the block-editor and a 2nd one in the site-editor.This PR combines the 2
useSetting
functions we currently have to avoid code duplication:name
param to the function - default touseBlockEditContext()
if empty.store
param to the function - default toblockEditorStore
if empty.store
params to calls.We will now have a single
useSetting
function, which can be imported from@wordpress/block-editor
.How has this been tested?
Tested the site-editor and block-editor, both seem to be working fine.
Checklist:
*.native.js
files for terms that need renaming or removal).