Skip to content

PM-40885: bug: Support overriding the colorscheme - #7206

Merged
david-livefront merged 1 commit into
mainfrom
PM-40885-colorscheme-override
Jul 24, 2026
Merged

PM-40885: bug: Support overriding the colorscheme#7206
david-livefront merged 1 commit into
mainfrom
PM-40885-colorscheme-override

Conversation

@david-livefront

@david-livefront david-livefront commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

PM-40885

📔 Objective

This PR fixes a bug in the QR Code Scan Screen (Password Manager and Authenticator) and the Card Scan Screen where we would force the UI to be in dark mode but the dynamic colors would be lost. This change still forces dark mode but preserves the dynamic color setting.

📸 Screenshots

Before After
Screen_recording_20260723_155121.mp4
after.mp4

@david-livefront
david-livefront requested a review from a team as a code owner July 23, 2026 20:50
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:bug Change Type - Bug labels Jul 23, 2026
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed this bug fix that preserves dynamic (Material You) colors on the QR Code Scan and Card Scan screens while still forcing them to render in dark mode. The change introduces a new ColorSchemeOverride composable, provides light/dark dynamic color schemes via new CompositionLocals in BitwardenTheme, and migrates the three scan screens to the new helper. The logic is sound: dynamic schemes are used only when isDynamicTheme is active, otherwise the static dark/light schemes are applied.

Code Review Details

No blocking findings.

Verified:

  • Adding context as the first parameter of BitwardenTheme is backward-compatible — all call sites (both MainActivitys, previews, and compose test harnesses) use named arguments.
  • Narrowing dynamicBitwardenColorScheme and toMaterialColorScheme to internal is safe; both are only referenced within the :ui theme module.
  • The isDynamicTheme gate in ColorSchemeOverride correctly routes to the dynamic schemes since dynamicBitwardenColorScheme sets isDynamicTheme = true for both light and dark variants.

Minor (non-blocking, not posted inline): the KDoc on LocalBitwardenDynamicDarkColorScheme reads "This is only to be used for dynamic that require the UI to have its colors overridden" — likely intended to mirror the light variant's wording.

@david-livefront david-livefront added the ai-review-vnext Request a Claude code review using the vNext workflow label Jul 23, 2026
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 47.36842% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (f64a08d) to head (bbf7b31).

Files with missing lines Patch % Lines
...arden/ui/platform/base/util/ColorSchemeOverride.kt 37.50% 7 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7206      +/-   ##
==========================================
- Coverage   86.20%   85.86%   -0.34%     
==========================================
  Files         884     1032     +148     
  Lines       64657    67403    +2746     
  Branches     9674     9771      +97     
==========================================
+ Hits        55737    57878    +2141     
- Misses       5456     6020     +564     
- Partials     3464     3505      +41     
Flag Coverage Δ
app-data 17.89% <0.00%> (+0.15%) ⬆️
app-ui-auth-tools 19.10% <0.00%> (+0.31%) ⬆️
app-ui-platform 17.08% <0.00%> (+0.52%) ⬆️
app-ui-vault 27.94% <42.10%> (+0.56%) ⬆️
authenticator 6.11% <36.84%> (+<0.01%) ⬆️
lib-core-network-bridge 4.02% <0.00%> (+<0.01%) ⬆️
lib-data-ui 1.15% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aj-rosado aj-rosado left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@david-livefront

Copy link
Copy Markdown
Collaborator Author

Thanks @aj-rosado

@david-livefront
david-livefront added this pull request to the merge queue Jul 24, 2026
Merged via the queue into main with commit 79d515f Jul 24, 2026
28 of 30 checks passed
@david-livefront
david-livefront deleted the PM-40885-colorscheme-override branch July 24, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants