Skip to content

fix(search-filters): prevent nested filter dropdowns from disappearing on reopen#2421

Merged
kodiakhq[bot] merged 3 commits into
mainfrom
cursor/fix-nested-filter-reopen-9811
Jun 5, 2026
Merged

fix(search-filters): prevent nested filter dropdowns from disappearing on reopen#2421
kodiakhq[bot] merged 3 commits into
mainfrom
cursor/fix-nested-filter-reopen-9811

Conversation

@MikeShi42
Copy link
Copy Markdown
Contributor

Summary

Nested filter dropdowns in ClickStack Search (e.g. ResourceAttributes, LogAttributes) showed no filters after being closed and reopened.

Root Cause

The bug was introduced by PR #2096 (Mantine v7 to v9 upgrade). In Mantine v9, Accordion.Panel uses a Collapse component that transitions through display: none state when closed. The NestedFilterGroup component previously used {isExpanded && ...} to conditionally mount its panel content, including the scroll container used by @tanstack/react-virtual.

When reopening:

  1. isExpanded becomes true - content mounts inside the Panel
  2. But the Collapse wrapper still has display: none (until an async requestAnimationFrame fires)
  3. The virtualizer observes the scroll element via useLayoutEffect and calls getBoundingClientRect() - returns {width: 0, height: 0} because the element is inside display: none
  4. With outerSize = 0, the virtualizer's calculateRange() returns null - getVirtualItems() returns []
  5. No child filter items are rendered

In Mantine v7, this pattern worked because the Panel either fully unmounted its content or didn't transition through display: none, so the virtualizer always had valid dimensions when content was present.

Fix

Remove the {isExpanded && ...} conditional wrapper from NestedFilterGroup. The scroll container now stays mounted inside the Accordion.Panel at all times, maintaining the virtualizer's connection to it.

This is safe because:

  • The virtualizer already uses count: isExpanded ? childFilters.length : 0 to prevent rendering items when collapsed
  • The Accordion.Panel's built-in Collapse animation handles visibility
  • When reopening, the ResizeObserver detects the scroll element's dimension change (from display: none to display: block) and the virtualizer renders items correctly

Testing

  • All 1208 unit tests pass
  • Added a regression test that verifies open - close - reopen behavior keeps the panel accessible
  • Lint and TypeScript checks pass

Linear Issue: HDX-4471

Open in Web Open in Cursor 

…g on reopen

In Mantine v9, the Accordion Panel uses a Collapse component that
transitions through display:none state. NestedFilterGroup previously used
{isExpanded && ...} to conditionally mount its content inside the Panel.
This caused the virtualizer's scroll element to be inside a display:none
container when reopening, resulting in zero dimensions from
getBoundingClientRect(). With no viewport size, the virtualizer rendered
zero items, making the dropdown appear empty.

The fix removes the conditional mount guard. The virtualizer already uses
count: isExpanded ? childFilters.length : 0 to control rendering, so no
items are rendered when collapsed. The Accordion Panel's built-in Collapse
animation handles visibility. When reopening, the scroll element is
already in the DOM, allowing the virtualizer's ResizeObserver to detect
dimension changes and render items correctly.

Fixes: HDX-4471

Co-authored-by: Mike Shi <mike@hyperdx.io>
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 5, 2026

🦋 Changeset detected

Latest commit: de1ec84

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jun 5, 2026 2:23pm
hyperdx-storybook Ready Ready Preview, Comment Jun 5, 2026 2:23pm

Request Review

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

E2E Test Results

All tests passed • 198 passed • 3 skipped • 1286s

Status Count
✅ Passed 198
❌ Failed 0
⚠️ Flaky 3
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

@brandon-pereira
Copy link
Copy Markdown
Member

Tested and works, thanks!

@brandon-pereira brandon-pereira marked this pull request as ready for review June 5, 2026 14:20
@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Jun 5, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 1
  • Production lines changed: 209 (+ 23 in test files, excluded from tier calculation)
  • Branch: cursor/fix-nested-filter-reopen-9811
  • Author: MikeShi42

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

Deep Review

✅ No critical issues found. The fix is sound: removing the {isExpanded && …} mount guard keeps the @tanstack/react-virtual scroll container connected across collapse/reopen, and the retained count: isExpanded ? childFilters.length : 0 guard still prevents rows from rendering while collapsed. The correctness, performance, and frontend-races reviewers all confirmed the approach resolves the regression without introducing a runtime failure on the happy path.

🟡 P2 — recommended

  • packages/app/src/components/__tests__/DBSearchPageFilters.test.tsx:695 — the new regression test asserts only the panel's aria-hidden toggling, which is driven by Mantine's Accordion and would pass against the unfixed code, so it never exercises the actual symptom (child FilterGroup items failing to reappear after reopen).
    • Fix: after the final reopen, assert screen.getAllByTestId(/nested-filter-group-attr/) has length > 0, mocking getBoundingClientRect to a non-zero height so the virtualizer materializes rows in jsdom.
    • testing, correctness, maintainability, kieran-typescript, project-standards
  • packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx:94 — the always-mounted scroll container now relies on an undocumented invariant coupled to count: isExpanded ? childFilters.length : 0; a future editor re-adding an {isExpanded && …} guard to trim hidden DOM would silently reintroduce this exact bug.
    • Fix: add a comment at the virtualizer/scroll-container explaining that the scroll element must remain mounted and that visibility is controlled solely by the count gate.
    • maintainability
🔵 P3 nitpicks (3)
  • packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx:153 — the empty-state Group ("No properties found") and the scroll container now mount in the DOM even while collapsed, a behavior change from the prior gated mount; harmless because Mantine's Collapse hides it via CSS.
    • Fix: optionally note the intentional always-mounted behavior, or leave as-is since it is functionally invisible.
  • .git/HEAD:1 — the branch uses a cursor/ prefix, while AGENTS.md:176 requires agent-generated branches to use claude/, agent/, or ai/.
    • Fix: name future agent branches with one of the sanctioned prefixes.
  • packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx:1 — commit d48ed04 carries a Co-Authored-By trailer, which AGENTS.md:204 prohibits.
    • Fix: omit Co-Authored-By trailers on future commits.

Pre-existing (not introduced by this diff)

  • packages/app/src/components/DBSearchPageFilters/NestedFilterGroup.tsx:87isExpanded is initialized from hasSelections only at mount, so a group will not auto-expand if selectedValues arrives after mount; unchanged by this PR but more visible now that the panel stays mounted.

Reviewers (9): correctness, testing, maintainability, project-standards, kieran-typescript, julik-frontend-races, performance, agent-native, learnings-researcher.

Testing gaps:

  • The empty-state path (childFilters.length === 0) through a collapse/reopen cycle is uncovered.
  • No test covers childFilters changing while collapsed and then rendering the new count on expand.
  • Virtualizer row rendering is not directly assertable in jsdom without mocking getBoundingClientRect, so the entire NestedFilterGroup suite can only verify accordion state, not virtualized content.

@kodiakhq kodiakhq Bot merged commit 56c5866 into main Jun 5, 2026
20 checks passed
@kodiakhq kodiakhq Bot deleted the cursor/fix-nested-filter-reopen-9811 branch June 5, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants