Rename and stabilize useEditorFeature as useSetting #31587
Conversation
youknowriad
added
Needs Dev Note
youknowriad
requested review from
oandregal,
gziolo,
jorgefilipecosta and
aaronrobertshaw
youknowriad
requested review from
ajitbohra,
cameronvoell,
ellatrix and
guarani
as code owners
youknowriad
force-pushed
the
update/use-theme-setting
branch
from
bb26920 to
d9b24ee
Compare
|
Size Change: -26 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
|
I'm not stoked with the name |
|
Fair,
I think this particular API should be thought more in relation to the block-editor package semantics and not WordPress semantics, in other words, whatever name we choose here is the same one that should be used to pass these settings to the block editor provider Right now, we use I think it's a bit hard to reply to this question but right now I can't think of anything better than |
I think those are great frames of mind for this. I see two paths forward:
Isn't this at odds with separating the block-editor package from WordPress semantics suggested above? Independently from what direction/name we chose for this API, I was thinking that this data could be structured in the store in this way:
|
|
I'm not sure we should make this API retrieve any block-editor setting because setting a "media uploader" or a "link fetcher" or "reusable blocks" is not something that can be mixed with "allowing colors" or "allowed image sizes".
Not sure I understand this.. The block can't set/unset a setting?
No in the sense that I think that a "theme" is something that can have meaning in any block-editor. "Theme" might not be the right word though but it's the best one that I can think of right now.
Yes maybe, it does make So it seems we have two options:
To give more context to the discussion, here are some of the top level settings that we already have. Do we think that these settings can make sense as well in a |
|
Sorry, I messed up the words. I meant:
|
youknowriad
changed the title
|
Renamed to just useSetting per the discussion here :) |
youknowriad
force-pushed
the
update/use-theme-setting
branch
from
9f2e89c to
33973ec
Compare
|
To make sure I follow: if we go with I'm fine if we do that in a follow-up PR, just want to confirm I understand the direction. |
Maybe later, it doesn't matter right now, I expect over time most of these settings will disappear in favor of theme.json related ones. Also the internal implementation of the hook is not important for me. |
|
I do think thought that for 5.8, |
So the idea is, at the moment, only expose things configured via theme.json. I see how that can be confusing for consumers in the interim, but I also understand that there's still a mechanism to query the whole settings object (the store's selector used by the hook), so we should be fine. The important part to keep stable for consumers is the path ( |
| @@ -50,19 +50,19 @@ const deprecatedFlags = { | |||
| }; | |||
|
|
|||
| /** | |||
| * Hook that retrieves the setting for the given editor feature. | |||
| * Hook that retrieves the editor setting. | |||
There was a problem hiding this comment.
Should we clarify this to state it's not for all settings, but #31587 (comment) ?
youknowriad
removed
the
Needs Dev Note
Related #31416
This is an important API for Global Settings (theme.json) APIs