Skip to content

Fixes #26305: Implement progressive infinite scroll for Explore dropdowns#27747

Open
mohitjeswani01 wants to merge 2 commits intoopen-metadata:mainfrom
mohitjeswani01:feat/26305-explore-dropdown-infinite-scroll
Open

Fixes #26305: Implement progressive infinite scroll for Explore dropdowns#27747
mohitjeswani01 wants to merge 2 commits intoopen-metadata:mainfrom
mohitjeswani01:feat/26305-explore-dropdown-infinite-scroll

Conversation

@mohitjeswani01
Copy link
Copy Markdown

Describe your changes:

Fixes #26305

I worked on implementing a progressive infinite scroll mechanism for Explore page quick filters (Tags, Owners) because loading 15,000+ aggregation buckets simultaneously caused severe browser hangs and DOM rendering bottlenecks.

What changes did I make?

  • Progressive Size Increment Strategy: Because Elasticsearch terms aggregations do not natively support offset/cursor-based pagination (from/after), I implemented a progressive size increment strategy (fetching size=50, then size=100, etc.) to simulate infinite scrolling.
  • Opt-in Architecture: Added an isPaginated boolean prop to SearchDropdown (defaulting to false) to ensure we don't break backward compatibility with Domain, Glossary, and Lineage page usages.
  • Clean Event Handling: Wrapped the dropdownRender menu in a scrollable div with a pure React onScroll listener, completely avoiding fragile document.querySelector DOM manipulations.

How did I test my changes?

  • Jest Unit Tests: Added 6 new tests to SearchDropdown.test.tsx verifying scroll threshold mathematics, isLoadingMore guards, and UI state changes.
  • Playwright E2E Tests: Added a dedicated test in ExploreQuickFilters.spec.ts that intercepts the /search/aggregate network request to explicitly verify that size=50 and size=100 payloads are requested sequentially upon scrolling.

(Note: As my local environment is resource-constrained, UI screenshots are omitted, but functionality is strictly verified via the attached Playwright CI tests).
image

Type of change:

  • Improvement

Checklist:

  • I have read the CONTRIBUTING document.

  • My PR title is Fixes #26305: Implement progressive infinite scroll for Explore dropdowns

  • I have commented on my code, particularly in hard-to-understand areas.

  • I have added tests around the new logic.


🚀 A Personal Note: WeMakeDevs × OpenMetadata Hackathon Wrap-up

This PR marks my final contribution for the WeMakeDevs hackathon. Pushing 24 PRs over the last two weeks—ranging from AI Root Cause Analysis engines to Core DB Reliability and Frontend optimizations—has been an incredible engineering gauntlet. I deeply appreciate the maintainers (@harshach, @ulixius9, @Rohit0301 and complete collate team) for their rigorous reviews and high standards. The hackathon may be ending, but I will definitely be sticking around the OpenMetadata community. Looking forward to what we build next!

@mohitjeswani01 mohitjeswani01 requested a review from a team as a code owner April 26, 2026 21:18
Copilot AI review requested due to automatic review settings April 26, 2026 21:18
@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@mohitjeswani01
Copy link
Copy Markdown
Author

Hi @harshach Sir Could you please add the safe to test label so i can monitor the CI? thank you 🙏!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Apr 26, 2026

Code Review 👍 Approved with suggestions 3 resolved / 4 findings

Implement progressive infinite scroll for Explore dropdowns, resolving race conditions and unnecessary API calls. Address potential cross-dropdown state interference to ensure independent pagination across fields.

💡 Bug: Shared pagination state may cause cross-dropdown interference

📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreQuickFilters.tsx:53-64 📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreQuickFilters.tsx:267-275

isLoadingMore, hasMore, options, currentSizeRef, activeFieldRef, and searchTextRef are all single instances shared across every SearchDropdown rendered in the fields.map(). Since Ant Design <Dropdown> components don't auto-close siblings, if a user rapidly switches between dropdowns, the stale hasMore/isLoadingMore state from the previous dropdown could affect the newly opened one.

In practice this is partially mitigated because getInitialOptions resets the state, but there's a timing window between opening a new dropdown and the reset completing.

