feat(web): improve theme system clarity and fix login page theme sele… #5203
Conversation
…ction - Add descriptive labels to distinguish workspace vs user theme settings - Fix login page theme selection not reflecting in dropdown - Add ThemeInfoCard component explaining theme system - Enhance ThemeSelect with effective theme indicators - Improve UX by clarifying theme scope and priority Resolves theme confusion between System > General > Theme and System > Basic > Edit > Theme
- Remove detailed theme system explanation from user preferences - Keep simple descriptive label for user theme preference - Maintain clean UI without technical details
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the theme management system to support both workspace-level default themes and user-level theme preferences. The changes allow user preferences to override workspace defaults and improve the UI to clearly communicate the theme hierarchy.
Key changes:
- Enhanced
ThemeSelectcomponent to show effective theme when user preference differs from workspace default - Updated settings UI to clarify distinction between workspace default theme and personal theme preferences
- Added local theme state management for the authentication page where user preferences cannot be persisted
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| web/src/components/ThemeSelect.tsx | Added observer wrapper, effective theme calculation, and visual indicators for active theme |
| web/src/components/Settings/WorkspaceSection.tsx | Updated label and added description to clarify workspace default theme setting |
| web/src/components/Settings/PreferencesSection.tsx | Added description explaining that personal theme overrides workspace default |
| web/src/components/AuthFooter.tsx | Implemented local state management for theme changes on login page with direct DOM updates |
Comments suppressed due to low confidence (1)
web/src/components/ThemeSelect.tsx:27
- Unused variable displayTheme.
const displayTheme = showEffectiveTheme ? effectiveTheme : currentTheme;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Calculate effective theme (user preference overrides workspace default) | ||
| const effectiveTheme = userStore.state.userGeneralSetting?.theme || workspaceStore.state.theme || "default"; | ||
|
|
||
| const displayTheme = showEffectiveTheme ? effectiveTheme : currentTheme; |
There was a problem hiding this comment.
The variable displayTheme is declared but never used. Consider removing it or using it in the Select component's value prop if it was intended to display the effective theme.
| const displayTheme = showEffectiveTheme ? effectiveTheme : currentTheme; |
|
|
||
| const displayTheme = showEffectiveTheme ? effectiveTheme : currentTheme; | ||
|
|
||
| const handleThemeChange = (newTheme: Theme) => { |
There was a problem hiding this comment.
The Theme type is referenced but not imported. Add import type { Theme } from \"@/store\"; to the imports at the top of the file.
|
|
||
| const AuthFooter = observer(({ className }: Props) => { | ||
| // Local state for login page theme since we can't persist to server | ||
| const [localTheme, setLocalTheme] = useState(workspaceStore.state.theme || "default"); |
There was a problem hiding this comment.
The initial state value uses workspaceStore.state.theme which only runs once on component mount. If the workspace theme changes externally, the component won't reflect this update. Consider using useState(() => workspaceStore.state.theme || \"default\") or directly using workspaceStore.state.theme without local state since the component is already wrapped with observer.
1 participant
…ction
Resolves theme confusion between System > General > Theme and System > Basic > Edit > Theme