fix(theme): stabilize multi-window theme sync and eliminate flicker#42
fix(theme): stabilize multi-window theme sync and eliminate flicker#42
Conversation
📝 WalkthroughWalkthroughThis pull request implements a custom theme provider system to replace the next-themes library. It introduces a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/tauri/src/App.tsx (1)
229-235: Consider centralizing repeated ThemeProvider config.This same prop set is duplicated across app roots; extracting a shared wrapper/config would reduce drift risk in future theme changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/App.tsx` around lines 229 - 235, Several app roots repeat the same ThemeProvider props (attribute="class", defaultTheme="system", enableSystem, storageKey="arandu-theme", disableTransitionOnChange"); extract these into a single shared wrapper component (e.g., AppThemeProvider or ThemeProviderWrapper) that composes ThemeProvider with that prop set and export it for reuse, then replace direct ThemeProvider usages with this wrapper across the codebase (search for ThemeProvider and storageKey="arandu-theme" to locate all instances) so future theme prop changes are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src/index.css`:
- Around line 133-135: The font-family declaration in the CSS block with `@apply`
bg-background text-foreground font-sans; is violating stylelint rules
(font-family-name-quotes and declaration spacing); fix it by using a lint-safe
font-family value (either remove unnecessary quotes: font-family: Inter,
system-ui, sans-serif; or use double quotes: font-family: "Inter", system-ui,
sans-serif;) and ensure each declaration in the same rule-set ends with a
semicolon and consistent spacing (keep `@apply` ..., then the font-family line,
then background-color: hsl(var(--background));) so the block containing `@apply`,
font-family, and background-color conforms to stylelint rules.
In `@apps/tauri/src/lib/theme.tsx`:
- Around line 81-82: The ThemeProvider's default storageKey ("theme") is
inconsistent with the bootstrap scripts; update the default value in the
ThemeProviderProps deconstruction inside the ThemeProvider function so
storageKey defaults to "arandu-theme" to match the window bootstrap (e.g.,
change the default from "theme" to "arandu-theme" where storageKey is defined in
the ThemeProvider component in apps/tauri/src/lib/theme.tsx).
- Around line 114-118: The current try/catch around
window.__TAURI__.event.emit(...) and the Promise chain from
window.__TAURI__.event.listen(...) don't handle async rejections; replace the
synchronous try/catch with either an awaited call inside an async function
wrapped in try/catch or append a .catch(...) to the emit() call to handle
promise rejections, and add .catch(...) to the listen() promise chain as well
(e.g., in the code paths using window.__TAURI__.event.emit and
window.__TAURI__.event.listen), ensuring any errors are either logged via the
existing logger or safely ignored so local state stays in sync.
---
Nitpick comments:
In `@apps/tauri/src/App.tsx`:
- Around line 229-235: Several app roots repeat the same ThemeProvider props
(attribute="class", defaultTheme="system", enableSystem,
storageKey="arandu-theme", disableTransitionOnChange"); extract these into a
single shared wrapper component (e.g., AppThemeProvider or ThemeProviderWrapper)
that composes ThemeProvider with that prop set and export it for reuse, then
replace direct ThemeProvider usages with this wrapper across the codebase
(search for ThemeProvider and storageKey="arandu-theme" to locate all instances)
so future theme prop changes are made in one place.
ℹ️ Review info
Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27bed7b1-91b4-407d-a882-0cdaf3ceb927
📒 Files selected for processing (16)
apps/tauri/index.htmlapps/tauri/settings.htmlapps/tauri/src/App.tsxapps/tauri/src/SettingsApp.tsxapps/tauri/src/WhisperApp.tsxapps/tauri/src/__tests__/components/ChatPanel.test.tsxapps/tauri/src/__tests__/components/HomeScreen.test.tsxapps/tauri/src/__tests__/components/TopBar.test.tsxapps/tauri/src/__tests__/lib/theme.test.tsxapps/tauri/src/__tests__/setup.tsapps/tauri/src/components/TopBar.tsxapps/tauri/src/components/settings/GeneralSettings.tsxapps/tauri/src/components/ui/sonner.tsxapps/tauri/src/index.cssapps/tauri/src/lib/theme.tsxapps/tauri/whisper.html
| @apply bg-background text-foreground font-sans; | ||
| font-family: 'Inter', system-ui, sans-serif; | ||
| background-color: hsl(var(--background)); |
There was a problem hiding this comment.
Resolve the stylelint violation in the body font declaration block.
Line 134 currently conflicts with font-family-name-quotes, and this block is also where declaration spacing is being linted.
💡 Suggested lint-safe adjustment
body {
`@apply` bg-background text-foreground font-sans;
- font-family: 'Inter', system-ui, sans-serif;
+
+ font-family: Inter, system-ui, sans-serif;
background-color: hsl(var(--background));📝 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.
| @apply bg-background text-foreground font-sans; | |
| font-family: 'Inter', system-ui, sans-serif; | |
| background-color: hsl(var(--background)); | |
| `@apply` bg-background text-foreground font-sans; | |
| font-family: Inter, system-ui, sans-serif; | |
| background-color: hsl(var(--background)); |
🧰 Tools
🪛 Biome (2.4.4)
[error] 133-133: Tailwind-specific syntax is disabled.
(parse)
🪛 Stylelint (17.3.0)
[error] 134-134: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 134-134: Unexpected quotes around "Inter" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/tauri/src/index.css` around lines 133 - 135, The font-family declaration
in the CSS block with `@apply` bg-background text-foreground font-sans; is
violating stylelint rules (font-family-name-quotes and declaration spacing); fix
it by using a lint-safe font-family value (either remove unnecessary quotes:
font-family: Inter, system-ui, sans-serif; or use double quotes: font-family:
"Inter", system-ui, sans-serif;) and ensure each declaration in the same
rule-set ends with a semicolon and consistent spacing (keep `@apply` ..., then the
font-family line, then background-color: hsl(var(--background));) so the block
containing `@apply`, font-family, and background-color conforms to stylelint
rules.
| storageKey = "theme", | ||
| }: ThemeProviderProps) { |
There was a problem hiding this comment.
Default storage key is inconsistent with bootstrap scripts.
Line 81 defaults to "theme", but the window bootstrap scripts read "arandu-theme" (for example, apps/tauri/index.html Line 11 and apps/tauri/whisper.html Line 10). This can reintroduce startup mismatch/flicker whenever ThemeProvider is used without an explicit storageKey.
🔧 Suggested fix
- storageKey = "theme",
+ storageKey = "arandu-theme",📝 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.
| storageKey = "theme", | |
| }: ThemeProviderProps) { | |
| storageKey = "arandu-theme", | |
| }: ThemeProviderProps) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/tauri/src/lib/theme.tsx` around lines 81 - 82, The ThemeProvider's
default storageKey ("theme") is inconsistent with the bootstrap scripts; update
the default value in the ThemeProviderProps deconstruction inside the
ThemeProvider function so storageKey defaults to "arandu-theme" to match the
window bootstrap (e.g., change the default from "theme" to "arandu-theme" where
storageKey is defined in the ThemeProvider component in
apps/tauri/src/lib/theme.tsx).
| try { | ||
| void window.__TAURI__.event.emit("theme-changed", nextTheme); | ||
| } catch { | ||
| // Ignore event bridge failures and keep local state in sync. | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/tauri/src/lib/theme.tsx | head -200Repository: devitools/arandu
Length of output: 7035
🏁 Script executed:
# Check the specific lines mentioned in the review
sed -n '110,125p' apps/tauri/src/lib/theme.tsx
sed -n '175,185p' apps/tauri/src/lib/theme.tsxRepository: devitools/arandu
Length of output: 736
🌐 Web query:
Tauri event.emit() Promise rejection handling - does emit return a promise
💡 Result:
Yes. In Tauri’s JavaScript API, emit is async and returns a Promise<void>. So you should await it or attach a .catch(...) to avoid unhandled promise rejections. [1]
Internally it just awaits an IPC invoke('plugin:event|emit', ...), so the promise only rejects if that IPC call fails (e.g., not running in a Tauri context, plugin/IPC error, payload serialization issues), not because a frontend listener “throws”. [2]
import { emit } from "@tauri-apps/api/event";
try {
await emit("my-event", { hello: "world" });
} catch (e) {
// handle failed emit (IPC/invoke failure)
console.error(e);
}
// or, fire-and-forget safely:
void emit("my-event").catch(console.error);Sources: [1] Tauri JS API docs for event.emit(): Promise<void> (tauri.app), [2] Tauri emit implementation (await invoke('plugin:event|emit', ...)) (github.com)
Citations:
- 1: https://tauri.app/reference/javascript/api/namespaceevent/?utm_source=openai
- 2: https://github.com/tauri-apps/tauri/blob/dev/packages/api/src/event.ts
Add .catch() handlers for Promise rejections in Tauri event calls.
window.__TAURI__.event.emit() and listen() return Promises, but synchronous try/catch blocks cannot catch async rejections. Line 115's try/catch won't handle failures from emit(), and line 180's Promise chain lacks rejection handling.
🔧 Suggested fix
- try {
- void window.__TAURI__.event.emit("theme-changed", nextTheme);
- } catch {
- // Ignore event bridge failures and keep local state in sync.
- }
+ void window.__TAURI__.event.emit("theme-changed", nextTheme).catch(() => {
+ // Ignore event bridge failures and keep local state in sync.
+ });- return () => {
- void unlisten.then((dispose) => dispose());
- };
+ return () => {
+ void unlisten
+ .then((dispose) => dispose())
+ .catch(() => {
+ // Ignore unlisten failures during teardown.
+ });
+ };Also applies to: 179-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/tauri/src/lib/theme.tsx` around lines 114 - 118, The current try/catch
around window.__TAURI__.event.emit(...) and the Promise chain from
window.__TAURI__.event.listen(...) don't handle async rejections; replace the
synchronous try/catch with either an awaited call inside an async function
wrapped in try/catch or append a .catch(...) to the emit() call to handle
promise rejections, and add .catch(...) to the listen() promise chain as well
(e.g., in the code paths using window.__TAURI__.event.emit and
window.__TAURI__.event.listen), ensuring any errors are either logged via the
existing logger or safely ignored so local state stays in sync.
2 participants
Summary
This PR fixes theme flicker/blinking when switching between
light,dark, andsystemin the Tauri app.The previous setup relied on
next-themesbehavior across multiple live webviews sharing the same storage key, which caused unstable theme application and visible flashes. This PR replaces that runtime path with a local theme provider designed for Arandu's multi-window setup.What changed
apps/tauri/src/lib/theme.tsxnext-themesusage in:App.tsxSettingsApp.tsxWhisperApp.tsxTopBar.tsxGeneralSettings.tsxcomponents/ui/sonner.tsxapps/tauri/index.htmlapps/tauri/settings.htmlapps/tauri/whisper.htmlapps/tauri/src/index.cssWhy
Arandu runs multiple windows (
main,settings,whisper), and theme changes need to be consistent across them without:This implementation makes theme changes deterministic and window-safe.
Notes
Summary by CodeRabbit
New Features
Chores