Skip to content

feat(search): allow free-text values when editing a filter pill#2471

Open
zoov-xavier wants to merge 3 commits into
hyperdxio:mainfrom
zoov-xavier:claude/editable-filter-pill-free-text-value
Open

feat(search): allow free-text values when editing a filter pill#2471
zoov-xavier wants to merge 3 commits into
hyperdxio:mainfrom
zoov-xavier:claude/editable-filter-pill-free-text-value

Conversation

@zoov-xavier

Copy link
Copy Markdown

Summary

The active filter pill's value picker (added in #2455) is a closed Select limited to the field's existing values. That means a filter can only ever be switched to a value already present in the sampled data. This PR replaces it with a Mantine Autocomplete so you can type any value while still seeing the existing suggestions.

  • Typed free text is committed on Enter or blur, in addition to picking from the suggestion list.
  • The picker opens with an empty input (the current value shown as a placeholder), so the full suggestion list is visible instead of being filtered down to just the current value.
  • Keyboard navigation is preserved: when an option is highlighted (Mantine marks it with [data-combobox-selected]), Enter submits that option natively via onOptionSubmit; free text is only committed when no option is highlighted.
  • A commit guard prevents applying the same value twice (picking an option fires onOptionSubmit, then onBlur as the menu closes).
  • Copy, polarity flip (include/exclude), one-click remove, and range/not-applied pill behavior are unchanged.

Why

Editing a pill only let you switch to a value that already exists in the data. When you want to filter on a value that hasn't appeared yet (a new service name, a release tag about to ship, an error code not seen in the sample window), you had no way to enter it from the pill — you had to type it as a raw query instead. Free-text entry closes that gap while keeping the suggestion-driven flow for the common case.

Test plan

  • yarn ci:unit ActiveFilterPills.test.tsx — 28 tests, incl. new coverage for free-typed values not in the list and for keyboard-navigated option selection via Enter
  • tsc --noEmit on @hyperdx/app — clean
  • yarn lint on @hyperdx/app — 0 errors
  • Manual: drove the pill menu in the dev stack — typing a value absent from the data and pressing Enter applies it; arrow-key navigation + Enter selects the highlighted option

A changeset (@hyperdx/app: patch) is included.

@vercel

vercel Bot commented Jun 16, 2026

Copy link
Copy Markdown

@zoov-xavier is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot

changeset-bot Bot commented Jun 16, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 3ae47eb

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

The active filter pill's value picker was a closed Select limited to the
field's existing values, so a filter could only be switched to a value
already present in the sampled data. Replace it with an Autocomplete that
accepts arbitrary typed input (committed on Enter or blur) while keeping
the existing suggestions.

Keyboard navigation of the dropdown is preserved: when an option is
highlighted, Enter submits that option natively; free text is only
committed when no option is highlighted. A commit guard avoids applying
the same value twice (onOptionSubmit followed by onBlur).
@zoov-xavier zoov-xavier force-pushed the claude/editable-filter-pill-free-text-value branch from 3afe05b to 828dc53 Compare June 16, 2026 14:15
@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the Mantine Select in the active-filter pill's value picker with an Autocomplete, enabling users to type any free-text value (committed on Enter or blur) in addition to choosing from the sampled-data suggestions. The scoped aria-controls DOM lookup (from previous review feedback) and the committedRef double-commit guard are correctly implemented.

  • Free-text entry: draftValue state starts empty so the full suggestion list is shown on open; commitValue trims, guards against the same value, and closes the popover on every path.
  • Keyboard navigation: onKeyDown checks the input's own listbox for a highlighted option before deciding whether Enter should commit free text or delegate to onOptionSubmit.
  • Tests: Two new cases cover keyboard-navigated selection and free-typed values; existing option-click tests are preserved.

Confidence Score: 4/5

Safe to merge for the Enter-key and pure-click flows, but the type-then-click mouse path commits the wrong value.

The onBlur handler commits draftValue before the option's click event fires, so a user who types to filter the suggestion list and then clicks a result gets their typed partial text applied as the filter instead of the selected option. The existing tests that cover option selection use fireEvent.click (which skips blur events), leaving this path untested and the defect undetected.

packages/app/src/components/ActiveFilterPills.tsx — the onBlur commit logic and the option-click tests in ActiveFilterPills.test.tsx

