fix(opencode): use themed TUI overlay dimmers#18750
fix(opencode): use themed TUI overlay dimmers#18750ChaoGlenXu wants to merge 1 commit intoanomalyco:devfrom
Conversation
|
The following comment was made by an LLM, it may be inaccurate: I found a potentially related PR:
This PR appears to be directly related to the current PR #18750, as both address the same issue (theme-aware overlay colors for TUI dialogs). You may want to verify if #14686 was previously closed/abandoned, or if #18750 is a fresh attempt to fix the same problem. Additionally, PR #13239 ("fix(tui): fix system theme contrast with transparency") may be tangentially related to theme contrast issues. |
atharvau
left a comment
There was a problem hiding this comment.
Code Review Summary
This PR fixes TUI overlay colors to be theme-aware instead of using hardcoded black backgrounds. It's a well-executed bug fix with good testing and clean implementation.
✅ Strengths
- Clear Problem/Solution: Addresses the specific issue of hardcoded black overlays on light terminals
- Clean Implementation: New
overlayColorhelper follows single responsibility principle - Good Test Coverage: Comprehensive unit tests covering all fallback scenarios
- Backwards Compatible: Preserves existing opacity behavior while fixing the color issue
- Consistent Application: Fixes the issue in both dialog backdrops and session sidebar overlays
✅ Code Quality
- Type Safety: Proper TypeScript types with good interface definition
- Error Prevention: Handles transparent colors gracefully with fallback logic
- Readable: Clear function logic with good naming conventions
- Testable: Pure function that's easy to unit test
✅ Testing
- Comprehensive Coverage: Tests primary use case, fallbacks, and custom opacity
- Good Test Structure: Clear test descriptions and expected behavior
- Edge Cases: Handles transparent backgroundMenu and backgroundPanel scenarios
🔍 Minor Observations
- Style Conformance: The code follows the established patterns well
- Performance: Minimal overhead - just color calculation, no performance concerns
- Maintainability: Helper function is easy to extend if more overlay types are needed
🎯 No Issues Found
- ✅ No security concerns
- ✅ No bugs detected
- ✅ No performance issues
- ✅ No style violations
- ✅ Good error handling
- ✅ Proper type safety
📊 Test Coverage: Excellent ✅
- Unit tests cover all code paths
- Edge cases properly tested
- Integration with existing theme system verified
🚀 Recommendations
This PR is ready to merge as-is. The implementation is clean, well-tested, and solves the stated problem effectively.
Overall Assessment: Approved ✅
This is a high-quality bug fix that improves the user experience without introducing any risks or technical debt.
Issue for this PR
Closes #13363
Type of change
What does this PR do?
This fixes TUI dialogs and overlay dimmers using a hardcoded translucent black backdrop, which looks wrong on light terminals when using the system theme.
The current code hardcodes black overlay colors for the dialog backdrop and the narrow-screen session sidebar overlay. This change replaces those hardcoded colors with a shared theme-aware overlay helper that derives the overlay color from the current TUI theme, while preserving the existing dimmer opacity.
That keeps dark terminals dark, but avoids black-looking overlays on light terminals.
How did you verify your code works?
I added a focused test for the overlay color helper and ran:
bun test test/cli/tui/overlay.test.tsI also ran:
bun run typecheckAnd I verified the branch push passed the repo pre-push hook, including:
bun turbo typecheckScreenshots / recordings
Not included. This is a small TUI overlay color fix and I verified it through targeted tests and typecheck.
Checklist