-
Notifications
You must be signed in to change notification settings - Fork 794
feat: Implement user theme preference settings #3160
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
base: master
Are you sure you want to change the base?
Conversation
…e appearance handling - Introduced a new `ThemeAppearance` component to manage user theme preferences, allowing selection between 'auto', 'light', and 'dark' themes. - Updated state management to include `userThemePreference`, replacing the previous `themeAppearance` handling. - Removed deprecated theme appearance logic from various components and files, streamlining the codebase. - Added internationalization support for theme appearance settings across multiple languages. - Enhanced the UI to reflect user-selected theme preferences dynamically.
WalkthroughReplace per-server theme handling with a single persisted global Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant SettingsUI as Settings UI
participant Redux as Store (actions/reducers)
participant Persist as Persisted State
participant Shell
participant TopBar
User->>SettingsUI: open Settings -> select theme option
SettingsUI->>Redux: dispatch SETTINGS_USER_THEME_PREFERENCE_CHANGED(value)
Redux->>Persist: persist userThemePreference (versioned migration)
Redux-->>Shell: state update (userThemePreference changed)
Shell->>Shell: compute currentTheme (userThemePreference === 'auto' ? machineTheme : value)
Shell-->>TopBar: trigger re-render with updated theme/sidebarBg
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Updated the `handleChangeTheme` function to include validation for theme preference values, ensuring only 'auto', 'light', or 'dark' are accepted. This change prevents invalid values from being dispatched, enhancing the robustness of the theme management logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/ui/components/TopBar/index.tsx (1)
10-18: Transparent window background handling is correctly wiredSelector usage and the
sidebarBglogic cleanly ensure the top bar only drops the tint when on macOS with transparency enabled; otherwise it falls back to'tint', which is consistent with prior behavior.If you want to improve readability slightly, you could introduce an
isMacOSconstant (e.g.,const isMacOS = process.platform === 'darwin';) and reuse it in thesidebarBgexpression.Also applies to: 29-29
src/i18n/sv.i18n.json (1)
297-303: Swedish themeAppearance block looks consistent; phrasing nit is optionalKeys and structure match
settings.options.themeAppearance.*usage, and translations are clear. If you want slightly more idiomatic wording, you could consider “Följ systemet” instead of “Följ system”, but this is purely stylistic.src/ui/components/SettingsView/features/ThemeAppearance.tsx (1)
1-74: Component wiring is solid; consider tightening typing and stylingThe overall setup (Redux selector, dispatch, i18n keys, Fuselage usage) looks good and matches the new state shape. Two small improvements you might consider:
- Avoid broad casting of
Keyto the union
Casting viaString(value) as 'auto' | 'light' | 'dark'trusts the runtime entirely. Since the options are known, you can guard explicitly, which keeps TypeScript and runtime aligned:- const handleChangeTheme = useCallback( - (value: Key) => { - const stringValue = String(value) as 'auto' | 'light' | 'dark'; - dispatch({ - type: SETTINGS_USER_THEME_PREFERENCE_CHANGED, - payload: stringValue, - }); - }, - [dispatch] - ); + const handleChangeTheme = useCallback( + (value: Key) => { + const stringValue = String(value); + if (stringValue === 'auto' || stringValue === 'light' || stringValue === 'dark') { + dispatch({ + type: SETTINGS_USER_THEME_PREFERENCE_CHANGED, + payload: stringValue, + }); + } + }, + [dispatch] + );
- Prefer theme tokens over raw inline styles where possible (optional)
Thestyle={{ paddingTop: '4px' }}on the surroundingBoxcould be replaced with a Fuselage spacing prop (e.g.,pt='x4') to stay on theme tokens, if that matches the intended spacing.src/ui/components/Shell/index.tsx (1)
29-31: Theme derivation fromuserThemePreferenceis correct; consider extracting a shared hookUsing
userThemePreference === 'auto' ? machineTheme : userThemePreferenceto drivecurrentThemealigns well with the new preference model, and the narrowing ensuresautonever reachesPaletteStyleTag. To avoid duplicating this logic elsewhere (e.g., other components that need to know the effective theme), consider extracting a small hook likeuseCurrentTheme()that encapsulates this logic and returns aThemesvalue.Also applies to: 40-46
src/i18n/nn.i18n.json (1)
7-13: Nynorsk themeAppearance translations are fine; spelling tweak is optionalThe new
themeAppearanceblock is correctly structured and matches the keys used in code. If you want to polish the phrasing, you might change “Vel fargtemaet for applikasjonen.” to “Vel fargetemaet for applikasjonen.”, but this is non-blocking.src/app/PersistableValues.ts (1)
95-102: PersisteduserThemePreferencewiring looks correct; consider centralizing the union typeDefining
PersistableValues_4_10_0and the>=4.10.0migration withuserThemePreference: 'auto' | 'light' | 'dark'is consistent with the reducer and actions, and defaulting to'auto'makes sense for upgrades.Given this union is now repeated across actions, reducer, persistable type, and UI, it might be worth extracting a shared
UserThemePreferencetype (and perhaps aDEFAULT_USER_THEME_PREFERENCE) to avoid drift if more values are added later.Also applies to: 180-183
src/ui/actions.ts (1)
121-122: NewSETTINGS_USER_THEME_PREFERENCE_CHANGEDaction is consistent; type reuse would helpThe added action constant and its payload type
'auto' | 'light' | 'dark'fit cleanly into the existingUiActionTypeToPayloadMappattern and line up with the reducer and UI.Similar to the persistence layer, you might want to introduce a shared
UserThemePreferencetype and reuse it here (and in the reducer/PersistableValues) instead of repeating the string union in multiple places.Also applies to: 256-256
src/ui/reducers/userThemePreference.ts (1)
1-27: Reducer logic is correct; consider adding runtime validation like other settings reducersThe reducer correctly updates on
SETTINGS_USER_THEME_PREFERENCE_CHANGEDand initializes fromAPP_SETTINGS_LOADED, with a sensible default of'auto'.To harden against malformed persisted settings or incorrectly constructed actions (mirroring e.g.
isTransparentWindowEnabled’s boolean guard), you could validate the payload before accepting it:- case SETTINGS_USER_THEME_PREFERENCE_CHANGED: - return action.payload; + case SETTINGS_USER_THEME_PREFERENCE_CHANGED: { + const value = action.payload; + if (value === 'auto' || value === 'light' || value === 'dark') { + return value; + } + console.warn( + `Invalid payload for ${SETTINGS_USER_THEME_PREFERENCE_CHANGED}:`, + value + ); + return state; + } @@ - case APP_SETTINGS_LOADED: { - const { userThemePreference = state } = action.payload; - return userThemePreference; - } + case APP_SETTINGS_LOADED: { + const { userThemePreference = state } = action.payload; + if ( + userThemePreference === 'auto' || + userThemePreference === 'light' || + userThemePreference === 'dark' + ) { + return userThemePreference; + } + console.warn('Invalid userThemePreference loaded from settings:', userThemePreference); + return state; + }This keeps runtime behaviour robust even if persisted data is corrupted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (40)
src/app/PersistableValues.ts(2 hunks)src/app/selectors.ts(1 hunks)src/i18n/ar.i18n.json(1 hunks)src/i18n/de-DE.i18n.json(1 hunks)src/i18n/en.i18n.json(1 hunks)src/i18n/es.i18n.json(1 hunks)src/i18n/fi.i18n.json(1 hunks)src/i18n/fr.i18n.json(1 hunks)src/i18n/hu.i18n.json(1 hunks)src/i18n/it-IT.i18n.json(1 hunks)src/i18n/ja.i18n.json(1 hunks)src/i18n/nb-NO.i18n.json(1 hunks)src/i18n/nn.i18n.json(1 hunks)src/i18n/no.i18n.json(1 hunks)src/i18n/pl.i18n.json(1 hunks)src/i18n/pt-BR.i18n.json(1 hunks)src/i18n/ru.i18n.json(1 hunks)src/i18n/se.i18n.json(1 hunks)src/i18n/sv.i18n.json(1 hunks)src/i18n/tr-TR.i18n.json(1 hunks)src/i18n/uk-UA.i18n.json(1 hunks)src/i18n/zh-CN.i18n.json(1 hunks)src/i18n/zh-TW.i18n.json(1 hunks)src/i18n/zh.i18n.json(1 hunks)src/injected.ts(0 hunks)src/servers/common.ts(0 hunks)src/servers/preload/api.ts(0 hunks)src/servers/preload/themeAppearance.ts(0 hunks)src/servers/reducers.ts(0 hunks)src/store/rootReducer.ts(2 hunks)src/ui/actions.ts(2 hunks)src/ui/components/ServersView/DocumentViewer.tsx(1 hunks)src/ui/components/ServersView/ServerPane.tsx(0 hunks)src/ui/components/ServersView/index.tsx(0 hunks)src/ui/components/SettingsView/GeneralTab.tsx(2 hunks)src/ui/components/SettingsView/features/ThemeAppearance.tsx(1 hunks)src/ui/components/SettingsView/features/TransparentWindow.tsx(1 hunks)src/ui/components/Shell/index.tsx(1 hunks)src/ui/components/TopBar/index.tsx(2 hunks)src/ui/reducers/userThemePreference.ts(1 hunks)
💤 Files with no reviewable changes (7)
- src/servers/preload/themeAppearance.ts
- src/injected.ts
- src/ui/components/ServersView/ServerPane.tsx
- src/servers/common.ts
- src/ui/components/ServersView/index.tsx
- src/servers/reducers.ts
- src/servers/preload/api.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/app/selectors.tssrc/ui/components/SettingsView/features/ThemeAppearance.tsxsrc/ui/components/TopBar/index.tsxsrc/ui/components/SettingsView/features/TransparentWindow.tsxsrc/ui/reducers/userThemePreference.tssrc/ui/components/ServersView/DocumentViewer.tsxsrc/store/rootReducer.tssrc/ui/components/SettingsView/GeneralTab.tsxsrc/app/PersistableValues.tssrc/ui/actions.tssrc/ui/components/Shell/index.tsx
src/ui/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/ui/**/*.tsx: Implement React UI with functional components and hooks
Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from@rocket.chat/fuselageinstead of raw HTML elements
For Fuselage theming, validate tokens againstTheme.d.tsand only use documented values
Name React component files in PascalCase; non-component files should follow camelCase naming
Files:
src/ui/components/SettingsView/features/ThemeAppearance.tsxsrc/ui/components/TopBar/index.tsxsrc/ui/components/SettingsView/features/TransparentWindow.tsxsrc/ui/components/ServersView/DocumentViewer.tsxsrc/ui/components/SettingsView/GeneralTab.tsxsrc/ui/components/Shell/index.tsx
src/store/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Write Redux actions following the Flux Standard Action (FSA) convention
Files:
src/store/rootReducer.ts
🧠 Learnings (4)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : For Fuselage theming, validate tokens against `Theme.d.ts` and only use documented values
Applied to files:
src/app/selectors.tssrc/ui/components/SettingsView/features/ThemeAppearance.tsxsrc/ui/reducers/userThemePreference.tssrc/ui/components/ServersView/DocumentViewer.tsxsrc/ui/components/SettingsView/GeneralTab.tsxsrc/ui/actions.tssrc/ui/components/Shell/index.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : Implement React UI with functional components and hooks
Applied to files:
src/ui/components/SettingsView/features/ThemeAppearance.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from `rocket.chat/fuselage` instead of raw HTML elements
Applied to files:
src/ui/components/SettingsView/features/ThemeAppearance.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/ui/components/SettingsView/features/TransparentWindow.tsx
🧬 Code graph analysis (4)
src/ui/components/SettingsView/features/ThemeAppearance.tsx (5)
src/ui/reducers/userThemePreference.ts (1)
userThemePreference(11-27)src/store/rootReducer.ts (1)
RootState(111-111)src/store/index.ts (1)
dispatch(38-40)src/store/actions.ts (1)
RootAction(44-46)src/ui/actions.ts (1)
SETTINGS_USER_THEME_PREFERENCE_CHANGED(121-122)
src/ui/components/TopBar/index.tsx (2)
src/ui/reducers/isTransparentWindowEnabled.ts (1)
isTransparentWindowEnabled(11-35)src/store/rootReducer.ts (1)
RootState(111-111)
src/ui/reducers/userThemePreference.ts (3)
src/store/actions.ts (1)
ActionOf(42-42)src/ui/actions.ts (1)
SETTINGS_USER_THEME_PREFERENCE_CHANGED(121-122)src/app/actions.ts (1)
APP_SETTINGS_LOADED(6-6)
src/ui/components/Shell/index.tsx (4)
src/ui/reducers/userThemePreference.ts (1)
userThemePreference(11-27)src/store/rootReducer.ts (1)
RootState(111-111)src/ui/reducers/isTransparentWindowEnabled.ts (1)
isTransparentWindowEnabled(11-35)src/app/reducers/machineTheme.ts (1)
machineTheme(8-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check (macos-latest)
🔇 Additional comments (25)
src/i18n/zh.i18n.json (1)
8-14: LGTM!The themeAppearance translations for simplified Chinese are correctly structured and align with the broader internationalization effort across all locales in this PR.
src/ui/components/SettingsView/features/TransparentWindow.tsx (1)
61-61: LGTM!Minor formatting adjustment to add a trailing newline for consistency.
src/i18n/zh-TW.i18n.json (1)
104-110: LGTM!The themeAppearance translations for Traditional Chinese are correctly added and consistent with the multilingual rollout of this feature.
src/i18n/uk-UA.i18n.json (1)
107-113: LGTM!The Ukrainian translations for themeAppearance are correctly structured and align with the feature's internationalization requirements.
src/i18n/no.i18n.json (1)
298-304: LGTM!The Norwegian translations for themeAppearance are properly added with the correct structure and placement within the settings configuration.
src/i18n/tr-TR.i18n.json (1)
125-131: LGTM!The Turkish translations for themeAppearance are correctly implemented and consistent with the internationalization pattern used across all locales in this PR.
src/ui/components/SettingsView/GeneralTab.tsx (2)
14-14: LGTM!The ThemeAppearance component import is correctly added.
34-34: LGTM!The ThemeAppearance component is properly integrated into the GeneralTab settings. Rendering it unconditionally (without platform-specific logic) is appropriate since theme preferences should be available across all platforms.
src/i18n/ar.i18n.json (1)
8-14: LGTM!The Arabic translations for themeAppearance are correctly structured and complete the internationalization coverage for this feature across all supported locales.
src/i18n/hu.i18n.json (1)
288-295: NewthemeAppearancetranslations look consistent and correctly wiredThe new
settings.options.themeAppearanceblock is structurally correct JSON, positioned consistently next totransparentWindow, and the Hungarian strings match the intended semantics for the theme selector.src/app/selectors.ts (1)
74-75: Selector wiring foruserThemePreferenceis correct and consistentAdding
userThemePreferencetoselectPersistableValuesfollows the existing pattern and will expose the new state field to persistence without altering selector behavior.src/i18n/en.i18n.json (1)
297-304: EnglishthemeAppearancestrings and placement look goodThe
themeAppearanceblock is added undersettings.optionswith clear, consistent copy and valid JSON; keys match those expected by the new ThemeAppearance UI.src/i18n/se.i18n.json (1)
7-13: SwedishthemeAppearancelocalization matches the new settingThe new
themeAppearanceentry is correctly nested undersettings.optionsand the Swedish strings accurately convey theme selection options.src/i18n/ru.i18n.json (1)
266-272: RussianthemeAppearanceblock is accurate and well-integratedThe added Russian translations for theme selection are clear, idiomatic, and correctly placed under
settings.optionsalongsidetransparentWindow.src/i18n/nb-NO.i18n.json (1)
7-13: NorwegianthemeAppearanceoption is correctly addedThe
themeAppearancelocalization undersettings.optionshas appropriate Bokmål phrasing and valid JSON, aligning with other locale implementations.src/i18n/es.i18n.json (1)
288-294: SpanishthemeAppearancestrings are clear and consistentThe new
themeAppearancesection is properly nested and the Spanish copy cleanly communicates the theme options (auto,light,dark) in line with other locales.src/i18n/fr.i18n.json (1)
263-269: FrenchthemeAppearancelocalization fits well with existing settingsThe added
themeAppearanceblock is structurally correct and the French translations accurately describe the theme preference options while matching the surrounding style.src/i18n/it-IT.i18n.json (1)
7-13: Italian themeAppearance i18n entry looks goodThe new
themeAppearanceblock is well-structured, matches the existing options pattern, and the labels are clear for Italian users.src/i18n/pl.i18n.json (1)
136-142: Polish themeAppearance translations are consistentThe
themeAppearanceentry mirrors the structure and semantics of other locales, and the Polish wording is clear and aligned with existing settings text.src/i18n/ja.i18n.json (1)
221-229: Japanese themeAppearance block is well-formedKeys and structure are correct, and the Japanese strings clearly describe the theme options (auto/light/dark) in line with other locales.
src/i18n/pt-BR.i18n.json (1)
280-289: pt-BR themeAppearance strings are clear and consistentThe new
themeAppearanceoption is correctly placed undersettings.options, and the labels/readings in pt-BR are natural and consistent with the rest of the file.src/i18n/zh-CN.i18n.json (1)
158-167: zh-CN themeAppearance translations look correctThe
themeAppearanceblock follows the existing options structure, and the Chinese strings accurately convey the theme choices (follow system/light/dark).src/i18n/de-DE.i18n.json (1)
262-271: German themeAppearance option is properly localizedThe
themeAppearanceentry is structurally correct and the German phrasing (“Design”, “Farbschema”, “System folgen”) is clear and consistent with the rest of the settings.src/store/rootReducer.ts (1)
44-45: userThemePreference reducer integration into rootReducer is correctImporting
userThemePreferenceand adding it tocombineReducerscleanly exposes the new slice onRootState, aligning with the rest of the UI reducers and supporting the new theme preference flow.Also applies to: 87-87
src/i18n/fi.i18n.json (1)
264-270: ThemeAppearance i18n block is correctly structured and keyed.The Finnish translation block is properly aligned with the
ThemeAppearance.tsxcomponent, which uses exactly these keys:auto,light,dark,title, anddescription. The Redux reducer and application constants confirm acceptance of these three theme options. All 25 language files follow the same structure consistently. The translations are semantically accurate.
…ate for user preferences - Replaced the use of `useDarkMode` with Redux selectors to determine the theme based on user preferences and machine theme. - Enhanced theme logic to support 'auto', 'light', and 'dark' settings, improving the flexibility and responsiveness of the theme management in the DocumentViewer component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/ui/components/SettingsView/features/ThemeAppearance.tsx (1)
70-70: Prefer Fuselage spacing props over inline styles.The inline
style={{ paddingTop: '4px' }}should use Fuselage's spacing props for consistency with the design system. Consider usingpaddingBlockStart='x4'or an appropriate Fuselage spacing token.Apply this diff to use Fuselage spacing props:
- <Box display='flex' alignItems='center' style={{ paddingTop: '4px' }}> + <Box display='flex' alignItems='center' paddingBlockStart='x4'>Based on coding guidelines requiring validation of tokens against Theme.d.ts and using Fuselage components properly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/i18n/nn.i18n.json(1 hunks)src/ui/components/SettingsView/features/ThemeAppearance.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/ui/components/SettingsView/features/ThemeAppearance.tsx
src/ui/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/ui/**/*.tsx: Implement React UI with functional components and hooks
Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from@rocket.chat/fuselageinstead of raw HTML elements
For Fuselage theming, validate tokens againstTheme.d.tsand only use documented values
Name React component files in PascalCase; non-component files should follow camelCase naming
Files:
src/ui/components/SettingsView/features/ThemeAppearance.tsx
🧠 Learnings (3)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : For Fuselage theming, validate tokens against `Theme.d.ts` and only use documented values
Applied to files:
src/ui/components/SettingsView/features/ThemeAppearance.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : Implement React UI with functional components and hooks
Applied to files:
src/ui/components/SettingsView/features/ThemeAppearance.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from `rocket.chat/fuselage` instead of raw HTML elements
Applied to files:
src/ui/components/SettingsView/features/ThemeAppearance.tsx
🧬 Code graph analysis (1)
src/ui/components/SettingsView/features/ThemeAppearance.tsx (5)
src/ui/reducers/userThemePreference.ts (1)
userThemePreference(11-27)src/store/rootReducer.ts (1)
RootState(111-111)src/store/index.ts (1)
dispatch(38-40)src/store/actions.ts (1)
RootAction(44-46)src/ui/actions.ts (1)
SETTINGS_USER_THEME_PREFERENCE_CHANGED(121-122)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
🔇 Additional comments (5)
src/i18n/nn.i18n.json (1)
7-14: LGTM! Translations are properly structured.The Norwegian Nynorsk translations for theme appearance settings are correctly formatted and include all required keys that the ThemeAppearance component expects.
src/ui/components/SettingsView/features/ThemeAppearance.tsx (4)
1-16: LGTM! Imports follow project conventions.The imports correctly use Fuselage components from
@rocket.chat/fuselageand follow the functional component with hooks pattern as specified in the coding guidelines.
29-45: Excellent input validation!The validation logic in
handleChangeThemecorrectly guards against invalid values before dispatching the action. The type assertion on line 41 is safe because of the preceding validation checks.
47-54: LGTM! Options are properly memoized.The options array is correctly generated using i18n translations and memoized with the appropriate dependency on the translation function.
22-27: LGTM! State management is correctly implemented.The component properly uses Redux hooks with correct typing and state selection that aligns with the reducer structure defined in
src/ui/reducers/userThemePreference.ts.
… dependencies - Eliminated the use of Redux selectors for theme management in the DocumentViewer component, replacing it with a static 'tint' background and default color settings. - Streamlined the component's code by removing unnecessary theme logic, enhancing readability and maintainability.
…arance, TransparentWindow, and userThemePreference files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/ui/components/SettingsView/features/ThemeAppearance.tsx (1)
58-76: Consider replacing inline style with Fuselage layout props or theme-aware spacingThe
style={{ paddingTop: '4px' }}on the right‑hand Box is purely cosmetic but slightly diverges from Fuselage/theming conventions. You could instead use spacing/layout props (e.g., margin/padding props or a class backed by theme tokens) to keep styling consistent and theme-friendly.src/ui/reducers/userThemePreference.ts (1)
11-22: Centralize the'auto' | 'light' | 'dark'union into a shared typeThe theme preference union is used here and in
ThemeAppearance.tsx. To avoid drift and make future additions safer, consider extracting a sharedtype UserThemePreference = 'auto' | 'light' | 'dark';(e.g., in a smallui/typesor similar module) and using it in:
- This reducer’s state type and APP_SETTINGS_LOADED payload.
- The action payload type in
ui/actions.ts.- The
ThemeAppearancecomponent’suserThemePreferenceand dispatch payload.This keeps all call sites in sync if you ever introduce more modes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/ui/components/SettingsView/features/ThemeAppearance.tsx(1 hunks)src/ui/components/SettingsView/features/TransparentWindow.tsx(1 hunks)src/ui/reducers/userThemePreference.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/ui/components/SettingsView/features/ThemeAppearance.tsxsrc/ui/components/SettingsView/features/TransparentWindow.tsxsrc/ui/reducers/userThemePreference.ts
src/ui/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/ui/**/*.tsx: Implement React UI with functional components and hooks
Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from@rocket.chat/fuselageinstead of raw HTML elements
For Fuselage theming, validate tokens againstTheme.d.tsand only use documented values
Name React component files in PascalCase; non-component files should follow camelCase naming
Files:
src/ui/components/SettingsView/features/ThemeAppearance.tsxsrc/ui/components/SettingsView/features/TransparentWindow.tsx
🧠 Learnings (4)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : For Fuselage theming, validate tokens against `Theme.d.ts` and only use documented values
Applied to files:
src/ui/components/SettingsView/features/ThemeAppearance.tsxsrc/ui/reducers/userThemePreference.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : Implement React UI with functional components and hooks
Applied to files:
src/ui/components/SettingsView/features/ThemeAppearance.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from `rocket.chat/fuselage` instead of raw HTML elements
Applied to files:
src/ui/components/SettingsView/features/ThemeAppearance.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/ui/components/SettingsView/features/TransparentWindow.tsx
🧬 Code graph analysis (2)
src/ui/components/SettingsView/features/ThemeAppearance.tsx (4)
src/ui/reducers/userThemePreference.ts (1)
userThemePreference(11-27)src/store/rootReducer.ts (1)
RootState(111-111)src/store/index.ts (1)
dispatch(38-40)src/ui/actions.ts (1)
SETTINGS_USER_THEME_PREFERENCE_CHANGED(121-122)
src/ui/reducers/userThemePreference.ts (3)
src/store/actions.ts (1)
ActionOf(42-42)src/ui/actions.ts (1)
SETTINGS_USER_THEME_PREFERENCE_CHANGED(121-122)src/app/actions.ts (1)
APP_SETTINGS_LOADED(6-6)
🪛 GitHub Check: check (ubuntu-latest)
src/ui/components/SettingsView/features/ThemeAppearance.tsx
[failure] 82-82:
Delete ⏎
src/ui/components/SettingsView/features/TransparentWindow.tsx
[failure] 61-61:
Delete ⏎⏎
src/ui/reducers/userThemePreference.ts
[failure] 28-28:
Delete ⏎
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
🔇 Additional comments (2)
src/ui/components/SettingsView/features/ThemeAppearance.tsx (1)
22-77: ThemeAppearance wiring and validation look solidNice use of Redux + hooks: selector/dispatch are correctly typed,
handleChangeThemedefensively filters values to'auto' | 'light' | 'dark', and options are memoized with i18n-backed labels. The Select wiring (options,value,onChange) matches the expected shape.src/ui/reducers/userThemePreference.ts (1)
11-26: Reducer behavior and typing look correctThe reducer cleanly handles both the settings change action and initialization from
APP_SETTINGS_LOADED, with a sensible'auto'default and a fallback whenuserThemePreferenceis missing from older payloads. Typing viaActionOf<...>keeps the action union tight.
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop the extra trailing newline to match repo formatting
Static checks are complaining about the blank line at the very end of the file. Removing it should get the formatter/CI happy again.
export const ThemeAppearance = (props: ThemeAppearanceProps) => {
@@
);
};
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }; | |
| }; |
🧰 Tools
🪛 GitHub Check: check (ubuntu-latest)
[failure] 82-82:
Delete ⏎
🤖 Prompt for AI Agents
In src/ui/components/SettingsView/features/ThemeAppearance.tsx around lines 81
to 82, there is an extra trailing newline at the end of the file; remove the
blank line so the file ends immediately after the closing brace/semicolon to
match repository formatting and satisfy static checks/CI.
| }; | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing blank lines to satisfy formatting checks
The only change here is two extra blank lines at the end of the file, and CI is flagging them. Please drop the trailing empties so the formatter/linter passes.
return (
<Field className={props.className}>
@@
);
};
-
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }; | |
| }; |
🧰 Tools
🪛 GitHub Check: check (ubuntu-latest)
[failure] 61-61:
Delete ⏎⏎
🤖 Prompt for AI Agents
In src/ui/components/SettingsView/features/TransparentWindow.tsx around lines
60-62, the file contains two extra trailing blank lines at EOF; remove the extra
blank lines so the file ends immediately after the final closing token and
ensure there is only a single newline at the end of file (no multiple empty
lines) to satisfy the formatter/linter.
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing newline at EOF to satisfy formatting checks
CI is flagging the extra blank line at the end of this file. Trimming it should resolve the check failure.
export const userThemePreference: Reducer<
@@
}
};
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }; | |
| }; |
🧰 Tools
🪛 GitHub Check: check (ubuntu-latest)
[failure] 28-28:
Delete ⏎
🤖 Prompt for AI Agents
In src/ui/reducers/userThemePreference.ts around lines 27-28, the file ends with
an extra blank line (trailing newline) that breaks formatting checks; remove the
blank line at EOF so the file ends immediately after the closing brace/semicolon
with no additional empty line.
ThemeAppearancecomponent to manage user theme preferences, allowing selection between 'auto', 'light', and 'dark' themes.userThemePreference, replacing the previousthemeAppearancehandling.Summary by CodeRabbit
New Features
Behavior Change
Internationalization
✏️ Tip: You can customize this high-level summary in your review settings.