Important Files Changed

Filename Overview
packages/app/src/components/ActiveFilterPills.tsx Replaces Mantine Select with Autocomplete for free-text filter value entry; introduces draftValue state and committedRef guard, but has a blur-before-click ordering issue where typing to filter then clicking a suggestion commits the typed draft instead of the selected option
packages/app/src/components/tests/ActiveFilterPills.test.tsx Adds two new tests for keyboard navigation + Enter and free-typed value not in the list; existing option-click tests use fireEvent.click (no blur) rather than userEvent.click, leaving the type-then-click path untested
.changeset/filter-pill-free-text-value.md Standard changeset file marking this as a patch for @hyperdx/app; description matches the PR intent

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User
    participant Pill as FilterPill (click)
    participant Popover
    participant AC as Autocomplete
    participant commitValue

    User->>Pill: click pill
    Pill->>Popover: setOpened(true)
    Note over AC: draftValue='', committedRef=false

    alt Free-text → Enter
        User->>AC: type "418"
        AC->>AC: setDraftValue("418")
        User->>AC: press Enter (no highlighted option)
        AC->>commitValue: commitValue("418")
        commitValue->>commitValue: "committedRef=true"
        commitValue-->>Pill: onReplaceValue("418")
        commitValue->>Popover: setOpened(false)
        AC->>commitValue: onBlur → commitValue("418") [guarded]
    end

    alt Keyboard nav → Enter
        User->>AC: ArrowDown × N
        AC->>AC: highlight option
        User->>AC: press Enter
        AC->>commitValue: onOptionSubmit("404")
        commitValue->>commitValue: "committedRef=true"
        commitValue-->>Pill: onReplaceValue("404")
        commitValue->>Popover: setOpened(false)
        AC->>commitValue: onBlur → commitValue("") [guarded]
    end

    alt Type to filter then click option ⚠️
        User->>AC: "type "4" (draft="4")"
        User->>AC: mousedown on "404" option
        Note over AC: blur fires BEFORE click
        AC->>commitValue: onBlur → commitValue("4")
        commitValue->>commitValue: "committedRef=true ⚠️"
        commitValue-->>Pill: onReplaceValue("4") [wrong value]
        AC->>commitValue: onOptionSubmit("404") [blocked by ref]
    end
Loading
%%{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"}}}%%
sequenceDiagram
    participant User
    participant Pill as FilterPill (click)
    participant Popover
    participant AC as Autocomplete
    participant commitValue

    User->>Pill: click pill
    Pill->>Popover: setOpened(true)
    Note over AC: draftValue='', committedRef=false

    alt Free-text → Enter
        User->>AC: type "418"
        AC->>AC: setDraftValue("418")
        User->>AC: press Enter (no highlighted option)
        AC->>commitValue: commitValue("418")
        commitValue->>commitValue: "committedRef=true"
        commitValue-->>Pill: onReplaceValue("418")
        commitValue->>Popover: setOpened(false)
        AC->>commitValue: onBlur → commitValue("418") [guarded]
    end

    alt Keyboard nav → Enter
        User->>AC: ArrowDown × N
        AC->>AC: highlight option
        User->>AC: press Enter
        AC->>commitValue: onOptionSubmit("404")
        commitValue->>commitValue: "committedRef=true"
        commitValue-->>Pill: onReplaceValue("404")
        commitValue->>Popover: setOpened(false)
        AC->>commitValue: onBlur → commitValue("") [guarded]
    end

    alt Type to filter then click option ⚠️
        User->>AC: "type "4" (draft="4")"
        User->>AC: mousedown on "404" option
        Note over AC: blur fires BEFORE click
        AC->>commitValue: onBlur → commitValue("4")
        commitValue->>commitValue: "committedRef=true ⚠️"
        commitValue-->>Pill: onReplaceValue("4") [wrong value]
        AC->>commitValue: onOptionSubmit("404") [blocked by ref]
    end
Loading

Reviews (3): Last reviewed commit: "Merge branch 'main' into claude/editable..." | Re-trigger Greptile

Comment thread packages/app/src/components/ActiveFilterPills.tsx Outdated
Comment thread packages/app/src/components/ActiveFilterPills.tsx
Comment thread packages/app/src/components/__tests__/ActiveFilterPills.test.tsx Outdated
zoov-xavier and others added 2 commits June 16, 2026 16:32
- Scope the highlighted-option lookup to this input's own listbox (via
  aria-controls) instead of the whole document, so another open combobox
  on the page can't make us swallow Enter and drop free-text entry.
