fix(ui): use Untitled-UI dropdowns for Add Assets drawer quick-filters#29168
fix(ui): use Untitled-UI dropdowns for Add Assets drawer quick-filters#29168siddhant1 wants to merge 1 commit into
Conversation
The Domains / Data Products "Add Assets" drawer is an Untitled-UI SlideoutMenu (react-aria Modal/Dialog). Its quick-filter dropdowns used the Ant Design SearchDropdown, whose popup portals to document.body — outside the dialog subtree, where react-aria's `inert` attribute makes it non-interactive (clicks fall through and the popup closes). z-index cannot fix `inert`. Add a QuickFilterDropdown built on the Untitled-UI PopoverTrigger/Popover (react-aria) with a search box, a checkbox option list with counts, single/multi-select, a null-option and Apply/Clear. Its popover renders in the dialog's top layer and stays interactive without any AntD-over-drawer workaround. ExploreQuickFilters renders it behind an `untitledDropdown` flag that only the Add Assets drawer passes (variant === 'drawer'); the Modal variant, Explore page, Glossary tab and Lineage controls keep the AntD SearchDropdown unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const handleClear = () => { | ||
| setSelectedOptions([]); | ||
| }; |
There was a problem hiding this comment.
handleClear leaves null option staged
handleClear only resets selectedOptions but never touches nullOptionSelected. When a user selects the null row alongside 2+ regular options, "Clear All" clears the checkboxes but leaves nullOptionSelected = true. The subsequent "Update" call then emits [{ key: NULL_OPTION_KEY, … }] — the null filter is silently re-applied even though the user asked to clear everything.
| const handleClear = () => { | |
| setSelectedOptions([]); | |
| }; | |
| const handleClear = () => { | |
| setSelectedOptions([]); | |
| setNullOptionSelected(false); | |
| }; |
| const debouncedOnSearch = useMemo( | ||
| () => debounce((value: string) => onSearch(value, searchKey), 500), | ||
| [onSearch, searchKey] | ||
| ); |
There was a problem hiding this comment.
debouncedOnSearch is created via useMemo, so a new debounced function is returned every time onSearch or searchKey changes. The previous debounced instance is never cancelled, meaning a 500 ms pending call on the old instance can still fire after the reference has changed. Because onSearch is passed as an inline arrow function from ExploreQuickFilters (which recreates it on every render), this happens on every parent re-render while the user is typing. The fix is to cancel the old function in a useEffect cleanup: return () => debouncedOnSearch.cancel();.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| const isOptionSelected = (option: SearchDropdownOption) => | ||
| selectedOptions.some((selected) => selected.key === option.key); | ||
|
|
||
| const showClearAllBtn = !singleSelect && selectedOptions.length > 1; |
There was a problem hiding this comment.
💡 Edge Case: Clear All does not reset the null-option selection
handleClear only clears the staged option list (setSelectedOptions([])) and never resets nullOptionSelected. The "Clear All" button is shown whenever selectedOptions.length > 1, independent of the null-option checkbox. So if a user has checked the "No " option plus two or more regular options and then clicks "Clear All", the null option stays checked and is still emitted by handleApply (values = [{ key: NULL_OPTION_KEY, ... }]). The user reasonably expects "Clear All" to clear everything, including the null row. Consider also resetting the null option in handleClear.
Reset the null-option staging state alongside the regular options when clearing.:
const handleClear = () => {
setSelectedOptions([]);
setNullOptionSelected(false);
};
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| const debouncedOnSearch = useMemo( | ||
| () => debounce((value: string) => onSearch(value, searchKey), 500), | ||
| [onSearch, searchKey] | ||
| ); |
There was a problem hiding this comment.
💡 Quality: Debounced onSearch is never cancelled on close/unmount
debouncedOnSearch (500ms debounce) is created via useMemo but is never cancelled. If a user types in the search box and then immediately closes the popover (or the component unmounts), the pending debounced call still fires after the timeout, triggering onSearch -> getFilterOptions, which calls setOptions/setIsOptionsLoading on the parent. At best this is a wasted aggregation request; it can also produce a state update for a no-longer-visible filter. Cancel the debounced function on unmount (and optionally when the popover closes).
Cancel any pending debounced search when the debounced function changes or the component unmounts.:
useEffect(() => {
return () => {
debouncedOnSearch.cancel();
};
}, [debouncedOnSearch]);
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
Code Review 👍 Approved with suggestions 0 resolved / 2 findingsReplaces Ant Design dropdowns with interactive Untitled-UI components in the Add Assets drawer to resolve react-aria inertness issues. Clear All fails to reset null-option selections, and the search debounce requires cleanup on unmount. 💡 Edge Case: Clear All does not reset the null-option selection📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/QuickFilterDropdown.tsx:93 📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/QuickFilterDropdown.tsx:135-148 📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/QuickFilterDropdown.tsx:178-189
Reset the null-option staging state alongside the regular options when clearing.💡 Quality: Debounced onSearch is never cancelled on close/unmount📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/QuickFilterDropdown.tsx:62-65 📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/QuickFilterDropdown.tsx:99-110
Cancel any pending debounced search when the debounced function changes or the component unmounts.🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Describe your changes:
The Domains / Data Products → Add Assets drawer quick-filters (Entity Type, Owners, Tag, Tier, Service) are now fully interactive.
Why: The Add Assets drawer is an Untitled-UI
SlideoutMenu(react-ariaModal/Dialog). The quick filters used the Ant DesignSearchDropdown, whose popup portals todocument.body— outside the dialog subtree, where react-aria'sinertattribute makes it non-interactive (clicks fall through to the content behind and the popup closes).z-indexcannot overrideinert; the popup must live inside the dialog's layer.What: Add a new
QuickFilterDropdownbuilt on the Untitled-UIPopoverTrigger/Popover(react-aria), whose popover renders in the dialog's own top layer — so it stays interactive with no AntD-over-drawer workaround (nogetPopupContainer, noConfigProvider). It reproduces the current filter UX: compact button trigger, an in-popover search box, a checkbox option list with counts, single/multi-select, a null-option row, and Clear/Apply (selection staged until Apply).ExploreQuickFiltersrenders it behind anuntitledDropdownflag that only the Add Assets drawer passes (variant === 'drawer'); the Add Assets modal variant, the Explore page, the Glossary asset tab and Lineage controls keep the existing AntDSearchDropdown— zero change to those surfaces.It reuses the existing data plumbing unchanged (
ExploreQuickFilterField, option fetching,getQuickFilterQuery), so behavior matches today's filters.Type of change:
High-level design:
Small, additive, drawer-scoped. One new consumer-app component composed from
@openmetadata/ui-core-componentsprimitives (PopoverTrigger,Popover,Button,Input,Checkbox) — no core-component changes.ExploreQuickFiltersgains an optionaluntitledDropdownprop (defaults tofalse), so the 4 other call sites are untouched. react-aria nestedPopoverinside theSlideoutMenudialog renders in the shared top layer — that's what keeps it interactive.Rejected approaches (per review): routing the AntD popup into the dialog via
getPopupContainer/ aConfigProviderdrawer provider (keeps an AntD overlay in the Untitled drawer); the field-styleMultiSelect/Autocomplete(wrong UX); enhancing the coreDropdownprimitive (touches a shared core component).Tests:
Use cases covered
Unit tests
QuickFilterDropdown.test.tsx— fetch-on-open, stage-then-Apply emits the selected value, single-select keeps one value, null-option emits the null key, debounced search callsonSearch.Backend / Ingestion integration tests
Playwright (UI) tests
InputOutputPorts.spec.ts→ Port drawers show Entity Type quick filter — upgraded from a visibility-only check to a real interactivity check: opens the filter popover inside the drawer and types into its search box (regression guard for the inertdocument.body-portal bug).Manual testing performed
tsc --noEmiton the changed files — clean. UI checkstyle (organize-imports → eslint → prettier) — clean. Jest (40 tests across the new + existing Explore filter suites) — pass.UI screen recording / screenshots:
Checklist:
🤖 Generated with Claude Code
Greptile Summary
Introduces
QuickFilterDropdown, a react-aria (PopoverTrigger/Popover) replacement for the AntDSearchDropdownin the Add Assets drawer. Because AntD portals todocument.body(which react-aria'sSlideoutMenumarksinert), the existing filters were non-interactive; this fix routes the popover through the same react-aria top layer as the drawer.QuickFilterDropdown— new self-contained component that reproduces the full filter UX (search, multi/single-select, counts, null option, stage-then-Apply). It is opt-in viauntitledDropdown={variant === 'drawer'}inuseAssetSelectionContent; all other call sites (Explore page, Glossary, Lineage) continue usingSearchDropdownunchanged.handleClearonly resetsselectedOptionsand leavesnullOptionSelectedunchanged, so "Clear All" does not actually clear the null-option selection; a subsequent "Update" silently re-applies it.Confidence Score: 3/5
Safe to merge for Explore/Glossary/Lineage surfaces; the drawer filter UX gains interactivity, but the Clear All button in QuickFilterDropdown leaves the null-option filter active when a user selects a null option alongside multiple regular options and then tries to clear everything.
The core motivation is sound and the architectural approach cleanly fixes the inert problem. However, handleClear only resets selectedOptions and never touches nullOptionSelected, so Clear All followed by Update still emits the null filter. This is a present functional defect on the new path that will be observable in the drawer filters for any field that has a null option (Owners, Tier). The fix is a one-liner, but the current code ships with wrong behavior.
openmetadata-ui/src/main/resources/ui/src/components/Explore/QuickFilterDropdown.tsx — the handleClear function and the debouncedOnSearch memo both need attention before merge.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[ExploreQuickFilters] -->|untitledDropdown=false| B[SearchDropdown\nAntD — portals to document.body] A -->|untitledDropdown=true| C[QuickFilterDropdown\nreact-aria PopoverTrigger] B -->|Portal target| D[document.body\ninert inside SlideoutMenu ❌] C -->|Top-layer popover| E[react-aria top layer\nshared with SlideoutMenu ✅] F[useAssetSelectionContent] -->|variant === drawer| G[untitledDropdown=true] F -->|variant === modal| H[untitledDropdown=false] G --> A H --> A C --> I[handleApply\nemit onChange] C --> J[handleClear\n⚠️ misses nullOptionSelected]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[ExploreQuickFilters] -->|untitledDropdown=false| B[SearchDropdown\nAntD — portals to document.body] A -->|untitledDropdown=true| C[QuickFilterDropdown\nreact-aria PopoverTrigger] B -->|Portal target| D[document.body\ninert inside SlideoutMenu ❌] C -->|Top-layer popover| E[react-aria top layer\nshared with SlideoutMenu ✅] F[useAssetSelectionContent] -->|variant === drawer| G[untitledDropdown=true] F -->|variant === modal| H[untitledDropdown=false] G --> A H --> A C --> I[handleApply\nemit onChange] C --> J[handleClear\n⚠️ misses nullOptionSelected]Reviews (1): Last reviewed commit: "fix(ui): use Untitled-UI dropdowns for A..." | Re-trigger Greptile