Skip to content

PM-38610: Bug: Update BitwardenBasicDialog to allow scrolling content#7029

Open
david-livefront wants to merge 1 commit into
release/2026.5-rc55from
PM-38610-scrollable-basic-dialog-rc
Open

PM-38610: Bug: Update BitwardenBasicDialog to allow scrolling content#7029
david-livefront wants to merge 1 commit into
release/2026.5-rc55from
PM-38610-scrollable-basic-dialog-rc

Conversation

@david-livefront
Copy link
Copy Markdown
Collaborator

🎟️ Tracking

🍒 Cherry pick
PM-38610

📔 Objective

This PR updates the BitwardenBasicDialog to be more like the BitwardenTwoButtonDialog in order to support scrolling content.

📸 Screenshots

Before After
Screen_recording_20260605_155645.mp4

@david-livefront david-livefront requested a review from a team as a code owner June 5, 2026 21:00
@david-livefront david-livefront changed the title PM-38610: Update BitwardenBasicDialog to allow scrolling content PM-38610: Bug: Update BitwardenBasicDialog to allow scrolling content Jun 5, 2026
@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 Jun 5, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Cherry-pick that rewrites BitwardenBasicDialog to wrap Dialog + Column with a verticalScroll on the message text, allowing long content to scroll instead of being clipped. The implementation mirrors the proven BitwardenTwoButtonDialog pattern (same maxDialogHeight/maxDialogWidth clamps, same scroll-driven dividers, same FlowRow button layout). Public API, parameter names, and all test tags are preserved, so the 60+ call sites across :app and :authenticator are unaffected. Before/after screenshots in the PR demonstrate the intended scrolling behavior.

Code Review Details

No findings.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.26%. Comparing base (3e92848) to head (899b4f8).

Additional details and impacted files
@@                   Coverage Diff                   @@
##           release/2026.5-rc55    #7029      +/-   ##
=======================================================
+ Coverage                86.08%   86.26%   +0.17%     
=======================================================
  Files                     1026     1008      -18     
  Lines                    66702    66464     -238     
  Branches                  9356     9323      -33     
=======================================================
- Hits                     57419    57333      -86     
+ Misses                    6063     5917     -146     
+ Partials                  3220     3214       -6     
Flag Coverage Δ
app-data 17.62% <ø> (ø)
app-ui-auth-tools 19.31% <ø> (ø)
app-ui-platform 16.99% <ø> (-0.01%) ⬇️
app-ui-vault 28.26% <ø> (-0.02%) ⬇️
authenticator 6.21% <ø> (+0.01%) ⬆️
lib-core-network-bridge 4.07% <ø> (-0.01%) ⬇️
lib-data-ui 1.14% <ø> (+<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.

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

Labels

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.

1 participant