Skip to content

fix(opencode): use themed TUI overlay dimmers#18750

Open
ChaoGlenXu wants to merge 1 commit intoanomalyco:devfrom
ChaoGlenXu:codex/fix-light-dialog-overlay
Open

fix(opencode): use themed TUI overlay dimmers#18750
ChaoGlenXu wants to merge 1 commit intoanomalyco:devfrom
ChaoGlenXu:codex/fix-light-dialog-overlay

Conversation

@ChaoGlenXu
Copy link

Issue for this PR

Closes #13363

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

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.ts

I also ran:

bun run typecheck

And I verified the branch push passed the repo pre-push hook, including:

bun turbo typecheck

Screenshots / recordings

Not included. This is a small TUI overlay color fix and I verified it through targeted tests and typecheck.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions
Copy link
Contributor

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.

Copy link

@atharvau atharvau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 overlayColor helper 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

  1. Style Conformance: The code follows the established patterns well
  2. Performance: Minimal overhead - just color calculation, no performance concerns
  3. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System theme makes dialogs/overlays look black on light terminals

2 participants