Add theme change button and fix HUD problem#442
Add theme change button and fix HUD problem#442uunaigarr wants to merge 3 commits intosiddharthvaddem:mainfrom
Conversation
- add light theme support across editor panels, dialogs, timeline and recording HUD - replace dark-only surfaces and low-contrast text with theme-aware tokens - fix recorder HUD layout so the record button no longer pushes side controls off-screen - hide open video/open project actions while recording to make room for recording controls
- add light theme support across editor panels, dialogs, timeline and recording HUD - replace remaining dark-only surfaces and low-contrast text with theme-aware tokens - fix recorder HUD layout so the record button no longer pushes side controls off-screen - hide open video/open project actions while recording to make room for recording controls
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adds a ThemeContext with persistent dark/light theme, introduces editor/glass design tokens and utility classes, and migrates ~25 components from hardcoded colors to those tokens; App and VideoEditor are wired to use the theme and the Toaster now respects theme dynamically. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9450834f9a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "peerDependencies": { | ||
| "picomatch": "^3 || ^4" | ||
| "msw": "^2.4.9", | ||
| "vite": "^6.0.0 || ^7.0.0-0" |
There was a problem hiding this comment.
This lockfile edit removes the nested vite package entries under @vitest/browser/@vitest/browser-playwright while keeping @vitest/mocker records that still declare a vite peer, which makes the lock inconsistent with itself. On this commit, npm ci --dry-run --ignore-scripts fails with Missing: vite@ from lock file, so clean installs in CI or fresh clones will fail until the lockfile is regenerated correctly.
Useful? React with 👍 / 👎.
| # Vitest browser mode screenshots | ||
| __screenshots__/ No newline at end of file | ||
| __screenshots__/ | ||
| package-lock.json |
There was a problem hiding this comment.
Adding package-lock.json to .gitignore will cause future lockfile updates to be silently omitted from commits, which undermines reproducible installs for this npm app and makes lock/package drift more likely. Given this same change set already introduces a lock mismatch, ignoring the lockfile will make it harder to prevent or review similar breakages.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/components/video-editor/VideoEditor.tsx (1)
288-309:⚠️ Potential issue | 🟠 MajorKeep
webcamSizePresetin the project snapshot.
saveProject()andapplyLoadedProject()still persist this field, butcurrentProjectSnapshotdropped it here. That means a webcam size tweak can stop affectinghasUnsavedChanges, so the editor may happily say “nothing to save” and lose that change on close.🐛 Proposed fix
return createProjectSnapshot(currentProjectMedia, { wallpaper, shadowIntensity, showBlur, motionBlurAmount, borderRadius, padding, cropRegion, zoomRegions, trimRegions, speedRegions, annotationRegions, aspectRatio, webcamLayoutPreset, webcamMaskShape, + webcamSizePreset, webcamPosition, exportQuality, exportFormat, gifFrameRate, gifLoop, gifSizePreset, }); }, [ currentProjectMedia, wallpaper, shadowIntensity, showBlur, motionBlurAmount, borderRadius, padding, cropRegion, zoomRegions, trimRegions, speedRegions, annotationRegions, aspectRatio, webcamLayoutPreset, webcamMaskShape, + webcamSizePreset, webcamPosition, exportQuality, exportFormat, gifFrameRate, gifLoop, gifSizePreset, ]);Also applies to: 310-332
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 288 - 309, The project snapshot creation omitted webcamSizePreset causing webcam size changes to be ignored for unsaved-change detection; update all places that call createProjectSnapshot (e.g., the call using currentProjectMedia and the other call around lines 310–332) to include the webcamSizePreset property alongside webcamLayoutPreset, webcamMaskShape, and webcamPosition so the snapshot contains webcamSizePreset; ensure the same property name (webcamSizePreset) is passed into createProjectSnapshot wherever project snapshots are constructed.src/components/video-editor/AnnotationSettingsPanel.tsx (1)
376-388:⚠️ Potential issue | 🟠 MajorThese color pickers still force a dark popover.
Each
PopoverContentis pinned tobg-[#1a1a1c], so opening the palette in light mode drops a dark slab into an otherwise themed panel. kinda defeats the theme pass right where users are staring. Swap these to the same semantic surface tokens (bg-popover,editor-panel-soft, etc.) so the picker actually follows the selected theme.Also applies to: 410-427, 570-584
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/AnnotationSettingsPanel.tsx` around lines 376 - 388, PopoverContent instances for the color picker are hardcoded to bg-[`#1a1a1c`], forcing a dark background; find the PopoverContent components (used alongside PopoverTrigger in AnnotationSettingsPanel, e.g., the instances wrapping the color palette around the Button) and replace the literal bg-[`#1a1a1c`] with the appropriate semantic surface tokens such as bg-popover or editor-panel-soft (or the project's equivalent theme token) so the popover follows light/dark themes; apply the same change to the other PopoverContent occurrences referenced in the review (the blocks around lines ~410-427 and ~570-584) to ensure theme consistency across all color pickers.src/components/video-editor/SettingsPanel.tsx (2)
1363-1382:⚠️ Potential issue | 🟠 MajorActive export-format tabs lose contrast in light mode.
The active state is only
bg-[#34B27B]/10plustext-white. On a light surface that’s basically white text on a barely-tinted chip. Either make the active chip solid green or keep the subtle background and switch the text to a dark/brand color.🎨 One simple fix
exportFormat === "mp4" - ? "bg-[`#34B27B`]/10 border-[`#34B27B`]/50 text-white" + ? "bg-[`#34B27B`] border-[`#34B27B`] text-white" : "bg-background border-input editor-text-muted hover:bg-accent hover:text-foreground", @@ exportFormat === "gif" - ? "bg-[`#34B27B`]/10 border-[`#34B27B`]/50 text-white" + ? "bg-[`#34B27B`] border-[`#34B27B`] text-white" : "bg-background border-input editor-text-muted hover:bg-accent hover:text-foreground",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/SettingsPanel.tsx` around lines 1363 - 1382, The active export-format button styling (in the buttons using exportFormat and onExportFormatChange) uses a very light bg-[`#34B27B`]/10 with text-white, causing poor contrast in light mode; update the active class for the buttons (the className logic that checks exportFormat === "mp4" / "gif") to use a higher-contrast combination — either make the active chip a solid brand green background (e.g., stronger bg like bg-[`#34B27B`] or equivalent token) with appropriate foreground (white) OR keep the subtle bg and switch the active text color to a dark/brand color (not white) so the label (t("exportFormat.mp4") / gif) is readable; ensure the change is applied to both export-format buttons (the ones using exportFormat, onExportFormatChange, Film, and getTestId).
811-829:⚠️ Potential issue | 🟠 MajorA few section surfaces are still dark-mode-only.
These containers still use
bg-white/[0.02]and the layout preset trigger still usesbg-black/20, so light theme ends up with washed-out cards and a muddy select. I’d swap these to the same semantic surfaces used elsewhere (editor-panel-soft,bg-background,bg-popover, etc.) before calling the theme pass done.Also applies to: 955-964, 1069-1084
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/SettingsPanel.tsx` around lines 811 - 829, Several containers still use hardcoded dark-mode colors (bg-white/[0.02], bg-black/20) causing washed-out cards in light theme; update the AccordionItem with value="layout" and the SelectTrigger used for webcamLayoutPreset to use the semantic surface classes used elsewhere (e.g., replace bg-white/[0.02] with editor-panel-soft or bg-background, and replace bg-black/20 with bg-popover or editor-panel-soft) so themes render correctly. Apply the same replacements to the other similar sections noted (the other AccordionItem/select blocks referenced) so all cards and triggers use the shared semantic surface classes instead of hardcoded color utilities.
🧹 Nitpick comments (4)
src/components/video-editor/timeline/TimelineEditor.tsx (1)
669-669: Consider tokenizing the grid gradient too.Line 669 still hardcodes color values in arbitrary Tailwind syntax. lowkey risky for future theme tuning; a shared token/class would keep this fully aligned with the rest of the migration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/timeline/TimelineEditor.tsx` at line 669, The gradient background on the grid is using hardcoded color values in the className on the div inside TimelineEditor, so replace that inline Tailwind arbitrary colors with a shared theme token or utility class (e.g., create a CSS rule or Tailwind plugin class like .bg-grid-gradient that references design tokens / CSS variables such as --color-grid and --color-grid-dark) and update the div in TimelineEditor.tsx to use that class instead of the long bg-[linear-gradient(...)] string; ensure the new token/class covers both light and dark variants so theme tuning stays centralized and consistent with other components.src/components/video-editor/BlurSettingsPanel.tsx (1)
67-67: Nit: cleaner to avoid hardcoded white in hover border.
hover:border-white/20is kinda off-pattern now that this panel is tokenized; usinghover:border-borderkeeps light/dark behavior consistent.♻️ Small consistency tweak
- : "bg-background border-input hover:bg-accent hover:border-white/20", + : "bg-background border-input hover:bg-accent hover:border-border",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/BlurSettingsPanel.tsx` at line 67, In BlurSettingsPanel (the component that builds the className string containing "bg-background border-input hover:bg-accent hover:border-white/20") replace the hardcoded "hover:border-white/20" token with the design token "hover:border-border" so the hover border uses the tokenized color and respects light/dark theming; keep the rest of the class string intact.src/index.css (1)
188-192: Tokenize.editor-kbdaccent color instead of hardcoding it.
#34b27bon Line 191 bypasses the new token system. recommend a semantic var (e.g.--editor-accent) for consistent theming/tuning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.css` around lines 188 - 192, Replace the hardcoded color in the .editor-kbd rule with a semantic CSS variable so theming works consistently: change the color property in .editor-kbd to use a token like --editor-accent (fallback optional) and add/update the token declaration where global CSS variables are defined (the same place other vars like --editor-hover / --editor-border-strong live) so consumers can tune the accent color centrally.src/contexts/ThemeContext.tsx (1)
53-61: Avoid double-applying the theme class on each toggle.
applyThemeruns insetTheme(Line 60) and again in the effect (Line 69). Works, but it’s redundant DOM churn.nit: cleaner single-source apply path
const setTheme = useCallback((newTheme: Theme) => { setThemeState(newTheme); try { localStorage.setItem(THEME_STORAGE_KEY, newTheme); } catch { // localStorage may be unavailable } - applyTheme(newTheme); }, []);Also applies to: 67-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/contexts/ThemeContext.tsx` around lines 53 - 61, The theme is being applied twice: once inside setTheme and again in the downstream effect; remove the redundant call by having setTheme only update state and storage (keep setThemeState and localStorage logic) and let the existing effect that watches themeState call applyTheme centrally; update references to the functions setTheme and applyTheme (and the effect that currently calls applyTheme) so there is a single source of DOM updates for theme changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Line 37: Remove the `package-lock.json` entry from `.gitignore` and commit the
lockfile so npm installs are deterministic: edit `.gitignore` to delete the line
containing "package-lock.json", then add and commit `package-lock.json` (git add
package-lock.json && git commit -m "Track package-lock.json"); confirm
`package.json` still declares npm as the package manager and push the commit so
CI/prod and local dev use the same lockfile.
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 244-245: The styled <option> elements in LaunchWindow.tsx (inside
the select rendering where getLocaleName(loc) is used) rely on classes like
bg-popover and text-foreground which are not consistently applied on native
selects (especially macOS/Windows); update the select rendering to either remove
option-level styles and apply safe select-level styles or replace this native
<select> with the existing custom dropdown component used elsewhere so contrast
is reliable across platforms, then test the locale dropdown (the option loop
using getLocaleName(loc)) in macOS and Windows light/dark themes to ensure
legibility and adjust colors or fallback styles accordingly; also apply the same
change to the other occurrences noted (the option blocks at the other two
locations).
In `@src/components/launch/SourceSelector.module.css`:
- Around line 14-15: The .selected card state still uses a hardcoded dark
gradient; update the .selected rule in SourceSelector.module.css to use design
tokens (e.g., reuse --glass-card and related token variables like --glass-border
or introduce a new token such as --glass-card-selected) instead of the
near-black gradient so the selected card responds correctly to light/dark
themes; modify the .selected selector to reference those CSS variables and
ensure any border/shadow values also use tokens for consistent theming.
In `@src/components/video-editor/BlurSettingsPanel.tsx`:
- Around line 94-99: Replace the hardcoded dark color class "text-slate-300" in
BlurSettingsPanel.tsx with the theme-aware text token used elsewhere (e.g.,
text-muted-foreground or the project's equivalent) so the label contrast adapts
to light/dark themes; update both occurrences—the span wrapping
t("annotation.blurIntensity") and the sibling span with "editor-text-muted
font-mono" (the second instance around lines ~130-133)—to use the theme token
instead of "text-slate-300".
In `@src/components/video-editor/ExportDialog.tsx`:
- Around line 122-123: ExportDialog currently derives the filename using
exportedFilePath.split("/").pop(), which fails on Windows paths; update the
extraction to be cross-platform (e.g., implement a small helper like getBasename
that uses a regex or splits on both slashes/backslashes such as
exportedFilePath.split(/[\\/]/).pop(), or, if running in Node context, use
path.basename) and replace the current expression in the JSX span that
references exportedFilePath so the displayed basename works for both POSIX and
Windows paths.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1732-1738: The theme-toggle button (the icon-only button that
calls toggleTheme and checks theme) needs an accessible name via aria-label
sourced from i18n: import or use your existing translation hook/function (e.g.,
t or useTranslation) and set aria-label on the button to a translated string
that mirrors the title logic (e.g., aria-label={t(theme === "dark" ?
"switch_to_light_mode" : "switch_to_dark_mode")}) so screen readers get an
accessible, localized label for the toggle.
In `@src/contexts/ThemeContext.tsx`:
- Around line 37-39: The theme is being applied twice and causes a flash because
applyTheme is called synchronously in setTheme and again in a later useEffect;
replace that effect with useLayoutEffect so the DOM/class is updated before the
first paint and remove the direct call to applyTheme inside setTheme so theme
application happens only once via the layout effect; specifically, change the
effect that currently uses useEffect to useLayoutEffect (the one that reads
theme and toggles the .dark class) and delete the applyTheme call from the
setTheme implementation so setTheme only updates state.
---
Outside diff comments:
In `@src/components/video-editor/AnnotationSettingsPanel.tsx`:
- Around line 376-388: PopoverContent instances for the color picker are
hardcoded to bg-[`#1a1a1c`], forcing a dark background; find the PopoverContent
components (used alongside PopoverTrigger in AnnotationSettingsPanel, e.g., the
instances wrapping the color palette around the Button) and replace the literal
bg-[`#1a1a1c`] with the appropriate semantic surface tokens such as bg-popover or
editor-panel-soft (or the project's equivalent theme token) so the popover
follows light/dark themes; apply the same change to the other PopoverContent
occurrences referenced in the review (the blocks around lines ~410-427 and
~570-584) to ensure theme consistency across all color pickers.
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 1363-1382: The active export-format button styling (in the buttons
using exportFormat and onExportFormatChange) uses a very light bg-[`#34B27B`]/10
with text-white, causing poor contrast in light mode; update the active class
for the buttons (the className logic that checks exportFormat === "mp4" / "gif")
to use a higher-contrast combination — either make the active chip a solid brand
green background (e.g., stronger bg like bg-[`#34B27B`] or equivalent token) with
appropriate foreground (white) OR keep the subtle bg and switch the active text
color to a dark/brand color (not white) so the label (t("exportFormat.mp4") /
gif) is readable; ensure the change is applied to both export-format buttons
(the ones using exportFormat, onExportFormatChange, Film, and getTestId).
- Around line 811-829: Several containers still use hardcoded dark-mode colors
(bg-white/[0.02], bg-black/20) causing washed-out cards in light theme; update
the AccordionItem with value="layout" and the SelectTrigger used for
webcamLayoutPreset to use the semantic surface classes used elsewhere (e.g.,
replace bg-white/[0.02] with editor-panel-soft or bg-background, and replace
bg-black/20 with bg-popover or editor-panel-soft) so themes render correctly.
Apply the same replacements to the other similar sections noted (the other
AccordionItem/select blocks referenced) so all cards and triggers use the shared
semantic surface classes instead of hardcoded color utilities.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 288-309: The project snapshot creation omitted webcamSizePreset
causing webcam size changes to be ignored for unsaved-change detection; update
all places that call createProjectSnapshot (e.g., the call using
currentProjectMedia and the other call around lines 310–332) to include the
webcamSizePreset property alongside webcamLayoutPreset, webcamMaskShape, and
webcamPosition so the snapshot contains webcamSizePreset; ensure the same
property name (webcamSizePreset) is passed into createProjectSnapshot wherever
project snapshots are constructed.
---
Nitpick comments:
In `@src/components/video-editor/BlurSettingsPanel.tsx`:
- Line 67: In BlurSettingsPanel (the component that builds the className string
containing "bg-background border-input hover:bg-accent hover:border-white/20")
replace the hardcoded "hover:border-white/20" token with the design token
"hover:border-border" so the hover border uses the tokenized color and respects
light/dark theming; keep the rest of the class string intact.
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Line 669: The gradient background on the grid is using hardcoded color values
in the className on the div inside TimelineEditor, so replace that inline
Tailwind arbitrary colors with a shared theme token or utility class (e.g.,
create a CSS rule or Tailwind plugin class like .bg-grid-gradient that
references design tokens / CSS variables such as --color-grid and
--color-grid-dark) and update the div in TimelineEditor.tsx to use that class
instead of the long bg-[linear-gradient(...)] string; ensure the new token/class
covers both light and dark variants so theme tuning stays centralized and
consistent with other components.
In `@src/contexts/ThemeContext.tsx`:
- Around line 53-61: The theme is being applied twice: once inside setTheme and
again in the downstream effect; remove the redundant call by having setTheme
only update state and storage (keep setThemeState and localStorage logic) and
let the existing effect that watches themeState call applyTheme centrally;
update references to the functions setTheme and applyTheme (and the effect that
currently calls applyTheme) so there is a single source of DOM updates for theme
changes.
In `@src/index.css`:
- Around line 188-192: Replace the hardcoded color in the .editor-kbd rule with
a semantic CSS variable so theming works consistently: change the color property
in .editor-kbd to use a token like --editor-accent (fallback optional) and
add/update the token declaration where global CSS variables are defined (the
same place other vars like --editor-hover / --editor-border-strong live) so
consumers can tune the accent color centrally.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d586542-1205-422d-a949-2a4c824592e6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (28)
.gitignoresrc/App.tsxsrc/components/launch/LaunchWindow.tsxsrc/components/launch/SourceSelector.module.csssrc/components/launch/SourceSelector.tsxsrc/components/ui/accordion.tsxsrc/components/ui/audio-level-meter.tsxsrc/components/ui/switch.tsxsrc/components/ui/tooltip.tsxsrc/components/video-editor/AddCustomFontDialog.tsxsrc/components/video-editor/AnnotationSettingsPanel.tsxsrc/components/video-editor/BlurSettingsPanel.tsxsrc/components/video-editor/ExportDialog.tsxsrc/components/video-editor/FormatSelector.tsxsrc/components/video-editor/KeyboardShortcutsHelp.tsxsrc/components/video-editor/PlaybackControls.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/ShortcutsConfigDialog.tsxsrc/components/video-editor/TutorialHelp.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/timeline/Item.tsxsrc/components/video-editor/timeline/Row.tsxsrc/components/video-editor/timeline/Subrow.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/timeline/TimelineWrapper.tsxsrc/contexts/ThemeContext.tsxsrc/index.csstsconfig.json
| # Vitest browser mode screenshots | ||
| __screenshots__/ No newline at end of file | ||
| __screenshots__/ | ||
| package-lock.json |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify package-manager strategy before keeping package-lock.json ignored.
echo "1) Check ignore rule:"
rg -n '^package-lock\.json$' .gitignore
echo
echo "2) Discover lockfiles in repo:"
fd -a '^(package-lock\.json|pnpm-lock\.yaml|yarn\.lock)$'
echo
echo "3) Check package manager declaration(s) in package.json files:"
rg -n '"packageManager"\s*:' --iglob 'package.json'Repository: siddharthvaddem/openscreen
Length of output: 256
Remove package-lock.json from .gitignore
The repo explicitly declares npm as the package manager in package.json, so ignoring the lockfile is lowkey risky. Without package-lock.json tracked, installs become non-deterministic and you'll get audit drift between local dev and CI/prod. Delete line 37 and make sure the lockfile gets committed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore at line 37, Remove the `package-lock.json` entry from
`.gitignore` and commit the lockfile so npm installs are deterministic: edit
`.gitignore` to delete the line containing "package-lock.json", then add and
commit `package-lock.json` (git add package-lock.json && git commit -m "Track
package-lock.json"); confirm `package.json` still declares npm as the package
manager and push the commit so CI/prod and local dev use the same lockfile.
| <option key={loc} value={loc} className="bg-popover text-foreground"> | ||
| {getLocaleName(loc)} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l src/components/launch/LaunchWindow.tsxRepository: siddharthvaddem/openscreen
Length of output: 112
🏁 Script executed:
head -350 src/components/launch/LaunchWindow.tsx | tail -120Repository: siddharthvaddem/openscreen
Length of output: 5094
🏁 Script executed:
sed -n '240,290p' src/components/launch/LaunchWindow.tsxRepository: siddharthvaddem/openscreen
Length of output: 2213
🏁 Script executed:
sed -n '340,350p' src/components/launch/LaunchWindow.tsxRepository: siddharthvaddem/openscreen
Length of output: 554
🌐 Web query:
HTML option element CSS styling platform limitations macOS Windows
💡 Result:
HTML elements have severe limitations on CSS styling due to native OS rendering, with significant platform differences between macOS and Windows. Key limitations and platform differences: - macOS (Safari and Chrome/WebKit-based): Almost no CSS styling supported for . Properties like color, background-color, padding often fail completely. This is a known WebKit limitation where uses native macOS controls that ignore most CSS. Sources confirm text color does not work in Chrome/Safari on macOS. - Windows (Chrome/Edge, Firefox): Better support. Basic properties like color and background-color work in Chromium browsers (Chrome, Edge) and Firefox on Windows. Padding and advanced styling still limited. General cross-platform issues: - No support for padding, margins, borders, or complex layout on in any browser. - Font properties (font-size, font-family) may work inconsistently. - Limited to basic text styling where supported. Future solution: CSS 'appearance: base-select' enables full styling of and (Chrome/Edge 135+, Opera 120+ as of 2026), but no Safari/Firefox support yet, so falls back to native behavior. Recommendation: For consistent styling, replace native with custom dropdowns using
- /
- , and JavaScript for functionality and accessibility. Examples: Native on macOS Chrome: option { color: red; } ignored. On Windows Chrome: May apply color. Citations: 1: https://stackoverflow.com/questions/28974069/css-dropdown-option-text-color-not-working-on-mac-os-x/28982746 2: https://github.com/Selects in Safari are styled differently to other browsers alphagov/govuk-frontend#3520 3: https://stackoverflow.com/questions/59523980/how-to-style-multiple-select-option-in-safari-other-than-color 4: https://stackoverflow.com/questions/12301729/styling-for-option-values-in-a-html-select-drop-down-doesnt-work-in-chrome-sa 5: http://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/option 6: https://developer.mozilla.org/en-US/docs/Learn/HTML/Forms/Advanced_styling_for_HTML_forms 7: https://stackoverflow.com/questions/48352211/html-select-element-rendered-differently-on-chrome-for-macos-and-chrome-for-wind 8: https://stackoverflow.com/questions/74304366/how-do-you-style-a-select-option-on-osx-microsoft-edge/74313577#74313577 9: http://developer.mozilla.org/en-US/docs/Learn_web_development/Extensions/Forms/Customizable_select 10: https://developer.mozilla.org/en-US/docs/Learn_web_development/Extensions/Forms/Customizable_select 11: https://caniuse.com/mdn-css_properties_appearance_base-select Verify native <option> legibility across platforms/themes. these styled <option> elements kinda cursed on native selects — macOS basically ignores all CSS (WebKit limitation), and Windows has spotty support. the bg-popover and text-foreground classes won't actually render on macOS, falling back to system defaults. sanity-check the actual contrast and readability with the native styling on both macOS and Windows builds before shipping, especially in light/dark themes. also applies to: lines 281-285, 344-348 🤖 Prompt for AI Agents Verify each finding against the current code and only fix it if needed. In `@src/components/launch/LaunchWindow.tsx` around lines 244 - 245, The styled <option> elements in LaunchWindow.tsx (inside the select rendering where getLocaleName(loc) is used) rely on classes like bg-popover and text-foreground which are not consistently applied on native selects (especially macOS/Windows); update the select rendering to either remove option-level styles and apply safe select-level styles or replace this native <select> with the existing custom dropdown component used elsewhere so contrast is reliable across platforms, then test the locale dropdown (the option loop using getLocaleName(loc)) in macOS and Windows light/dark themes to ensure legibility and adjust colors or fallback styles accordingly; also apply the same change to the other occurrences noted (the option blocks at the other two locations).
| background: var(--glass-card); | ||
| border: 1px solid var(--glass-border); |
There was a problem hiding this comment.
Tokenize the selected card state too.
Nice that the base card now reads from --glass-card, but .selected below still hardcodes a near-black gradient. In light mode the one card the user picks will flip right back to a dark surface, which kinda undoes the theme work for the most important state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/launch/SourceSelector.module.css` around lines 14 - 15, The
.selected card state still uses a hardcoded dark gradient; update the .selected
rule in SourceSelector.module.css to use design tokens (e.g., reuse --glass-card
and related token variables like --glass-border or introduce a new token such as
--glass-card-selected) instead of the near-black gradient so the selected card
responds correctly to light/dark themes; modify the .selected selector to
reference those CSS variables and ensure any border/shadow values also use
tokens for consistent theming.
| <div className="mt-4 p-3 rounded-lg bg-background border border-input"> | ||
| <div className="flex items-center justify-between mb-2"> | ||
| <span className="text-xs font-medium text-slate-300"> | ||
| {t("annotation.blurIntensity")} | ||
| </span> | ||
| <span className="text-[10px] text-slate-400 font-mono"> | ||
| <span className="text-[10px] editor-text-muted font-mono"> |
There was a problem hiding this comment.
Lowkey risky: hardcoded text-slate-300 can hurt light-theme contrast.
Line 96 and Line 131 still hardcode dark-leaning text while their containers are now theme-token based. that can make labels look washed out in light mode.
🎯 Proposed fix
- <span className="text-xs font-medium text-slate-300">
+ <span className="text-xs font-medium editor-text">
{t("annotation.blurIntensity")}
</span>
@@
- <div className="flex items-center gap-2 mb-2 text-slate-300">
+ <div className="flex items-center gap-2 mb-2 editor-text-muted">
<Info className="w-3.5 h-3.5" />
<span className="text-xs font-medium">{t("annotation.shortcutsAndTips")}</span>
</div>Also applies to: 130-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/video-editor/BlurSettingsPanel.tsx` around lines 94 - 99,
Replace the hardcoded dark color class "text-slate-300" in BlurSettingsPanel.tsx
with the theme-aware text token used elsewhere (e.g., text-muted-foreground or
the project's equivalent) so the label contrast adapts to light/dark themes;
update both occurrences—the span wrapping t("annotation.blurIntensity") and the
sibling span with "editor-text-muted font-mono" (the second instance around
lines ~130-133)—to use the theme token instead of "text-slate-300".
| <span className="text-xs editor-text-faint break-all max-w-xs mt-1"> | ||
| {exportedFilePath.split("/").pop()} |
There was a problem hiding this comment.
Filename parsing is POSIX-only right now.
Line 123 uses split("/"), so Windows-style paths won’t resolve to basename correctly.
nit: cleaner cross-platform basename extraction
- {exportedFilePath.split("/").pop()}
+ {exportedFilePath.split(/[/\\]/).pop()}📝 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.
| <span className="text-xs editor-text-faint break-all max-w-xs mt-1"> | |
| {exportedFilePath.split("/").pop()} | |
| <span className="text-xs editor-text-faint break-all max-w-xs mt-1"> | |
| {exportedFilePath.split(/[/\\]/).pop()} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/video-editor/ExportDialog.tsx` around lines 122 - 123,
ExportDialog currently derives the filename using
exportedFilePath.split("/").pop(), which fails on Windows paths; update the
extraction to be cross-platform (e.g., implement a small helper like getBasename
that uses a regex or splits on both slashes/backslashes such as
exportedFilePath.split(/[\\/]/).pop(), or, if running in Node context, use
path.basename) and replace the current expression in the JSX span that
references exportedFilePath so the displayed basename works for both POSIX and
Windows paths.
| <button | ||
| type="button" | ||
| onClick={toggleTheme} | ||
| className="flex items-center justify-center p-1.5 rounded-md editor-text-muted hover:text-foreground hover:bg-accent transition-all duration-150" | ||
| title={theme === "dark" ? "Switch to light mode" : "Switch to dark mode"} | ||
| > | ||
| {theme === "dark" ? <Sun size={14} /> : <Moon size={14} />} |
There was a problem hiding this comment.
Give the theme toggle a real accessible name.
This is an icon-only button, so relying on title is lowkey risky: screen readers won’t consistently use it, and the label is hardcoded English anyway. Please add an aria-label sourced from i18n so the new control is actually discoverable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/video-editor/VideoEditor.tsx` around lines 1732 - 1738, The
theme-toggle button (the icon-only button that calls toggleTheme and checks
theme) needs an accessible name via aria-label sourced from i18n: import or use
your existing translation hook/function (e.g., t or useTranslation) and set
aria-label on the button to a translated string that mirrors the title logic
(e.g., aria-label={t(theme === "dark" ? "switch_to_light_mode" :
"switch_to_dark_mode")}) so screen readers get an accessible, localized label
for the toggle.
| // Default to dark theme (app is designed for dark theme) | ||
| return "dark"; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ThemeContext.tsx" -type fRepository: siddharthvaddem/openscreen
Length of output: 101
🏁 Script executed:
cat -n src/contexts/ThemeContext.tsxRepository: siddharthvaddem/openscreen
Length of output: 2459
🏁 Script executed:
# Check for any CSS files related to theming
fd -e css -e scss | grep -i theme || echo "No theme-related CSS found in standard locations"Repository: siddharthvaddem/openscreen
Length of output: 118
🏁 Script executed:
# Check the entire src structure to understand styling approach
ls -la src/ 2>/dev/null | head -20Repository: siddharthvaddem/openscreen
Length of output: 880
🏁 Script executed:
cat -n src/index.css | head -100Repository: siddharthvaddem/openscreen
Length of output: 4320
🏁 Script executed:
cat -n src/App.cssRepository: siddharthvaddem/openscreen
Length of output: 945
🏁 Script executed:
# Check main.tsx to see how the app is bootstrapped
cat -n src/main.tsxRepository: siddharthvaddem/openscreen
Length of output: 488
First-paint flash is kinda cursed here, and theme gets applied twice.
On mount, theme state syncs to "dark" (line 51), but the .dark class only gets added in useEffect (line 69), which runs after the browser paints. Since :root defaults to light tokens, you get a light→dark flash on cold start.
Also, setTheme calls applyTheme directly (line 60), then the state update triggers the effect which calls it again (line 69). Nit: use useLayoutEffect instead so the class applies before paint, and remove the explicit applyTheme call from setTheme to avoid double-application.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/contexts/ThemeContext.tsx` around lines 37 - 39, The theme is being
applied twice and causes a flash because applyTheme is called synchronously in
setTheme and again in a later useEffect; replace that effect with
useLayoutEffect so the DOM/class is updated before the first paint and remove
the direct call to applyTheme inside setTheme so theme application happens only
once via the layout effect; specifically, change the effect that currently uses
useEffect to useLayoutEffect (the one that reads theme and toggles the .dark
class) and delete the applyTheme call from the setTheme implementation so
setTheme only updates state.
2 participants
Pull Request
Description
This PR implements full light/dark theme support across the app UI.
The app was previously designed as a dark-only experience. In this change, theme support is extended so the main editor, timeline, dialogs, pre-editor HUD, source picker, and shared UI elements all respond consistently to the selected theme. The update also improves contrast and text legibility in both modes by replacing remaining dark-only surfaces and hardcoded colors with theme-aware styling.
In addition, the recording HUD was adjusted so its controls behave better while recording:
the record button no longer causes the bar to expand unexpectedly
recording-only controls have room to appear without pushing other actions off-screen
non-essential actions are hidden while recording to keep the layout stable
Motivation
It felt like a worthwhile improvement to implement light/dark theme support and make the overall UI look cleaner and more consistent.
Type of Change
Related Issue(s)
When recording, the hud would expand and looked wrong.
Screenshots / Video
Screenshot (if applicable):
Bug fix:
Before:
After:
New feature:
Testing
Git pull code
Checklist
Summary by CodeRabbit
New Features
Style
Chores