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.

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

useSetting( 'color' ) does not consider deprecated flag #37094

Closed
oandregal opened this issue Dec 3, 2021 · 3 comments · Fixed by #37428
Closed

useSetting( 'color' ) does not consider deprecated flag #37094

oandregal opened this issue Dec 3, 2021 · 3 comments · Fixed by #37428
Assignees
Labels
[Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@oandregal
Copy link
Member

oandregal commented Dec 3, 2021

Description

useSetting allows consumers to access data coming from theme.json. For a few cases, we introduced logic to fallback to the corresponding deprecated setting (see), so userSetting( 'color.custom' ) will fallback to settings.disableCustomColors or typography.lineHeight will fallback to settings.enableCustomLineHeight.

While this works when accessing the setting directly, it doesn't when the consumer accesses the top-level key: useSetting( 'color' ) does not consider the special handling for color.custom and so on.

@oandregal oandregal added [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended labels Dec 3, 2021
@jorgefilipecosta
Copy link
Member

I'm not sure if we should fix this issue. useSetting('color') should not be used unless one really wants the raw value as they are in the structure. On #36841 I missed the fact that in one of the usages we need the back-compatibility and that was the issue.

@oandregal
Copy link
Member Author

I don't have strong opinions other than fixing the current status, which is bug-prone. Both options (adding the backcompatibility logic or don't allow using color directly) work for me.

@oandregal
Copy link
Member Author

For the record, @youknowriad also argued that we shouldn't allow using useSetting with top-level keys: #36900 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants