feat: enhance theme handling with custom theme normalization and error reporting#5002
feat: enhance theme handling with custom theme normalization and error reporting#5002OpeOginni wants to merge 7 commits into
Conversation
| function resolveColor( | ||
| value: ColorValue, | ||
| mode: "dark" | "light", | ||
| defs: Record<string, ColorValue>, | ||
| themeColors: Record<string, ColorValue | undefined>, | ||
| themeName: string, | ||
| ): RGBA | { error: string } { | ||
| if (value instanceof RGBA) return value | ||
| if (typeof value === "string") { | ||
| if (value === "transparent" || value === "none") return RGBA.fromInts(0, 0, 0, 0) | ||
| if (value.startsWith("#")) return RGBA.fromHex(value) |
There was a problem hiding this comment.
This code over here just seems insanely complex to me
There was a problem hiding this comment.
Do you think updating the name of the params would reduce the complexity?
The whole function itself just tries to turn the color value from whatever is provided in the many types we allow, to the RGBA type, based on user set defs (line 345) and other set theme attributes (line 346).
There was a problem hiding this comment.
I'm not really sure the cleanest solution but ideally we can make this follow style guide bit better, it isn't very idiomatic rn
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
OpeOginni
left a comment
OpeOginni
left a comment
There was a problem hiding this comment.
Completed Looking through Copilot Style Guide recommendation, and worked with OC on other places the code style could be improved and be a lot more idiomatic
00637c0 to
71e0ba2
Compare
f1ae801 to
08fa7f7
Compare
2 participants
Screen.Recording.2025-12-03.at.00.33.19.mov
Extra for #4429