Skip to content

feat(ui-core): add FilterSelect — unified filter dropdown primitive - #33011

Merged
harsh-vador merged 26 commits into
mainfrom
feat-filter-select-core
Sep 11, 2026
Merged

harsh-vador merged 26 commits into
mainfrom
feat-filter-select-core

Conversation

@harsh-vador

@harsh-vador harsh-vador commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Describe your changes

Fixes #29569

Part of the filter-dropdown unification effort (#31999 and the Collate explore-filter polish epic). This adds the shared primitive; follow-up PRs swap the app's filter dropdowns (SearchDropdown, QuickFilterDropdown, FilterChip, …) onto it.

FilterSelect (ui-core-components/application/filter-select/) — a filter dropdown built on the existing react-aria primitives (Dropdown, Input, Checkbox, Button):

  • Trigger variants: chip (pill), button, input-style — the three shapes used across the app today.
  • Selection: single / multiple; commits immediate (per toggle) or staged (Apply / Cancel / Clear all footer).
  • Search: built-in box; delegates to onSearch for async option fetching, otherwise filters locally.
  • Uniform rows: every row — value rows, the pinned "No X" nullOption, Select All — renders through Dropdown.Item + CheckboxBase size="sm", so checkbox size, alignment, and typography cannot drift per consumer.
  • Select All (Enhancement: Add "Select All" Support for Multi-Select Filter Dropdowns #29569): tri-state header row scoped to the currently displayed (filtered) rows; deselect keeps selections hidden by the active search.
  • Robustness: resolves react-aria's 'all' selection sentinel (⌘A) to concrete values; merges persisted-but-unfetched selected values (e.g. restored from a URL) into the list via resolveMissingLabel.
  • Counts as row addons, hideCounts to suppress; isLoading / emptyState handling.
Screen.Recording.2026-09-11.at.11.25.52.AM.mov

i18n

Adds six core-namespace keys (apply, cancel, clear-all, no-data-found, search, select-all) with real translations in all 20 language files. yarn check-i18n-all passes (548 translations, none English-fallback).

Testing

  • 14 unit tests (vitest + RTL) covering immediate/staged commits, select-all scoping, null-option rendering, local vs async search, missing-value merge, single-select, trigger count, empty state.
  • Storybook stories (immediate, staged + null option, single-select input variant).
  • yarn type-check, yarn lint, yarn build all clean. The one failing table test on main (can omit the selection cell for a full-width synthetic row) pre-exists and is unrelated.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

@github-actions github-actions Bot added the UI UI specific issues label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

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!

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 67cdedf8cbe68eaf6ddd2c6afa06470f6b6f6ec9 in Playwright run 34567299292, attempt 1.

✅ 576 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 46m 27s

⏱️ Max setup 4m 24s · max shard execution 15m 17s · max shard-job elapsed before upload 20m 4s · reporting 4s

🌐 225.07 requests/attempt · 2.25 app boots/UI scenario · 26.50% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 26.5% (convergence target: at most 15%).
  • Browser traffic was 225.07 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.25 per UI scenario (1346 boots / 597 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 141 0 0 0 0 0
✅ Shard chromium-02 148 0 0 0 0 0
✅ Shard chromium-03 129 0 0 0 0 0
✅ Shard data-asset-rules-01 65 0 0 0 0 0
✅ Shard domain-isolation-01 16 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 11 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@harsh-vador harsh-vador added the safe to test Add this label to run secure Github workflows on PRs label Sep 9, 2026
@harsh-vador harsh-vador self-assigned this Sep 9, 2026
harsh-vador and others added 3 commits September 9, 2026 11:59
A single filter-dropdown component so app-level filter dropdowns
(SearchDropdown, QuickFilterDropdown, FilterChip, and the Collate-side
variants) can converge on one implementation. Built on react-aria via the
existing Dropdown, Input, Checkbox and Button primitives.

Features:
- trigger variants: chip (pill), button, input-style
- single/multiple selection; immediate or staged (Apply/Cancel) commits
- search box with async delegation (onSearch) or local filtering
- per-row leading icon, count addon, uniform CheckboxBase size=sm rows
- pinned null option ("No X") rendered through the same row component
- tri-state Select All over the currently displayed (filtered) rows
- merges persisted-but-unfetched selected values into the list
- resolves react-aria's 'all' selection sentinel to concrete values

Adds six core locale keys (apply, cancel, clear-all, no-data-found,
search, select-all) translated across all 20 language files; check-i18n-all
passes. 14 unit tests, Storybook stories, type-check/lint/build clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Staged selections were only reset inside react-aria's onOpenChange, which
never fires for a programmatic open via the controlled isOpen prop. Move the
resync into an effect on the resolved open state so both paths refresh
staged from selectedValues.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Output of .github/scripts/generate_playwright_impact_map.py on current main;
the committed map had drifted (Tag.spec.ts split, StyleModal spec pruning).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
App-side filter options carry already-rendered icon elements
(getEntityIconWithBg, getServiceLogo), so the icon prop now takes an FC or a
ReactNode (isReactComponent guard). helperText renders a muted line under
the search box — the Explore page shows one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
isForwardRefComponent read component.$$typeof.toString() on any non-null
object; arrays and fragments have no $$typeof, so a FilterSelect icon given
as an array of nodes threw a TypeError. Optional-chain the read.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The legacy dropdown pinned its helper line under the scrollable list; only
the options scroll, the helper stays visible.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
harsh-vador and others added 2 commits September 10, 2026 15:16
Review feedback: 14px rows plus a 256px popover truncated FQN-length
labels; options render at 12px in a 320px popover.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
harsh-vador and others added 3 commits September 10, 2026 16:24
…ed story

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The chip default made every story render the pill trigger; Explore and the
migrated surfaces use the borderless button variant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- xs checkbox size on CheckboxBase/Checkbox; menu rows and Select all use it
- typography prop: trigger label 'medium' (default) or 'regular'; menu rows
  always regular, secondary text turning primary when selected
- selection count as a circular badge on the trigger instead of "Label: (N)";
  bordered trigger turns brand (text + border) when a filter is active
- row counts as bordered pills with locale separators (replaces Dropdown addon)
- no background on selected rows — the checkbox alone conveys selection
- footer: p-3, sm buttons, Clear all back to tertiary with regular weight
- input variant: placeholder prop, count badge, and triggerDisplay="chips" —
  removable chips in the field, popover anchored to the whole field
- stories rebuilt as product scenarios (trigger weight, filter bar, immediate,
  bordered, input forms); tests extended to 24 covering the new behavior

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
harsh-vador and others added 6 commits September 10, 2026 20:25
MenuTrigger publishes its press-to-open props through ButtonContext, and
every react-aria Button descendant consumes it. The chips field's remove
buttons were aria Buttons, so each ✕ press ran onRemove *and* toggled the
popover, and the trigger ref was claimed by whichever button mounted last.

Make the remove controls native buttons — they never read ButtonContext —
leaving the trailing aria button as the sole menu trigger. Cover both
directions: removing a chip leaves the menu closed, and clicking the field
trigger opens it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Immediate mode has nothing to apply, so the Apply footer never rendered and
there was no way to drop a whole selection without unticking each row. Give
it a quiet footer instead: the selection count on the left, Clear all on the
right, disabled while nothing is selected.

Adds label.count-selected and label.none-selected, translated across all 20
locales.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Apply reads "Apply (2)" while values are staged and falls back to plain
"Apply" once the staged set is empty, so the button says how much it is
about to commit.

Adds label.apply-count, translated across all 20 locales — a dedicated key
rather than appending "(n)" in code, so RTL locales place the parenthetical
themselves.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop the font-normal override on both footers' Clear all so it renders at
the sm button's own medium weight, against the regular-weight selection
count beside it. Both were already disabled while nothing is selected —
pin that with a test over the staged footer's round trip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 3 resolved / 3 findings

Introduces FilterSelect, a unified filter dropdown primitive supporting trigger variants (chip, button, input), single/multiple selection with immediate or staged commits, built-in search with async option fetching, tri-state Select All scoped to filtered rows, and count displays. Resolves staged selection resyncing on controlled open, Array/fragment icon node type checking, and chips field button nesting. Includes 14 unit tests, Storybook stories, i18n translations across 20 languages, and passes type-check, lint, and build. No issues found.

✅ 3 resolved
Edge Case: Staged selection not resynced on controlled/programmatic open

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/filter-select/filter-select.tsx:167-169 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/filter-select/filter-select.tsx:234-241
staged is only refreshed inside handleOpenChange, which react-aria's MenuTrigger fires on user interaction — not when the dropdown is opened by flipping the controlled isOpen prop. When a consumer opens the filter programmatically (controlled isOpen) in staged mode, or selectedValues changes while the popover is already open, staged stays stale and the checkboxes/footer reflect an outdated selection until the next user-driven open. Resync staged from selectedValues when the resolved open state transitions to true, independent of the react-aria callback.

Edge Case: Array/fragment icon node can throw in isReactComponent

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/filter-select/filter-select.tsx:118-121
With icon now widened to FC<{className?}> | ReactNode, an array of nodes is a type-valid value. When such an array reaches isReactComponentisForwardRefComponent, the guard does component.$$typeof.toString(); arrays are typeof 'object' and non-null but have no $$typeof, so this throws TypeError: Cannot read properties of undefined. Single elements and components are unaffected (elements carry $$typeof). If consumers may pass fragment/array icon nodes, guard against a missing $$typeof (e.g. component.$$typeof?.toString() === 'Symbol(react.forward_ref)') or document that icon must be a single element/component.

Edge Case: Chips field nests multiple buttons inside one MenuTrigger

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/filter-select/filter-select.tsx:170-184 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/application/filter-select/filter-select.tsx:475-489
ChipsField renders a plain <div> as the Dropdown.Root (react-aria MenuTrigger) child, containing one AriaButton per chip-remove plus the trailing menu-trigger AriaButton. react-aria broadcasts the trigger's press/open props through ButtonContext to every descendant Button that consumes it, so each chip's remove button likely inherits the menu's press-to-open handler in addition to its own onRemove — clicking ✕ would fire onChange AND open the popover, and the trigger ref/press target is ambiguous. The added tests only exercise chips with isOpen={false} (render + remove) and never open the dropdown by clicking, so this path is uncovered. Verify open-by-click and remove-by-click behave independently (e.g. wrap the remove buttons outside the MenuTrigger, or make a single dedicated trigger element), and add a test that opens the chips popover via click.

Options

Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancement: Add "Select All" Support for Multi-Select Filter Dropdowns

2 participants