feat(editor): light + medium chrome theme modes - #228
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Dark, Medium, and Light themes for the v3 editor chrome, with tokenized CSS overrides, persisted selection, runtime theme APIs, v3 initialization support, a View-menu cycling action, and regression tests. ChangesChrome theme modes
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ViewMenu
participant EditorThemeAPI
participant LocalStorage
participant PluginEditor
ViewMenu->>EditorThemeAPI: call editorCycleTheme()
EditorThemeAPI->>LocalStorage: read and write editorTheme
EditorThemeAPI->>PluginEditor: apply data-editor-theme
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/v3-theme.css`:
- Around line 300-302: Update the Light theme styling for `#plugin-editor`
.editor-transport-rec so its text color uses a darker red with sufficient
contrast against the light panel and button backgrounds, while preserving the
existing semantic color and dark-theme behavior.
- Around line 27-41: Update the default variables in
`#plugin-editor`[data-v3-layout="1"] so the v3 menu/transport chrome preserves the
legacy colors `#263349`, `#131c2e`, `#24324a`, `#334a6b`, and `#475569` instead of
inheriting the new shared palette. Keep these exact defaults either by restoring
the corresponding values or by introducing dedicated tokens for those controls.
In `@src/main.js`:
- Around line 1780-1792: Update editorApplyTheme to accept an already-resolved
theme argument, and have editorSetTheme pass its computed theme directly when
applying the change. Preserve the existing dark-theme attribute removal and
non-dark attribute assignment, while retaining _editorThemePref() only for
callers that need to derive the current preference.
In `@src/menu-bar.js`:
- Around line 147-148: Gate the “Theme: Dark → Medium → Light” menu item using
the existing v3-layout condition so it is included only when the v3 layout is
active. Update the menu construction near the theme-cycle entry while preserving
the separator and existing editorCycleTheme handler for v3 mode.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f08e1b5-9b79-43d4-afb0-c9438cac7812
📒 Files selected for processing (4)
CHANGELOG.mdassets/v3-theme.csssrc/main.jssrc/menu-bar.js
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/theme_chrome.test.mjs`:
- Around line 85-92: Update the light-theme assertions in the test covering
`#plugin-editor`[data-v3-layout="1"][data-editor-theme="light"] to verify exact
intended values: use `#b45309` for --ed-warn and --ed-warn-soft, `#be123c` for
--ed-danger, and `#4f46e5` for --ed-info. Replace the inequality-only checks while
preserving the existing presence assertions.
- Around line 95-102: Update the test around the record button state assertion
to cover the pressed state explicitly: inspect the actual :active selector in
addition to :hover and assert it pins color: `#fb7185`, or rename the test
description to accurately state hover-only coverage if pressed behavior is not
intended. Keep the existing hover assertion intact.
- Around line 58-65: Update the timeline island theme test around the dark and
island variable maps to define the required --ed-* token names explicitly, then
assert both rules contain exactly that set rather than relying on dark’s current
keys and a minimum count. Preserve the existing value-equality checks for every
required token.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c2fef787-45c4-4b20-8e45-8e63888ac7bf
📒 Files selected for processing (3)
assets/v3-theme.csssrc/main.jstests/theme_chrome.test.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- assets/v3-theme.css
- src/main.js
…m, harden theme tests CodeRabbit round 2 on #228: - editorApplyTheme(theme) applies the already-known value instead of re-reading localStorage, so a silently-failed setItem can't leave the DOM attr on a stale theme while setStatus reports the new one. - The View > Theme item is v3Only (hidden via the existing hide-gate pattern): data-editor-theme is only consumed under [data-v3-layout="1"], so in the classic shell it changed the status text and nothing else. - theme_chrome tests: assert the explicit REQUIRED_TOKENS set on both the dark default and island rules (not >=13 mirroring dark), pin the exact light inks (#b45309/#be123c/#4f46e5), and cover the [aria-pressed] pressed selector the test name promised. - menu_model test for the v3 hide-gate. The rec button's RESTING color was already themed on this branch (var(--ed-danger) -> #be123c on light), so no CSS change was needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The dark-blue workspace is hard to read in bright rooms. Theme the editor chrome (menu bar, toolbars, transport bar, panels, buttons, popovers) through CSS variables with three presets: the original Dark, a dim mid-tone Medium, and a bright Light. View ▸ "Theme: Dark → Medium → Light" cycles them; the choice persists in localStorage. Scope is deliberate — the timeline canvas stays dark in every theme (waveform/note contrast is tuned for it), the LCD transport display stays dark (reads as a hardware readout), and category buttons keep their semantic colors. Only the surrounding chrome recolors. - assets/v3-theme.css: hoist the chrome color mappings (core Tailwind utilities, .editor-menu-*, .editor-transport-*, lint-pop) onto --ed-* variables; add [data-editor-theme="medium"|"light"] blocks. - src/main.js: editorApplyTheme / window.editorSetTheme / editorCycleTheme, persisted pref, applied in _applyV3Layout. - src/menu-bar.js: View-menu "Theme: Dark → Medium → Light" item. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
f48c197 to
ff7fa1a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/v3-theme.css`:
- Around line 53-54: Update the --ed-text-faint color values in the Medium and
Light theme variable blocks, including the corresponding definitions at the
referenced theme sections, so they achieve at least 4.5:1 contrast against each
theme’s panel background. Keep the existing variable names and theme structure
unchanged.
- Around line 55-56: Define a theme-specific on-accent foreground token
alongside the accent variables in each affected theme, ensuring the Light accent
and foreground pair meets contrast requirements. Replace hardcoded near-white
active-state text colors with this token in the focused menu items, open menu
titles, lint rows, and pressed transport controls referenced by the comment.
- Around line 437-464: Replace the hardcoded dark-theme colors in the
`#plugin-editor` .editor-guide-body h3, strong, and code rules with the
appropriate --ed-* theme tokens so headings, emphasized text, and inline code
remain readable in light mode; preserve the existing typography, spacing,
borders, and layout styling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e5470fb7-0cbb-4a36-8e3a-022bba889f62
📒 Files selected for processing (4)
CHANGELOG.mdassets/v3-theme.csssrc/main.jssrc/menu-bar.js
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- src/main.js
- src/menu-bar.js
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/v3-theme.css`:
- Around line 53-54: Update the --ed-text-faint color values in the Medium and
Light theme variable blocks, including the corresponding definitions at the
referenced theme sections, so they achieve at least 4.5:1 contrast against each
theme’s panel background. Keep the existing variable names and theme structure
unchanged.
- Around line 55-56: Define a theme-specific on-accent foreground token
alongside the accent variables in each affected theme, ensuring the Light accent
and foreground pair meets contrast requirements. Replace hardcoded near-white
active-state text colors with this token in the focused menu items, open menu
titles, lint rows, and pressed transport controls referenced by the comment.
- Around line 437-464: Replace the hardcoded dark-theme colors in the
`#plugin-editor` .editor-guide-body h3, strong, and code rules with the
appropriate --ed-* theme tokens so headings, emphasized text, and inline code
remain readable in light mode; preserve the existing typography, spacing,
borders, and layout styling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e5470fb7-0cbb-4a36-8e3a-022bba889f62
📒 Files selected for processing (4)
CHANGELOG.mdassets/v3-theme.csssrc/main.jssrc/menu-bar.js
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- src/main.js
- src/menu-bar.js
🛑 Comments failed to post (3)
assets/v3-theme.css (3)
53-54: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Increase faint-text contrast for Medium and Light.
--ed-text-faintfalls below normal-text contrast in both new themes, including on small menu headers and shortcut labels. Use values that reach at least 4.5:1 against their panel backgrounds.Also applies to: 69-70, 160-163, 260-275
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/v3-theme.css` around lines 53 - 54, Update the --ed-text-faint color values in the Medium and Light theme variable blocks, including the corresponding definitions at the referenced theme sections, so they achieve at least 4.5:1 contrast against each theme’s panel background. Keep the existing variable names and theme structure unchanged.
55-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Provide an on-accent foreground token.
Medium’s
#38bdf8accent with the hardcoded near-white active text is only about 2.0:1 contrast. It affects focused menu items, open menu titles, lint rows, and pressed transport controls. Add a theme-specific on-accent token and use it in these active states; Light also needs a contrast-compliant accent/foreground pair.Also applies to: 71-72, 105-108, 169-169, 240-240, 256-259, 297-298
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/v3-theme.css` around lines 55 - 56, Define a theme-specific on-accent foreground token alongside the accent variables in each affected theme, ensuring the Light accent and foreground pair meets contrast requirements. Replace hardcoded near-white active-state text colors with this token in the focused menu items, open menu titles, lint rows, and pressed transport controls referenced by the comment.
437-464: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n -C 4 --glob '*.{js,html,css}' 'editor-guide-body|editor-guide' .Repository: got-feedBack/feedBack-plugin-editor
Length of output: 7366
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '780,810p' screen.html printf '\n---\n' sed -n '1,220p' assets/v3-theme.cssRepository: got-feedBack/feedBack-plugin-editor
Length of output: 14950
Use theme tokens for the user guide prose The guide modal is tokenized in light mode, but these hardcoded heading/strong/code colors stay dark-theme specific and can get too faint on the light panel; switch them to
--ed-*tokens or keep the guide surface dark.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/v3-theme.css` around lines 437 - 464, Replace the hardcoded dark-theme colors in the `#plugin-editor` .editor-guide-body h3, strong, and code rules with the appropriate --ed-* theme tokens so headings, emphasized text, and inline code remain readable in light mode; preserve the existing typography, spacing, borders, and layout styling.
The dark-blue workspace is hard to read in bright rooms. Theme the editor chrome (menu bar, toolbars, transport bar, panels, buttons, popovers) through CSS variables with three presets: the original Dark, a dim mid-tone Medium, and a bright Light. View ▸ "Theme: Dark → Medium → Light" cycles them; the choice persists in localStorage. Scope is deliberate — the timeline canvas stays dark in every theme (waveform/note contrast is tuned for it), the LCD transport display stays dark (reads as a hardware readout), and category buttons keep their semantic colors. Only the surrounding chrome recolors. - assets/v3-theme.css: hoist the chrome color mappings (core Tailwind utilities, .editor-menu-*, .editor-transport-*, lint-pop) onto --ed-* variables; add [data-editor-theme="medium"|"light"] blocks. - src/main.js: editorApplyTheme / window.editorSetTheme / editorCycleTheme, persisted pref, applied in _applyV3Layout. - src/menu-bar.js: View-menu "Theme: Dark → Medium → Light" item. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
ff7fa1a to
bac8644
Compare
# Conflicts: # CHANGELOG.md
…at/editor-theme-modes # Conflicts: # assets/v3-theme.css # src/main.js # src/menu-bar.js
What & why
The dark-blue workspace is hard to read in bright rooms. This adds three chrome themes to the editor — the original Dark, a dim mid-tone Medium, and a bright Light — cycled from View ▸ Theme: Dark → Medium → Light and persisted in
localStorage.Scope (deliberate)
Only the surrounding chrome recolors — menu bar, toolbars, transport bar, panels, buttons, popovers:
How
assets/v3-theme.css— hoist the chrome color mappings (core Tailwind utility classes,.editor-menu-*,.editor-transport-*, lint-pop) onto a--ed-*variable palette; add[data-editor-theme="medium"]and[data-editor-theme="light"]override blocks. Dark is the default (no attribute).src/main.js—editorApplyTheme()/window.editorSetTheme(name)/window.editorCycleTheme(), persisted pref, applied inside_applyV3Layoutso it survives screen re-injection.src/menu-bar.js— View-menu Theme: Dark → Medium → Light item wired toeditorCycleTheme.Verification
npm test— 114 pass.npm run lint— 0 errors (3 pre-existing ratchet warnings, none in this change).🤖 Generated with Claude Code
Summary by CodeRabbit