Suggested fix
Consider keying the pagination state by the active field key, or moving `isLoadingMore`/`hasMore` into per-field state (e.g., a `Record<string, { isLoadingMore: boolean; hasMore: boolean }>`).
✅ 3 resolved
Bug: aggregations prop silently ignored, causing unnecessary API calls

📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreQuickFilters.tsx:39-46
The aggregations prop was removed from the component's destructured parameters, but it still exists in ExploreQuickFiltersProps and callers like ExploreV1.component.tsx and AssetsTabs.component.tsx still pass it. Previously, fetchDefaultOptions checked aggregations?.[key]?.buckets before making an API call — this was an optimization that avoided a redundant network request when aggregation data was already available from the parent's search response. Now every dropdown open triggers a fresh /search/aggregate call even when the data is already in props, introducing unnecessary latency and server load.

Bug: Race condition: concurrent scroll events bypass isLoadingMore guard

📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreQuickFilters.tsx:232-246 📄 openmetadata-ui/src/main/resources/ui/src/components/SearchDropdown/SearchDropdown.tsx:244-255
In handleScrollEnd, the isLoadingMore guard reads from the React state closure. When rapid scroll events fire before React re-renders (which is common since scroll events fire at high frequency), multiple invocations all see isLoadingMore = false and proceed to call fetchAggregationBuckets concurrently. This results in duplicate API requests with the same parameters. The same issue exists in handleMenuScroll in SearchDropdown.tsx which also checks the isLoadingMore prop from the closure.

A useRef is the correct synchronization primitive here since it is updated synchronously and reads always reflect the latest value.

Edge Case: E2E test.afterAll at module scope may conflict with test isolation

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ExploreQuickFilters.spec.ts:487-494
The test.afterAll('Cleanup') block at line 487 is at module scope (outside any test.describe). It deletes entities (table, domain, tier, tierWithoutAsset) that were created in earlier test.describe blocks. If Playwright runs describe blocks in parallel or in a different worker, these entities may not exist, causing the cleanup to fail. Additionally, the new test.afterAll('Cleanup metric entity') at line 430 inside the 'Metric search result highlight' describe block deletes the metric, but the module-scope afterAll may also try to reference shared state.

🤖 Prompt for agents
Code Review: Implement progressive infinite scroll for Explore dropdowns, resolving race conditions and unnecessary API calls. Address potential cross-dropdown state interference to ensure independent pagination across fields.

1. 💡 Bug: Shared pagination state may cause cross-dropdown interference
   Files: openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreQuickFilters.tsx:53-64, openmetadata-ui/src/main/resources/ui/src/components/Explore/ExploreQuickFilters.tsx:267-275

   `isLoadingMore`, `hasMore`, `options`, `currentSizeRef`, `activeFieldRef`, and `searchTextRef` are all single instances shared across every `SearchDropdown` rendered in the `fields.map()`. Since Ant Design `<Dropdown>` components don't auto-close siblings, if a user rapidly switches between dropdowns, the stale `hasMore`/`isLoadingMore` state from the previous dropdown could affect the newly opened one.
   
   In practice this is partially mitigated because `getInitialOptions` resets the state, but there's a timing window between opening a new dropdown and the reset completing.

   Suggested fix:
   Consider keying the pagination state by the active field key, or moving `isLoadingMore`/`hasMore` into per-field state (e.g., a `Record<string, { isLoadingMore: boolean; hasMore: boolean }>`).

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@mohitjeswani01
Copy link
Copy Markdown
Author

@gitar-bot
Acknowledged as a theoretical edge case.
In practice, this is fully mitigated by the current implementation:
getInitialOptions resets all state synchronously before any async work: isLoadingMoreRef.current = false, currentSizeRef.current = pageSize, setIsLoadingMore(false), setHasMore(false), setOptions([]).

isLoadingMoreRef is a ref, not state — it updates synchronously, so even if a user rapidly switches dropdowns, the guard is immediately cleared for the new dropdown.

AntD with destroyPopupOnHide destroys the previous popup DOM, so the old scroll container and its event listeners are torn down before the new one mounts.

Moving to per-field Record<string, { isLoadingMore, hasMore }> would add complexity without solving a real user-facing issue. Happy to revisit if testing reveals an actual race, but the current synchronous reset approach is safe for this use case.

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.

Infinite scroll in dropdowns on explore page

2 participants