- Pin the keyboard-navigation test to the expected option ('404') instead
  of accepting any value from the list.
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Deep Review

Multi-agent review of the free-text filter-pill value picker (SelectAutocomplete). Eight reviewers ran across correctness, races, TypeScript, testing, maintainability, standards, agent-native parity, and prior learnings. One P1 blocker centered on commit-on-blur interacting with the dropdown's own action buttons.

🔴 P0/P1 — must fix

  • packages/app/src/components/ActiveFilterPills.tsx:348onBlur unconditionally runs commitValue, which always calls setOpened(false), so once the value input has focus, clicking the in-dropdown Copy or Include/Exclude ActionIcon blurs the input, closes the popover, and unmounts the button before its click fires — silently swallowing the action, and committing any typed draft as a value replacement on the way out.
    • Fix: In onBlur, skip the commit-and-close when event.relatedTarget is still inside the dropdown, or add onMouseDown={e => e.preventDefault()} to the Copy and polarity ActionIcons so focus stays on the input and their onClick fires.
    • julik-frontend-races, correctness, kieran-typescript

🟡 P2 — recommended

  • packages/app/src/components/ActiveFilterPills.tsx:338 — the Enter free-text-vs-highlighted-option branch reads Mantine's internal [data-combobox-selected] attribute via an imperative document.getElementById(aria-controls) lookup, which a Mantine markup change could silently invert.
    • Fix: Drive the highlighted-option check from Mantine's useCombobox store selection state instead of querying rendered DOM attributes.
    • maintainability, kieran-typescript
  • packages/app/src/components/ActiveFilterPills.tsx:348 — the new commit-on-blur path and the committedRef double-commit guard have no test coverage; existing option tests assert toHaveBeenCalledWith rather than call count, so a duplicate or missing commit would pass unnoticed.
    • Fix: Add tests that type a value then blur (assert one replaceFilterValue with the trimmed value) and that pick an option then blur (assert replaceFilterValue called exactly once).
    • testing, maintainability, kieran-typescript
🔵 P3 nitpicks (4)
  • packages/app/src/components/ActiveFilterPills.tsx:154 — the reset effect keyed on [pill.value, opened] clears committedRef on both open and close, coupling the guard's correctness to effect-vs-blur ordering.
    • Fix: Reset committedRef only on the open transition so the guard is ordering-independent.
  • packages/app/src/components/ActiveFilterPills.tsx:196commitValue hides an unconditional setOpened(false) side effect behind a name that reads as a pure commit.
    • Fix: Rename to commitValueAndClose or lift the close to each call site.
  • packages/app/src/components/ActiveFilterPills.tsx:197 — no coverage for whitespace-trimming, whitespace-only/empty draft, the no-op branch when the typed value equals the current value, or the placeholder text.
    • Fix: Add focused assertions for trim normalization, the equals-current no-op, and the pill.value/Loading values... placeholder.
  • packages/app/src/components/ActiveFilterPills.tsx:1 — the file is 566 lines, over the documented 300-line component cap (pre-existing; the diff grows it).
    • Fix: Extract the pill value picker into its own component in a follow-up.

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

Testing gaps:

  • Commit-on-blur (free text applied by clicking away) is untested end-to-end.
  • The committedRef double-commit guard (option pick followed by the closing blur) is not verified with a call-count assertion.
  • No test focuses the input then clicks Copy/Include/Exclude — the exact interaction behind the P1.
  • Enter-commits-free-text relies on Mantine's selectFirstOptionOnChange defaulting to false; a test typing a value absent from suggestions guards against a Mantine upgrade flipping that default.

@pulpdrew pulpdrew 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.

Just one question about a potential simplification, but I like the functionality!

commitValue(draftValue);
}
}}
onBlur={() => commitValue(draftValue)}

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.

Is this prop necessary here? I would generally not expect what I type to submit whenever clicking out of the popover, and it seems (while testing) like in that case commitValue is being called with an empty draftValue anyway.

And if we got rid of this, could we also get rid of the commitRef?

Please let me know if there is a reason this is needed that I'm overlooking!

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.

2 participants