Skip to content

fix(dashboard): keep More filters reachable on cross-filter apply#41351

Closed
sadpandajoe wants to merge 9 commits into
masterfrom
fix-filterbar-crossfilter-overflow
Closed

fix(dashboard): keep More filters reachable on cross-filter apply#41351
sadpandajoe wants to merge 9 commits into
masterfrom
fix-filterbar-crossfilter-overflow

Conversation

@sadpandajoe

Copy link
Copy Markdown
Member

SUMMARY

When a horizontal filter bar has enough native filters to overflow into the "More filters" dropdown, applying a cross-filter (which prepends a chip to the bar) could make the overflowed native filters disappear from both the bar and the dropdown, while the "More filters" button itself also vanished, leaving the hidden filters unreachable. Clearing the cross-filter restored them. The failure depends on the number of configured filters, not screen resolution.

Root cause: on an item-set change DropdownContainer resets its positional overflow index and re-measures; if that measurement catches a transient mid-reflow layout it can settle a "nothing overflows" verdict that never self-corrects (the recalculation effect's dependencies don't change again). Because the trigger's visibility is derived solely from the overflow count, that one bad verdict both strands the surplus filters in the clipped bar and removes the trigger to reach them.

Fix: treat a post-item-change "nothing overflows" read as provisional and run a single requestAnimationFrame confirmation pass that re-measures once the browser has reflowed, keeping the trigger mounted across the confirmation window. The confirmation is armed on every item-set change (covering a fit→overflow transition, not only the already-overflowing case) and is versioned/cancelled so a superseded frame from a rapid second change cannot clobber the newer state. This extends the intent of #38193 (which guarded only the transient reset window) to the settled case, in the same overflow/button-visibility area as #28060.

BEFORE/AFTER

Behavioral overflow bug, no visual redesign. Covered by automated tests rather than screenshots; see Testing Instructions.

TESTING INSTRUCTIONS

  • cd superset-frontend && npm run test -- DropdownContainer.overflow
  • The new DropdownContainer.overflow.test.tsx drives the real overflow recalculation (mocking only the container width and per-element geometry) and covers: clean re-measurement after a prepended chip, the transient-latch regression, the fit→overflow transition, over-correction (a genuine fit drops the trigger), and re-entrant item-set changes.
  • Manual: on a dashboard with enough native filters to show "More filters" in the horizontal filter bar, apply a cross-filter from a chart; the overflowed native filters and the "More filters" button must remain present and reachable.

ADDITIONAL INFORMATION

  • Changes UI
  • Includes DB Migration
  • Introduces new feature or API
  • Removes existing feature or API

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.64706% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.20%. Comparing base (18f1dd3) to head (f7c69b3).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...components/DropdownContainer/DropdownContainer.tsx 92.64% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #41351      +/-   ##
==========================================
+ Coverage   65.03%   65.20%   +0.16%     
==========================================
  Files        2744     2692      -52     
  Lines      153627   149631    -3996     
  Branches    35226    34676     -550     
==========================================
- Hits        99915    97560    -2355     
+ Misses      51805    50302    -1503     
+ Partials     1907     1769     -138     
Flag Coverage Δ
javascript 70.42% <92.64%> (+0.02%) ⬆️

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.

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

Fixes a dashboard filter-bar overflow regression where applying a cross-filter (which prepends a “chip” item) could cause the “More filters” trigger to disappear and strand overflowed filters by adding a one-shot requestAnimationFrame confirmation re-measure after item-set changes, plus a dedicated overflow-engine regression test suite.

Changes:

  • Adds item-set-change detection and a versioned/cancellable rAF confirmation pass to prevent a transient “nothing overflows” measurement from latching.
  • Adjusts dropdown-trigger mounting logic to stay present across the full confirmation window.
  • Introduces DropdownContainer.overflow.test.tsx to exercise real overflow recalculation behavior under jsdom via mocked geometry and deterministic rAF.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
superset-frontend/packages/superset-ui-core/src/components/DropdownContainer/DropdownContainer.tsx Adds rAF-based confirmation re-measurement and trigger-mounting guards to keep overflowed filters reachable.
superset-frontend/packages/superset-ui-core/src/components/DropdownContainer/DropdownContainer.overflow.test.tsx Adds regression tests covering cross-filter prepend, transient mis-measurement, fit→overflow, and re-entrant item-set changes.

@netlify

netlify Bot commented Jun 25, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 7829ad1
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a571b2ca6171300082a53a0
😎 Deploy Preview https://deploy-preview-41351--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@sadpandajoe
sadpandajoe force-pushed the fix-filterbar-crossfilter-overflow branch 2 times, most recently from f708027 to 1385377 Compare July 3, 2026 21:54
@sadpandajoe sadpandajoe added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Jul 3, 2026
@github-actions github-actions Bot added 🎪 1385377 🚦 building 🎪 ⌛ 48h Environment expires after 48 hours (default) and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Jul 3, 2026
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🎪 Showtime is building environment on GHA for 1385377

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🎪 Showtime deployed environment on GHA for 1385377

Environment: http://18.236.196.225:8080 (admin/admin)
Lifetime: 48h auto-cleanup
Updates: New commits create fresh environments automatically

@github-actions

Copy link
Copy Markdown
Contributor

🎪 Showtime deployed environment on GHA for f7c69b3

Environment: http://35.92.37.38:8080 (admin/admin)
Lifetime: 48h auto-cleanup
Updates: New commits create fresh environments automatically

@github-actions

Copy link
Copy Markdown
Contributor

🎪 Showtime is building environment on GHA for 7829ad1

@github-actions

Copy link
Copy Markdown
Contributor

🎪 Showtime deployed environment on GHA for 7829ad1

Environment: http://34.220.51.51:8080 (admin/admin)
Lifetime: 48h auto-cleanup
Updates: New commits create fresh environments automatically

@sadpandajoe

Copy link
Copy Markdown
Member Author

Closing this one.

The PR targeted a latch in the overflow recalculation: when the item set changes, the reset path zeroes the overflowed items, so the existing index === -1 && overflowedItems.length > 0 guard can't fire on the pass that follows, and a transient "everything fits" measurement could settle to -1 with no dependency change left to correct it.

That path is real in the code, but for it to bite, getBoundingClientRect() has to report a container wider than its settled width. gBCR forces synchronous layout, so it returns settled geometry rather than mid-reflow geometry — and the resize detector on the container plus the ResizeObserver on the children already re-run the recalculation whenever geometry genuinely changes. The behaviour that prompted this never reproduced on master, and #38193 already covers the flicker case it was traced to.

The regression suite here doesn't close that gap either. Against master's component it fails only where it forces getBoundingClientRect to return a transient value, or where it asserts on the requestAnimationFrame machinery this PR itself introduces; the two tests that measure honestly pass on master unchanged. So it demonstrates that master doesn't recover from a synthetic transient, not that there's a browser-reproducible defect.

On that basis the added ref/rAF coordination isn't carrying its weight in a shared core component. Happy to revisit if a reproduction on master turns up.

@github-actions github-actions Bot removed 🎪 7829ad1 📅 2026-07-15T05-32 🎪 ⌛ 48h Environment expires after 48 hours (default) labels Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants