transparent terminal themes #1561
Conversation
WalkthroughThe pull request introduces terminal transparency configuration for Wave, enabling users to customize the terminal's background transparency. This feature is implemented across multiple files, adding two new configuration keys: Changes
Sequence DiagramsequenceDiagram
participant User
participant SettingsMenu
participant TermViewModel
participant TerminalView
participant ComputeTheme
User->>SettingsMenu: Select Transparency
SettingsMenu->>TermViewModel: Update termTransparencyAtom
TermViewModel->>TerminalView: Trigger theme update
TerminalView->>ComputeTheme: Call with transparency
ComputeTheme-->>TerminalView: Return themed terminal
TerminalView->>User: Display updated terminal
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
frontend/app/view/term/termutil.ts (1)
7-10: LGTM: Consider memoizing color transformationsThe transparency calculation is correct and uses the type-safe colord library. For performance optimization, consider memoizing frequently used color transformations if they become a bottleneck.
docs/docs/config.mdx (1)
47-48: Document the valid range for term:transparency.While the documentation clearly states the default value (0.5), it would be helpful to specify the valid range for the transparency value (e.g., 0.0 to 1.0).
| term:theme | string | preset name of terminal theme to apply by default (default is "default-dark") | -| term:transparency | float64 | set the background transparency of terminal theme (default 0.5) | +| term:transparency | float64 | set the background transparency of terminal theme (0.0 = fully opaque, 1.0 = fully transparent, default 0.5) |frontend/app/view/term/term.tsx (1)
487-521: LGTM! Consider type safety improvement.The transparency menu implementation is well-structured and consistent with other settings. Consider using an enum or const object for transparency values to improve type safety and maintainability.
+const TRANSPARENCY_VALUES = { + DEFAULT: null, + TRANSPARENT: 0.5, + NONE: 0 +} as const; transparencySubMenu.push({ label: "Transparent Background", type: "checkbox", - checked: transparencyMeta == 0.5, + checked: transparencyMeta === TRANSPARENCY_VALUES.TRANSPARENT, click: () => { RpcApi.SetMetaCommand(TabRpcClient, { oref: WOS.makeORef("block", this.blockId), - meta: { "term:transparency": 0.5 }, + meta: { "term:transparency": TRANSPARENCY_VALUES.TRANSPARENT }, }); }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/docs/config.mdx(1 hunks)frontend/app/view/term/term.tsx(7 hunks)frontend/app/view/term/termtheme.ts(1 hunks)frontend/app/view/term/termutil.ts(1 hunks)frontend/types/gotypes.d.ts(2 hunks)pkg/waveobj/metaconsts.go(1 hunks)pkg/waveobj/wtypemeta.go(1 hunks)pkg/wconfig/defaultconfig/termthemes.json(5 hunks)pkg/wconfig/metaconsts.go(1 hunks)pkg/wconfig/settingsconfig.go(1 hunks)
🔇 Additional comments (13)
pkg/wconfig/metaconsts.go (1)
35-35: LGTM: Config key follows conventions
The new configuration key for terminal transparency follows the established naming pattern and is correctly placed within the terminal-related configuration section.
frontend/app/view/term/termtheme.ts (1)
20-21: LGTM: Verify atom subscription behavior
The transparency value is correctly retrieved from the model atom and properly passed to computeTheme.
Let's verify the atom usage pattern:
✅ Verification successful
LGTM: Atom usage pattern is consistent
The transparency atom is used consistently throughout the codebase:
- Correctly initialized with a default value of 0.5 via
useBlockAtom - Properly consumed using
useAtomValuein the theme computation - Appropriately integrated with the config override system
- Used consistently in both theme computation and terminal background handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent atom usage patterns
# Look for other termTransparencyAtom usages
rg 'termTransparencyAtom' -A 2 -B 2
# Check for similar transparency-related atoms
rg -g '*.{ts,tsx}' 'Atom.*[Tt]ransparency'
Length of output: 2835
pkg/waveobj/metaconsts.go (1)
96-96: LGTM! The constant follows the established naming pattern.
The new constant MetaKey_TermTransparency is correctly placed in the terminal-related section and follows the existing naming convention.
pkg/wconfig/defaultconfig/termthemes.json (4)
25-25: Good change: Removed hardcoded transparency from Default Dark theme.
The background color change from semi-transparent (#77) to solid (#000000) aligns well with the new configurable transparency feature.
28-53: LGTM! One Dark Pro theme is well-defined.
The theme includes all required colors and follows the established structure.
156-180: LGTM! Rose Pine theme is well-defined.
The theme includes all required colors and follows the established structure.
56-56: LGTM! Theme display orders are properly sequenced.
The display orders are logically arranged from 1 to 7 without gaps:
- Default Dark
- One Dark Pro
- Dracula
- Monokai
- Campbell
- Warm Yellow
- Rose Pine
Also applies to: 82-82, 108-108, 134-134, 158-158
pkg/waveobj/wtypemeta.go (1)
97-97: LGTM! The transparency field is well-defined.
The TermTransparency field is:
- Using
*float64which appropriately handles optional values - Following the established pattern with
omitemptytag - Documented with the default value
pkg/wconfig/settingsconfig.go (1)
62-62: LGTM! Well-structured field addition.
The TermTransparency field is properly defined as a pointer type with appropriate JSON tags, following the established patterns in the codebase.
frontend/types/gotypes.d.ts (1)
494-494: LGTM! Consistent type definitions.
The TypeScript type definitions are properly aligned with the Go struct, using appropriate types and modifiers.
Also applies to: 644-644
frontend/app/view/term/term.tsx (3)
66-66: LGTM! Well-implemented transparency atom.
The implementation correctly:
- Uses
useBlockAtomfor memoization - Bounds the transparency value between 0 and 1
- Provides a sensible default of 0.5
Also applies to: 208-213
786-787: LGTM! Proper integration with terminal configuration.
The transparency value is correctly retrieved and passed to the theme computation, maintaining consistency with the terminal's configuration system.
217-218: Verify computeTheme implementation.
The function now receives transparency as a third parameter. Let's verify the implementation in the imported module.
✅ Verification successful
computeTheme implementation is correctly handling transparency parameter
The function implementation properly:
- Accepts termTransparency as the third parameter
- Applies transparency to both background and selectionBackground colors using applyTransparencyToColor
- Returns a tuple of [theme, bgcolor] as expected by the destructuring pattern
- Sets theme background to transparent and returns the original (possibly transparency-modified) bgcolor separately
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify computeTheme function signature and implementation
# Test: Search for computeTheme function definition
ast-grep --pattern 'export function computeTheme($_, $_, $_) {
$$$
}'
Length of output: 73
Script:
#!/bin/bash
# Let's try a broader search for computeTheme function using ripgrep
rg "computeTheme" -A 5
Length of output: 2656
Script:
#!/bin/bash
# Let's examine the full implementation of computeTheme function
cat frontend/app/view/term/termutil.ts
Length of output: 1329
| // returns (theme, bgcolor, transparency (0 - 1.0)) | ||
| function computeTheme( | ||
| fullConfig: FullConfigType, | ||
| themeName: string, | ||
| termTransparency: number | ||
| ): [TermThemeType, string] { | ||
| let theme: TermThemeType = fullConfig?.termthemes?.[themeName]; | ||
| if (theme == null) { | ||
| theme = fullConfig?.termthemes?.[DefaultTermTheme] || ({} as any); | ||
| } | ||
| const themeCopy = { ...theme }; | ||
| if (termTransparency != null && termTransparency > 0) { | ||
| if (themeCopy.background) { | ||
| themeCopy.background = applyTransparencyToColor(themeCopy.background, termTransparency); | ||
| } | ||
| if (themeCopy.selectionBackground) { | ||
| themeCopy.selectionBackground = applyTransparencyToColor(themeCopy.selectionBackground, termTransparency); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Based on the search results from all scripts, I can now provide a final response:
Transparency implementation needs bounds validation
From the code analysis:
- Default transparency is set to 0.5 in term.tsx
- UI provides options for transparency: null (default), 0.5 (transparent), and 0 (no transparency)
- The
computeThemefunction only checks fortermTransparency != null && termTransparency > 0 - No upper bound validation or value clamping is implemented
The current implementation could allow transparency values greater than 1.0, which might lead to unexpected behavior. Consider adding validation or clamping to ensure transparency stays within [0, 1.0].
🔗 Analysis chain
LGTM: Verify transparency behavior with extreme values
The transparency application logic is sound, with proper null checks and conditional application. The code correctly handles both background and selection background colors.
Let's verify the theme computation with various transparency values:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for transparency value assignments to ensure they're within bounds
# Look for direct assignments of transparency values
rg -g '*.{ts,tsx,js,jsx}' 'termTransparency.*=.*[0-9]'
# Look for transparency configuration in JSON/YAML files
rg -g '*.{json,yaml,yml}' '"term:transparency":\s*[0-9]'
Length of output: 210
Script:
#!/bin/bash
# Let's search for all usages and assignments of termTransparency to understand its bounds
rg -g '*.{ts,tsx,js,jsx}' 'termTransparency' -B 2 -A 2
# Also search for any transparency-related configuration or validation
rg -g '*.{ts,tsx,js,jsx}' 'transparency.*=|transparency.*>|transparency.*<' -B 2 -A 2
# Look for any transparency-related types or interfaces
ast-grep --pattern 'interface $_ {
$$$
transparency
$$$
}'
Length of output: 6610
Script:
#!/bin/bash
# Let's check the implementation of applyTransparencyToColor to verify how it handles transparency values
ast-grep --pattern 'function applyTransparencyToColor($_) {
$$$
}'
# Also search for any transparency value validations or clamps
rg -g '*.{ts,tsx}' 'clamp.*transparency|Math\.(min|max).*transparency' -B 2 -A 2
Length of output: 153
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/app/view/term/term.tsx (1)
492-525: Consider using a constant for the default transparency value.The transparency menu implementation is solid, but consider extracting the default transparency value (0.5) into a named constant for better maintainability.
+ const DEFAULT_TERM_TRANSPARENCY = 0.5; transparencySubMenu.push({ label: "Transparent Background", type: "checkbox", - checked: transparencyMeta == 0.5, + checked: transparencyMeta == DEFAULT_TERM_TRANSPARENCY, click: () => { RpcApi.SetMetaCommand(TabRpcClient, { oref: WOS.makeORef("block", this.blockId), - meta: { "term:transparency": 0.5 }, + meta: { "term:transparency": DEFAULT_TERM_TRANSPARENCY }, }); }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/docs/keybindings.mdx(1 hunks)frontend/app/view/term/term.tsx(8 hunks)
🔇 Additional comments (5)
docs/docs/keybindings.mdx (1)
74-81: LGTM! Terminal keybindings are well documented.
The new terminal keybindings section clearly documents the standard terminal operations (copy, paste, clear) with their respective keyboard shortcuts, maintaining consistency with common terminal conventions.
frontend/app/view/term/term.tsx (4)
208-213: LGTM! Transparency configuration is well implemented.
The transparency configuration is properly implemented with:
- Bounded values between 0 and 1
- Default value of 0.5
- Proper atom setup for reactive updates
217-218: LGTM! Theme computation properly integrates transparency.
The theme computation correctly incorporates the transparency value when calculating background colors.
419-423: LGTM! Terminal clear keybinding is properly implemented.
The Cmd+k keybinding implementation:
- Correctly prevents default event behavior
- Properly clears the terminal
- Matches the documented behavior in keybindings.mdx
791-792: LGTM! Theme updates properly handle transparency changes.
The useEffect hook correctly incorporates transparency changes into theme updates.
No description provided.