fix: add checks for preventing HSL colors from entering React state #9893
Conversation
| @@ -30,13 +31,17 @@ export type AppearanceSettingsPageViewProps = { | |||
| preview: boolean, | |||
| ) => void; | |||
| }; | |||
|
|
|||
| const fallbackBgColor = hslToHex(colors.blue[7]); | |||
There was a problem hiding this comment.
KISS-related question: why do we need other changes in the PR? Did you identify any places where we can face HSL related issue?
EDIT:
I'm thinking about changes around the DashboardProvider.
There was a problem hiding this comment.
Yeah, KISS is definitely something I need to keep in mind more, but at least here, I feel like some kind of defensive changes (maybe not necessarily these) made sense.
The way I see it, there's a few risks right now:
DashboardProvideris already a top-level component that can be imported by anything else from the/componentsfolder. It's also a context provider, so any importing component's sub-children would be able to use it indirectly, as well- The component is already being globally available to some degree because it's directly integrated into the
RequireAuthcomponent - So many constants in the codebase are set up as HSL strings for readability. We could get rid of these in favor of hex strings, but they're a lot less intuitive to work with
I felt like having the provider be in charge of validating incoming state changes would be the best way to prevent any future mismatches from popping up
There was a problem hiding this comment.
Thanks for the explanation! Sounds good to me 👍
| @@ -30,13 +31,17 @@ export type AppearanceSettingsPageViewProps = { | |||
| preview: boolean, | |||
| ) => void; | |||
| }; | |||
|
|
|||
| const fallbackBgColor = hslToHex(colors.blue[7]); | |||
There was a problem hiding this comment.
Thanks for the explanation! Sounds good to me 👍
Closes #9888
Changes
AppearanceSettingsViewto prevent HSL colors from accidentally entering the componentDashboardProvider's state