feat(desktop): custom theme system — user-editable color variables, named themes, cross-device sync - #730
feat(desktop): custom theme system — user-editable color variables, named themes, cross-device sync#730h4yfans wants to merge 10 commits into
Conversation
…de, theme sync item
…ing + custom_themes migration
…me with custom overrides, portable customThemeId
…use-theme-sync custom theme resolution
…with hex+picker, built-in switch clears custom
…es for architecture boundary
…rchitecture notes; spec DB correction
|
React Doctor found 2 new issues in 1 file · 2 warnings · score 92 / 100 (Great) · 0 fixed · vs 2 warnings
Reviewed by React Doctor for commit |
| clearCustomThemeVariables(root) | ||
|
|
||
| expect(root.style.getPropertyValue('--background')).toBe('') | ||
| expect(clearCustomThemeVariables(root)).toBeUndefined() |
| const fallback = useMemo( | ||
| () => computedVarValue(def.cssVar), | ||
| // Re-read the base value whenever the override toggles off. | ||
| [def.cssVar, override] |
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useMemo re-runs whenever override changes even though it never uses it.
Fix → Don't blindly add missing dependencies. Read the hook callback first.
Bad:
useEffect(() => {
setCount(count + 1);
}, [count]);
Better:
useEffect(() => {
setCount((currentCount) => currentCount + 1);
}, []);
If the missing value is recreated every render, move it inside the hook or stabilize it before adding it to deps.
| useEffect(() => { | ||
| setName(theme.name) | ||
| setVariables(theme.variables) | ||
| }, [theme.id]) |
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useEffect can run with a stale theme.name, theme.variables & show your users old data.
Fix → Don't blindly add missing dependencies. Read the hook callback first.
Bad:
useEffect(() => {
setCount(count + 1);
}, [count]);
Better:
useEffect(() => {
setCount((currentCount) => currentCount + 1);
}, []);
If the missing value is recreated every render, move it inside the hook or stabilize it before adding it to deps.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
What
.memry/themes/<slug>.jsonin the vault + additivecustom_themestable (migration 0035) as sync/clock source of truththemesync item type (whole-item LWW per theme) +general.customThemeIdsynced field;themestays at the base value so older clients render the base themeNotes