feat: add support for theme override #1591
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rohanchkrabrty
changed the title
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds theme selection capability to the organization profile and preferences system. It introduces a Theme type, extends routing and component interfaces to propagate theme and onThemeChange props, updates the router context to include theme fields, and modifies preference pages to render theme selection UI. ChangesTheme Support Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Coverage Report for CI Build 25430271924Coverage remained the same at 41.963%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/sdk/react/views-new/preferences/preferences-view.tsx (1)
62-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWrong
ViewHeaderdescription — stale copy-paste
"Manage members for this domain."is clearly copied from the members view. This is user-visible text on the Preferences page.🐛 Proposed fix
- description="Manage members for this domain." + description="Customize your workspace preferences and settings."
🧹 Nitpick comments (3)
web/sdk/react/views/preferences/preferences-page.tsx (1)
75-75: 💤 Low value
Themetype is duplicated — import fromroutes.tsxinstead
Themeis already exported fromweb/sdk/react/components/organization/routes.tsx. Defining it again here means any future change (e.g., adding a new theme value) must be updated in multiple places.♻️ Proposed fix
Remove the local type alias and import from the canonical location:
+import type { Theme } from '~/react/components/organization/routes'; -type Theme = 'light' | 'dark' | 'system';web/sdk/react/views-new/preferences/preferences-view.tsx (1)
27-27: 💤 Low value
Themetype should be imported fromroutes.tsxrather than re-derived locally
type Theme = keyof typeof THEME_OPTIONSproduces the same union asroutes.tsx's exportedTheme, but the two definitions will silently diverge ifTHEME_OPTIONSgains a new key that isn't added toroutes.tsx(or vice versa). Since bothPreferencesViewand the router context (RouterContext) need compatibleThemetypes, a single source of truth avoids this risk.♻️ Proposed fix
+import type { Theme } from '~/react/components/organization/routes'; -type Theme = keyof typeof THEME_OPTIONS;web/sdk/react/components/organization/profile.tsx (1)
38-68: ⚡ Quick win
memoryHistoryandrouteTreeare recalculated on every render but only consumed on the first
useMemo(..., [])captures both values at mount and never reads the re-computed versions produced on subsequent renders. The calls tocreateMemoryHistoryandgetRootTreeon lines 38–44 are pure waste after the first render. Move them inside theuseMemoto make the intent explicit and eliminate the overhead.
customRoutesmust remain outside (it's also consumed byRouterProvider'scontextprop for live updates) — onlymemoryHistoryandrouteTreeneed to move.♻️ Proposed refactor
- const memoryHistory = createMemoryHistory({ - initialEntries: [defaultRoute] - }); - - const customRoutes = getCustomRoutes(customScreens); - - const routeTree = getRootTree({ customScreens }); - + const customRoutes = getCustomRoutes(customScreens); + const memoryRouter = useMemo( () => createRouter({ - routeTree, - history: memoryHistory, + routeTree: getRootTree({ customScreens }), + history: createMemoryHistory({ initialEntries: [defaultRoute] }), context: { organizationId, ...
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 390621db-7bd8-434e-88ad-e529a0d166f3
📒 Files selected for processing (5)
web/sdk/react/components/organization/preferences/index.tsxweb/sdk/react/components/organization/profile.tsxweb/sdk/react/components/organization/routes.tsxweb/sdk/react/views-new/preferences/preferences-view.tsxweb/sdk/react/views/preferences/preferences-page.tsx
3 participants
Summary
ThemeProvider—useTheme'ssetThemewas a no-op without it.themeandonThemeChangeprops toOrganizationProfileso consumers can control the preferences theme select externally instead of relying solely on the SDK'sThemeProvider.PreferencesPageandPreferencesView; when not provided, behavior falls back touseThemefrom Apsara.RouterProvider'scontextprop so prop updates no longer recreate the router and reset navigation state.Selectinviews-new/preferencesto render from a singleTHEME_OPTIONSmap.