Skip to content

fix: DH-23165: Fix spectrum Picker closing Popper dialogs on open - #2726

Open
vbabich wants to merge 6 commits into
deephaven:mainfrom
vbabich:vlad-DH-23165
Open

fix: DH-23165: Fix spectrum Picker closing Popper dialogs on open#2726
vbabich wants to merge 6 commits into
deephaven:mainfrom
vbabich:vlad-DH-23165

Conversation

@vbabich

@vbabich vbabich commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Add optional containPortals prop to the Popper dialog to allow nested spectrum Pickers and not dismiss the dialog on Picker open.

Verified manually on the http://localhost:4220/ide/styleguide#popover-dialog page and the PivotBuilder plugin aggregations config section (will post a separate PR in the deephaven-plugins repo).

Added unit and e2e tests.

@vbabich
vbabich marked this pull request as ready for review July 23, 2026 19:38
@vbabich
vbabich requested a review from Copilot July 23, 2026 19:38

Copilot AI 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.

Pull request overview

Adds opt-in portal containment to Popper, preventing nested Spectrum overlays from dismissing their parent dialog.

Changes:

  • Introduces containPortals using React Aria’s portal provider.
  • Adds stacking-context isolation for contained overlays.
  • Adds styleguide, unit, and E2E regression coverage.

Reviewed changes

Copilot reviewed 6 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/components/src/popper/Popper.tsx Implements portal containment.
packages/components/src/popper/Popper.scss Isolates contained overlay stacking.
packages/components/src/popper/Popper.test.tsx Tests opt-in and default rendering.
packages/components/package.json Adds the React Aria dependency.
packages/code-studio/src/styleguide/Dialog.tsx Adds a nested Picker example.
tests/styleguide.spec.ts Adds the E2E visual regression test.
package-lock.json Records the dependency update.

@vbabich
vbabich requested a review from mofojed July 23, 2026 19:47
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 44.44444% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.51%. Comparing base (b59031c) to head (a77c9e8).

Files with missing lines Patch % Lines
packages/code-studio/src/styleguide/Dialog.tsx 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2726      +/-   ##
==========================================
+ Coverage   51.40%   51.51%   +0.10%     
==========================================
  Files         797      797              
  Lines       45852    45859       +7     
  Branches    11906    11720     -186     
==========================================
+ Hits        23569    23622      +53     
+ Misses      22264    22218      -46     
  Partials       19       19              
Flag Coverage Δ
unit 51.51% <44.44%> (+0.10%) ⬆️

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.

@vbabich vbabich self-assigned this Jul 23, 2026
@vbabich
vbabich enabled auto-merge (squash) July 23, 2026 20:05
Comment thread tests/styleguide.spec.ts
@vbabich
vbabich requested a review from mofojed August 3, 2026 15:09
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.

3 participants