From d38f8807948f66e638e5613f268e082b3b322b02 Mon Sep 17 00:00:00 2001 From: William Mak Date: Thu, 27 Aug 2026 15:10:41 -0400 Subject: [PATCH 01/14] feat(explore): Add search filters to equation _if aggregates Wire EAP conditional aggregates into the Explore equation builder with search-style filter autocomplete for the first argument (avg_if(`span.op:db`,span.duration)). Co-authored-by: Cursor --- .../conditionalFilter.spec.ts | 157 ++++++ .../arithmeticBuilder/conditionalFilter.ts | 339 ++++++++++++ .../conditionalFilterAutocomplete.spec.tsx | 82 +++ .../conditionalFilterAutocomplete.tsx | 178 ++++++ .../components/arithmeticBuilder/context.tsx | 5 + .../components/arithmeticBuilder/index.tsx | 8 + .../arithmeticBuilder/token/freeText.tsx | 15 +- .../arithmeticBuilder/token/function.tsx | 307 +++++++++-- .../arithmeticBuilder/token/grid.tsx | 45 +- .../arithmeticBuilder/token/index.spec.tsx | 508 +++++++++++++++--- .../arithmeticBuilder/token/styles.tsx | 24 +- .../tokenizedInput/token/comboBox.spec.tsx | 48 +- .../tokenizedInput/token/comboBox.tsx | 101 +++- static/app/utils/discover/fields.spec.tsx | 37 ++ static/app/utils/discover/fields.tsx | 18 +- .../fields/exploreEquationAggregates.spec.tsx | 95 ++++ static/app/utils/fields/index.ts | 96 ++++ .../components/exploreArithmeticBuilder.tsx | 54 +- .../exploreEquationArithmeticBuilder.tsx | 55 ++ .../toolbarVisualize/visualizeEquation.tsx | 55 +- .../hooks/useExploreEquationBuilderConfig.ts | 97 ++++ .../useGetTraceItemAttributeValues.spec.tsx | 20 + .../hooks/useGetTraceItemAttributeValues.tsx | 10 +- .../app/views/explore/queryParams/context.tsx | 15 +- .../aggregateColumnEditorModal.spec.tsx | 29 +- .../tables/aggregateColumnEditorModal.tsx | 48 +- static/app/views/explore/utils.spec.tsx | 17 + .../utils/conditionalAggregate.spec.tsx | 33 ++ .../explore/utils/conditionalAggregate.tsx | 99 ++-- 29 files changed, 2241 insertions(+), 354 deletions(-) create mode 100644 static/app/components/arithmeticBuilder/conditionalFilter.spec.ts create mode 100644 static/app/components/arithmeticBuilder/conditionalFilter.ts create mode 100644 static/app/components/arithmeticBuilder/conditionalFilterAutocomplete.spec.tsx create mode 100644 static/app/components/arithmeticBuilder/conditionalFilterAutocomplete.tsx create mode 100644 static/app/utils/fields/exploreEquationAggregates.spec.tsx create mode 100644 static/app/views/explore/components/exploreEquationArithmeticBuilder.tsx create mode 100644 static/app/views/explore/hooks/useExploreEquationBuilderConfig.ts diff --git a/static/app/components/arithmeticBuilder/conditionalFilter.spec.ts b/static/app/components/arithmeticBuilder/conditionalFilter.spec.ts new file mode 100644 index 000000000000..c905aec7eca2 --- /dev/null +++ b/static/app/components/arithmeticBuilder/conditionalFilter.spec.ts @@ -0,0 +1,157 @@ +import { + formatConditionalFilterClause, + formatConditionalFilterTagValue, + getConditionalFilterClauseAtCursor, + getConditionalFilterEditPhase, + getConditionalFilterKeyQuery, + parseConditionalFilterInput, + replaceConditionalFilterClause, +} from 'sentry/components/arithmeticBuilder/conditionalFilter'; + +describe('getConditionalFilterClauseAtCursor', () => { + it('returns the full string as one clause when there are no boolean operators', () => { + expect(getConditionalFilterClauseAtCursor('organization.slug:sentry', 10)).toEqual({ + clause: 'organization.slug:sentry', + clauseCursorIndex: 10, + clauseEnd: 24, + clauseStart: 0, + }); + }); + + it('returns the clause after a boolean operator', () => { + const value = 'organization.slug:sentry and '; + expect(getConditionalFilterClauseAtCursor(value, value.length)).toEqual({ + clause: '', + clauseCursorIndex: 0, + clauseEnd: 29, + clauseStart: 29, + }); + }); + + it('returns the active clause when typing a second filter key', () => { + const value = 'organization.slug:sentry and span.op'; + expect(getConditionalFilterClauseAtCursor(value, value.length)).toEqual({ + clause: 'span.op', + clauseCursorIndex: 7, + clauseEnd: 36, + clauseStart: 29, + }); + }); + + it('does not split on boolean operators inside quoted values', () => { + const value = 'span.description:"foo and bar" and span.op:'; + expect(getConditionalFilterClauseAtCursor(value, value.length)).toEqual({ + clause: 'span.op:', + clauseCursorIndex: 8, + clauseEnd: 43, + clauseStart: 35, + }); + }); +}); + +describe('getConditionalFilterEditPhase', () => { + it('uses key mode before the first colon in a clause', () => { + expect( + getConditionalFilterEditPhase('organization.slug:sentry and span.op', 36) + ).toBe('key'); + }); + + it('uses value mode after the colon while typing an unquoted value', () => { + expect( + getConditionalFilterEditPhase('organization.slug:sentry and span.op:db', 39) + ).toBe('value'); + }); + + it('uses key mode after a boolean operator', () => { + const value = 'organization.slug:sentry and '; + expect(getConditionalFilterEditPhase(value, value.length)).toBe('key'); + }); + + it('uses key mode after a completed unquoted value and trailing space', () => { + const value = 'organization.slug:sentry '; + expect(getConditionalFilterEditPhase(value, value.length)).toBe('key'); + }); + + it('stays in value mode for an unclosed quoted value', () => { + const value = 'organization.slug:"hello there'; + expect(getConditionalFilterEditPhase(value, value.length)).toBe('value'); + }); + + it('uses key mode after a closed quoted value', () => { + const value = 'organization.slug:"hello there"'; + expect(getConditionalFilterEditPhase(value, value.length)).toBe('key'); + }); + + it('uses key mode when typing a key after a completed value', () => { + const value = 'organization.slug:sentry span'; + expect(getConditionalFilterEditPhase(value, value.length)).toBe('key'); + }); +}); + +describe('parseConditionalFilterInput', () => { + it('parses the active clause at the cursor', () => { + expect( + parseConditionalFilterInput('organization.slug:sentry and span.op:db', 39) + ).toEqual({ + filterKey: 'span.op', + valueQuery: 'db', + }); + }); + + it('strips an opening quote from the value query', () => { + const value = 'organization.slug:"hello there'; + expect(parseConditionalFilterInput(value, value.length)).toEqual({ + filterKey: 'organization.slug', + valueQuery: 'hello there', + }); + }); +}); + +describe('getConditionalFilterKeyQuery', () => { + it('reads the key query from the active clause', () => { + expect(getConditionalFilterKeyQuery('organization.slug:sentry and span', 34)).toBe( + 'span' + ); + }); + + it('reads the key query after a completed value', () => { + const value = 'organization.slug:sentry spa'; + expect(getConditionalFilterKeyQuery(value, value.length)).toBe('spa'); + }); + + it('returns an empty key query after a completed value and space', () => { + const value = 'organization.slug:sentry '; + expect(getConditionalFilterKeyQuery(value, value.length)).toBe(''); + }); +}); + +describe('replaceConditionalFilterClause', () => { + it('replaces only the active clause when selecting a key suggestion', () => { + const value = 'organization.slug:sentry and '; + expect(replaceConditionalFilterClause(value, value.length, 'span.op:')).toEqual({ + newCursorIndex: 37, + newValue: 'organization.slug:sentry and span.op:', + }); + }); + + it('appends a key after a completed value instead of replacing it', () => { + const value = 'organization.slug:sentry '; + expect(replaceConditionalFilterClause(value, value.length, 'span.op:')).toEqual({ + newCursorIndex: 33, + newValue: 'organization.slug:sentry span.op:', + }); + }); +}); + +describe('formatConditionalFilterTagValue', () => { + it('quotes values that contain spaces', () => { + expect(formatConditionalFilterTagValue('hello there')).toBe('"hello there"'); + expect(formatConditionalFilterClause('organization.slug', 'hello there')).toBe( + 'organization.slug:"hello there"' + ); + }); + + it('leaves simple values unquoted', () => { + expect(formatConditionalFilterTagValue('sentry')).toBe('sentry'); + }); +}); diff --git a/static/app/components/arithmeticBuilder/conditionalFilter.ts b/static/app/components/arithmeticBuilder/conditionalFilter.ts new file mode 100644 index 000000000000..23865402dd46 --- /dev/null +++ b/static/app/components/arithmeticBuilder/conditionalFilter.ts @@ -0,0 +1,339 @@ +import {escapeDoubleQuotes} from 'sentry/utils'; + +/** + * EAP `_if` filter args are backtick-wrapped (`avg_if(\`span.op:db\`,…)`). Detect them + * by parameter name so the editor can show the raw query and re-wrap on commit. + */ +export function isSearchFilterParameter( + parameter: {kind?: string; name?: string} | null | undefined +): boolean { + return parameter?.kind === 'value' && parameter.name === 'filter'; +} + +/** + * Remove backticks so the filter can be safely wrapped in them. + */ +export function escapeConditionalFilter(filter: string): string { + return filter.replace(/`/g, '').trim(); +} + +/** + * Wrap a search filter for use as the first argument of an EAP `_if` aggregate. + * Empty input becomes empty backticks so the arithmetic tokenizer keeps a filter slot. + */ +export function ensureSearchFilterArgument(value: string): string { + const escaped = escapeConditionalFilter(value); + return `\`${escaped}\``; +} + +/** + * Strip outer backticks from an `_if` filter argument for display / editing. + */ +export function unwrapSearchFilterArgument(value: string): string { + const trimmed = value.trim(); + if (trimmed.length >= 2 && trimmed.startsWith('`') && trimmed.endsWith('`')) { + return trimmed.slice(1, -1); + } + return trimmed; +} + +const NEEDS_QUOTING_RE = /[\s(),\\"]/; + +/** + * Quote a tag value when it contains spaces or other special search characters. + */ +export function formatConditionalFilterTagValue(value: string): string { + if (value === '') { + return '""'; + } + if ( + (value.startsWith('"') && value.endsWith('"') && value.length >= 2) || + /^\[[^\]]*\]$/.test(value) + ) { + return value; + } + if (NEEDS_QUOTING_RE.test(value)) { + return `"${escapeDoubleQuotes(value)}"`; + } + return value; +} + +export function formatConditionalFilterClause( + filterKey: string, + tagValue: string +): string { + return `${filterKey}:${formatConditionalFilterTagValue(tagValue)}`; +} + +type BooleanOperatorMatch = { + end: number; + start: number; +}; + +function isEscaped(value: string, index: number): boolean { + let backslashes = 0; + for (let i = index - 1; i >= 0 && value[i] === '\\'; i--) { + backslashes++; + } + return backslashes % 2 === 1; +} + +function hasUnclosedQuote(value: string): boolean { + let inQuotes = false; + for (let i = 0; i < value.length; i++) { + if (value[i] === '\\' && i + 1 < value.length) { + i++; + continue; + } + if (value[i] === '"' && !isEscaped(value, i)) { + inQuotes = !inQuotes; + } + } + return inQuotes; +} + +function stripQuotesForValueSearch(value: string): string { + if (value.startsWith('"')) { + const withoutOpen = value.slice(1); + if (!hasUnclosedQuote(value) && withoutOpen.endsWith('"')) { + return withoutOpen.slice(0, -1); + } + return withoutOpen; + } + return value; +} + +function findBooleanOperators(value: string): BooleanOperatorMatch[] { + const matches: BooleanOperatorMatch[] = []; + let inQuotes = false; + let index = 0; + + while (index < value.length) { + const char = value[index]!; + + if (char === '\\' && index + 1 < value.length) { + index += 2; + continue; + } + + if (char === '"' && !isEscaped(value, index)) { + inQuotes = !inQuotes; + index++; + continue; + } + + if (!inQuotes) { + const rest = value.slice(index); + const match = rest.match(/^(\s+)(and|or)(\s+)/i); + if (match) { + matches.push({ + start: index, + end: index + match[0].length, + }); + index += match[0].length; + continue; + } + } + + index++; + } + + return matches; +} + +function getConditionalFilterClauseBounds( + value: string, + cursorIndex: number, + booleanOperators: BooleanOperatorMatch[] = findBooleanOperators(value) +): {clauseEnd: number; clauseStart: number} { + const cursor = Math.max(0, Math.min(cursorIndex, value.length)); + let clauseStart = 0; + let clauseEnd = value.length; + + for (const operator of booleanOperators) { + if (cursor > operator.end) { + clauseStart = operator.end; + continue; + } + + if (cursor >= operator.start && cursor <= operator.end) { + return { + clauseStart: operator.end, + clauseEnd: value.length, + }; + } + + if (cursor < operator.start) { + clauseEnd = operator.start; + break; + } + } + + return {clauseStart, clauseEnd}; +} + +export function getConditionalFilterClauseAtCursor( + value: string, + cursorIndex: number +): { + clause: string; + clauseCursorIndex: number; + clauseEnd: number; + clauseStart: number; +} { + const {clauseStart, clauseEnd} = getConditionalFilterClauseBounds(value, cursorIndex); + const clause = value.slice(clauseStart, clauseEnd); + return { + clause, + clauseStart, + clauseEnd, + clauseCursorIndex: Math.max(0, Math.min(cursorIndex, value.length) - clauseStart), + }; +} + +export type ConditionalFilterEditContext = { + /** Text used to filter key suggestions or ComboBox input matching. */ + editText: string; + phase: 'key' | 'value'; + replaceEnd: number; + replaceStart: number; + filterKey?: string; + /** Tag-value API search string (quotes stripped). */ + valueQuery?: string; +}; + +/** + * Decide whether the cursor is editing a filter key or value, and which substring + * a suggestion should replace. + * + * Value mode continues while the value is still open: + * - empty value after `:` + * - unquoted value with no trailing whitespace yet + * - quoted value with an unclosed `"` + * + * Once a value is complete (`key:value `, or `key:"quoted"`), subsequent text is a + * new key and key autocomplete is shown. + */ +export function getConditionalFilterEditContext( + value: string, + cursorIndex: number +): ConditionalFilterEditContext { + const {clause, clauseStart, clauseEnd, clauseCursorIndex} = + getConditionalFilterClauseAtCursor(value, cursorIndex); + + const colonIndex = clause.indexOf(':'); + if (colonIndex === -1 || clauseCursorIndex <= colonIndex) { + return { + phase: 'key', + editText: clause.slice(0, clauseCursorIndex).trim(), + replaceStart: clauseStart, + replaceEnd: clauseEnd, + }; + } + + const filterKey = clause.slice(0, colonIndex).trim(); + const valuePart = clause.slice(colonIndex + 1); + const cursorInValue = Math.max(0, clauseCursorIndex - (colonIndex + 1)); + const beforeCursor = valuePart.slice(0, cursorInValue); + + // Unclosed quotes → keep editing the value (including spaces inside the quote). + if (hasUnclosedQuote(beforeCursor)) { + return { + phase: 'value', + editText: beforeCursor, + filterKey, + valueQuery: stripQuotesForValueSearch(beforeCursor), + replaceStart: clauseStart, + replaceEnd: clauseEnd, + }; + } + + // Closed quoted value: `"hello there"` or `"hello there" nextKey` + if (beforeCursor.startsWith('"')) { + const closedQuoteMatch = beforeCursor.match(/^"(?:[^"\\]|\\.)*"(\s*)(.*)$/); + if (closedQuoteMatch) { + const [, spaces = '', nextKey = ''] = closedQuoteMatch; + const quotedValue = beforeCursor.slice( + 0, + beforeCursor.length - spaces.length - nextKey.length + ); + const keyStartInClause = colonIndex + 1 + quotedValue.length + spaces.length; + return { + phase: 'key', + editText: nextKey, + replaceStart: clauseStart + keyStartInClause, + replaceEnd: clauseEnd, + }; + } + } + + // Unquoted value: complete once whitespace follows a non-empty token. + const unquotedMatch = beforeCursor.match(/^(\S+)(\s+)(.*)$/); + if (unquotedMatch) { + const [, completedValue, spaces, nextKey = ''] = unquotedMatch; + const keyStartInClause = colonIndex + 1 + completedValue!.length + spaces!.length; + return { + phase: 'key', + editText: nextKey, + replaceStart: clauseStart + keyStartInClause, + replaceEnd: clauseEnd, + }; + } + + // Still typing an unquoted value (or empty value after `:`). + return { + phase: 'value', + editText: beforeCursor, + filterKey, + valueQuery: beforeCursor, + replaceStart: clauseStart, + replaceEnd: clauseEnd, + }; +} + +export function getConditionalFilterEditPhase( + value: string, + cursorIndex: number +): 'key' | 'value' { + return getConditionalFilterEditContext(value, cursorIndex).phase; +} + +export function parseConditionalFilterInput( + value: string, + cursorIndex: number +): { + filterKey: string; + valueQuery: string; +} | null { + const context = getConditionalFilterEditContext(value, cursorIndex); + if (context.phase !== 'value' || !context.filterKey) { + return null; + } + return { + filterKey: context.filterKey, + valueQuery: context.valueQuery ?? '', + }; +} + +export function getConditionalFilterKeyQuery(value: string, cursorIndex: number): string { + const context = getConditionalFilterEditContext(value, cursorIndex); + if (context.phase !== 'key') { + return ''; + } + return context.editText.trim(); +} + +export function replaceConditionalFilterClause( + value: string, + cursorIndex: number, + newClause: string +): {newCursorIndex: number; newValue: string} { + const {replaceStart, replaceEnd} = getConditionalFilterEditContext(value, cursorIndex); + const newValue = value.slice(0, replaceStart) + newClause + value.slice(replaceEnd); + const newCursorIndex = replaceStart + newClause.length; + return {newValue, newCursorIndex}; +} + +export function isFilterKeySuggestion(value: string): boolean { + return value.endsWith(':') && value.indexOf(':') === value.length - 1; +} diff --git a/static/app/components/arithmeticBuilder/conditionalFilterAutocomplete.spec.tsx b/static/app/components/arithmeticBuilder/conditionalFilterAutocomplete.spec.tsx new file mode 100644 index 000000000000..e7af0ae1d990 --- /dev/null +++ b/static/app/components/arithmeticBuilder/conditionalFilterAutocomplete.spec.tsx @@ -0,0 +1,82 @@ +import {renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary'; + +import {useConditionalFilterAutocomplete} from 'sentry/components/arithmeticBuilder/conditionalFilterAutocomplete'; +import {FieldKind} from 'sentry/utils/fields'; + +const functionArguments = [{name: 'span.op', kind: FieldKind.TAG, label: 'span.op'}]; + +describe('useConditionalFilterAutocomplete', () => { + it('does not fetch values with an empty filter key', async () => { + const getFilterTagValues = jest.fn().mockResolvedValue([{value: 'db'}]); + + const {rerender} = renderHookWithProviders( + ({filterValue, selectionIndex}: {filterValue: string; selectionIndex: number}) => + useConditionalFilterAutocomplete({ + enabled: true, + filterValue, + functionArguments, + getFilterTagValues, + selectionIndex, + }), + {initialProps: {filterValue: '', selectionIndex: 0}} + ); + + expect(getFilterTagValues).not.toHaveBeenCalled(); + + rerender({filterValue: 'span.op', selectionIndex: 6}); + expect(getFilterTagValues).not.toHaveBeenCalled(); + + rerender({filterValue: 'span.op:', selectionIndex: 8}); + + await waitFor(() => { + expect(getFilterTagValues).toHaveBeenCalledWith({ + tag: expect.objectContaining({key: 'span.op'}), + searchQuery: '', + }); + }); + expect(getFilterTagValues).not.toHaveBeenCalledWith( + expect.objectContaining({ + tag: expect.objectContaining({key: ''}), + }) + ); + }); + + it('shows key suggestions when the cursor is before the colon', () => { + const getFilterTagValues = jest.fn().mockResolvedValue([{value: 'db'}]); + + const {result} = renderHookWithProviders(() => + useConditionalFilterAutocomplete({ + enabled: true, + filterValue: 'span.op:db', + functionArguments, + getFilterTagValues, + selectionIndex: 3, + }) + ); + + expect(result.current.items.map(item => item.label)).toEqual(['span.op:']); + expect(getFilterTagValues).not.toHaveBeenCalled(); + }); + + it('shows value suggestions when the cursor is after the colon', async () => { + const getFilterTagValues = jest.fn().mockResolvedValue([{value: 'db'}]); + + const {result} = renderHookWithProviders(() => + useConditionalFilterAutocomplete({ + enabled: true, + filterValue: 'span.op:db', + functionArguments, + getFilterTagValues, + selectionIndex: 10, + }) + ); + + await waitFor(() => { + expect(getFilterTagValues).toHaveBeenCalled(); + }); + + await waitFor(() => { + expect(result.current.items.map(item => item.label)).toEqual(['db']); + }); + }); +}); diff --git a/static/app/components/arithmeticBuilder/conditionalFilterAutocomplete.tsx b/static/app/components/arithmeticBuilder/conditionalFilterAutocomplete.tsx new file mode 100644 index 000000000000..c0d552d021d5 --- /dev/null +++ b/static/app/components/arithmeticBuilder/conditionalFilterAutocomplete.tsx @@ -0,0 +1,178 @@ +import {useMemo} from 'react'; +import {keepPreviousData, useQuery} from '@tanstack/react-query'; + +import type {SelectOptionWithKey} from '@sentry/scraps/compactSelect'; + +import { + formatConditionalFilterClause, + getConditionalFilterEditContext, +} from 'sentry/components/arithmeticBuilder/conditionalFilter'; +import type {FunctionArgument} from 'sentry/components/arithmeticBuilder/types'; +import type {GetTagValues} from 'sentry/components/searchQueryBuilder'; +import {FieldKind} from 'sentry/utils/fields'; +import {useDebouncedValue} from 'sentry/utils/useDebouncedValue'; + +function useFilterKeyItems( + attributes: FunctionArgument[] +): Array> { + return useMemo(() => { + return attributes.map(item => { + const key = item.name; + const filterKey = `${key}:`; + return { + key, + label: filterKey, + value: filterKey, + textValue: key, + hideCheck: true, + }; + }); + }, [attributes]); +} + +function useFilterValueItems({ + enabled, + filterKey, + valueQuery, + getFilterTagValues, + tagKind, +}: { + enabled: boolean; + filterKey: string; + valueQuery: string; + getFilterTagValues?: GetTagValues; + tagKind?: FieldKind; +}): Array> { + const tag = useMemo( + () => ({ + key: filterKey, + name: filterKey, + kind: tagKind, + }), + [filterKey, tagKind] + ); + + const queryKey = useMemo( + () => ['arithmetic-filter-tag-values', tag, valueQuery] as const, + [tag, valueQuery] + ); + const debouncedQueryKey = useDebouncedValue(queryKey); + const debouncedFilterKey = debouncedQueryKey[1].key; + + const {data} = useQuery({ + queryKey: debouncedQueryKey, + queryFn: ctx => + getFilterTagValues!({ + tag: ctx.queryKey[1], + searchQuery: ctx.queryKey[2] ?? '', + }), + // Gate on the *debounced* key. `enabled` flips true as soon as the user types `:`, + // but the query key still holds the previous empty key for one debounce window — + // fetching then hits `/attributes//values/` and 404-retries. + enabled: enabled && Boolean(getFilterTagValues && debouncedFilterKey), + placeholderData: keepPreviousData, + staleTime: 30_000, + retry: false, + }); + + return useMemo(() => { + if (!data?.length) { + return []; + } + + return data.map(item => { + const tagValue = typeof item === 'string' ? item : item.value; + return { + key: `filter-value:${filterKey}:${tagValue}`, + label: tagValue, + value: formatConditionalFilterClause(filterKey, tagValue), + textValue: tagValue, + hideCheck: true, + }; + }); + }, [data, filterKey]); +} + +export function useConditionalFilterAutocomplete({ + enabled, + filterValue, + functionArguments, + getFilterTagValues, + selectionIndex, +}: { + enabled: boolean; + filterValue: string; + functionArguments: FunctionArgument[]; + selectionIndex: number; + getFilterTagValues?: GetTagValues; +}) { + const editContext = useMemo( + () => (enabled ? getConditionalFilterEditContext(filterValue, selectionIndex) : null), + [enabled, filterValue, selectionIndex] + ); + + const editPhase = editContext?.phase ?? 'key'; + const parsedFilterInput = useMemo(() => { + if (editContext?.phase !== 'value' || !editContext.filterKey) { + return null; + } + return { + filterKey: editContext.filterKey, + valueQuery: editContext.valueQuery ?? '', + }; + }, [editContext]); + + const filterKeyItems = useFilterKeyItems(functionArguments); + const filterValueItems = useFilterValueItems({ + enabled: enabled && Boolean(parsedFilterInput && getFilterTagValues), + filterKey: parsedFilterInput?.filterKey ?? '', + valueQuery: parsedFilterInput?.valueQuery ?? '', + getFilterTagValues, + tagKind: functionArguments.find( + argument => argument.name === parsedFilterInput?.filterKey + )?.kind, + }); + + const comboBoxFilterValue = useMemo(() => { + if (!editContext) { + return ''; + } + if (editContext.phase === 'value' && parsedFilterInput && getFilterTagValues) { + return parsedFilterInput.valueQuery; + } + return editContext.phase === 'key' ? editContext.editText : ''; + }, [editContext, getFilterTagValues, parsedFilterInput]); + + const items = useMemo(() => { + if (!enabled) { + return []; + } + if (editPhase === 'value' && parsedFilterInput && getFilterTagValues) { + return filterValueItems; + } + const keyQuery = (editContext?.editText ?? '').trim().toLowerCase(); + if (!keyQuery) { + return filterKeyItems; + } + return filterKeyItems.filter( + item => + item.value.toLowerCase().includes(keyQuery) || + (item.textValue?.toLowerCase().includes(keyQuery) ?? false) + ); + }, [ + editContext?.editText, + editPhase, + enabled, + filterKeyItems, + filterValueItems, + getFilterTagValues, + parsedFilterInput, + ]); + + return { + comboBoxFilterValue, + editPhase, + items, + parsedFilterInput, + }; +} diff --git a/static/app/components/arithmeticBuilder/context.tsx b/static/app/components/arithmeticBuilder/context.tsx index e915ca60f753..a5b7dffac2f4 100644 --- a/static/app/components/arithmeticBuilder/context.tsx +++ b/static/app/components/arithmeticBuilder/context.tsx @@ -6,6 +6,7 @@ import type { FocusOverride, } from 'sentry/components/arithmeticBuilder/action'; import type {FunctionArgument} from 'sentry/components/arithmeticBuilder/types'; +import type {GetTagValues} from 'sentry/components/searchQueryBuilder'; import type {FieldDefinition} from 'sentry/utils/fields'; interface ArithmeticBuilderContextData { @@ -14,6 +15,10 @@ interface ArithmeticBuilderContextData { focusOverride: FocusOverride | null; functionArguments: FunctionArgument[]; getFieldDefinition: (key: string) => FieldDefinition | null; + /** + * Fetches tag values for `_if` combinator filter arguments (e.g. after `span.op:`). + */ + getFilterTagValues?: GetTagValues; getSuggestedKey?: (key: string) => string | null; references?: Set; } diff --git a/static/app/components/arithmeticBuilder/index.tsx b/static/app/components/arithmeticBuilder/index.tsx index 1a0f70d7f8ac..7985b8aaf638 100644 --- a/static/app/components/arithmeticBuilder/index.tsx +++ b/static/app/components/arithmeticBuilder/index.tsx @@ -9,6 +9,7 @@ import {ArithmeticBuilderContext} from 'sentry/components/arithmeticBuilder/cont import type {Expression} from 'sentry/components/arithmeticBuilder/expression'; import {TokenGrid} from 'sentry/components/arithmeticBuilder/token/grid'; import type {FunctionArgument} from 'sentry/components/arithmeticBuilder/types'; +import type {GetTagValues} from 'sentry/components/searchQueryBuilder'; import type {FieldDefinition} from 'sentry/utils/fields'; import {FieldKind} from 'sentry/utils/fields'; import {PanelProvider} from 'sentry/utils/panelProvider'; @@ -21,6 +22,10 @@ interface ArithmeticBuilderProps { className?: string; 'data-test-id'?: string; disabled?: boolean; + /** + * Fetches tag values for `_if` combinator filter arguments in equations. + */ + getFilterTagValues?: GetTagValues; /** * This is used when a user types in a search key and submits the token. * The submission happens when the user types a colon or presses enter. @@ -45,6 +50,7 @@ export function ArithmeticBuilder({ aggregations, functionArguments, getFieldDefinition, + getFilterTagValues, getSuggestedKey, className, disabled, @@ -73,6 +79,7 @@ export function ArithmeticBuilder({ }), functionArguments, getFieldDefinition, + getFilterTagValues, getSuggestedKey, references, }; @@ -82,6 +89,7 @@ export function ArithmeticBuilder({ aggregations, functionArguments, getFieldDefinition, + getFilterTagValues, getSuggestedKey, references, ]); diff --git a/static/app/components/arithmeticBuilder/token/freeText.tsx b/static/app/components/arithmeticBuilder/token/freeText.tsx index 8082d876ed9f..6987f33e99b3 100644 --- a/static/app/components/arithmeticBuilder/token/freeText.tsx +++ b/static/app/components/arithmeticBuilder/token/freeText.tsx @@ -62,6 +62,8 @@ export function ArithmeticTokenFreeText({ focusable: true, }); + const isCollapsed = !token.text.trim(); + return ( - + ) { evt.stopPropagation(); } -const GridCell = styled('div')` +const GridCell = styled('div', { + shouldForwardProp: prop => prop !== 'collapsed', +})<{collapsed?: boolean}>` position: relative; display: flex; align-items: stretch; @@ -646,8 +651,10 @@ const GridCell = styled('div')` width: 100%; input { - padding: 0 ${p => p.theme.space.xs}; - min-width: 9px; + padding: 0 ${p => (p.collapsed ? 0 : p.theme.space.xs)}; + min-width: ${p => (p.collapsed ? 0 : '9px')}; width: 100%; + height: 100%; + min-height: 100%; } `; diff --git a/static/app/components/arithmeticBuilder/token/function.tsx b/static/app/components/arithmeticBuilder/token/function.tsx index 7da5083a3ff2..cd72bc465a8f 100644 --- a/static/app/components/arithmeticBuilder/token/function.tsx +++ b/static/app/components/arithmeticBuilder/token/function.tsx @@ -1,4 +1,4 @@ -import type {ChangeEvent, FocusEvent, RefObject} from 'react'; +import type {ChangeEvent, FocusEvent, MouseEvent, RefObject} from 'react'; import {useCallback, useMemo, useRef, useState} from 'react'; import {css} from '@emotion/react'; import styled from '@emotion/styled'; @@ -10,6 +10,14 @@ import type {CollectionChildren, KeyboardEvent, Node} from '@react-types/shared' import type {SelectOptionWithKey} from '@sentry/scraps/compactSelect'; import {Flex} from '@sentry/scraps/layout'; +import { + ensureSearchFilterArgument, + isFilterKeySuggestion, + isSearchFilterParameter, + replaceConditionalFilterClause, + unwrapSearchFilterArgument, +} from 'sentry/components/arithmeticBuilder/conditionalFilter'; +import {useConditionalFilterAutocomplete} from 'sentry/components/arithmeticBuilder/conditionalFilterAutocomplete'; import {useArithmeticBuilder} from 'sentry/components/arithmeticBuilder/context'; import type { Token, @@ -30,6 +38,22 @@ import {t} from 'sentry/locale'; import {defined} from 'sentry/utils/defined'; import {FieldKind, FieldValueType, prettifyTagKey} from 'sentry/utils/fields'; +function resolveArgumentDisplayLabel( + parameterDefinition: + | {defaultLabel?: string; kind?: string; name?: string} + | null + | undefined, + fallbackLabel: string +): string { + if (parameterDefinition?.kind === 'column' && parameterDefinition.defaultLabel) { + return parameterDefinition.defaultLabel; + } + if (isSearchFilterParameter(parameterDefinition)) { + return unwrapSearchFilterArgument(fallbackLabel); + } + return fallbackLabel; +} + interface ArithmeticTokenFunctionProps { item: Node; state: ListState; @@ -44,6 +68,7 @@ export function ArithmeticTokenFunction({ const functionArguments = token.attributes; const ref = useRef(null); + const skipArgumentFocusRef = useRef(false); const {rowProps, gridCellProps} = useGridListItem({ item, ref, @@ -51,6 +76,21 @@ export function ArithmeticTokenFunction({ focusable: defined(functionArguments) && functionArguments.length > 0, // if there are no arguments, it's not focusable }); + const onRowFocus = useCallback( + (evt: FocusEvent) => { + if (skipArgumentFocusRef.current) { + skipArgumentFocusRef.current = false; + return; + } + rowProps.onFocus?.(evt); + }, + [rowProps] + ); + + const onFunctionNameMouseDown = useCallback(() => { + skipArgumentFocusRef.current = true; + }, []); + const isFocused = item.key === state.selectionManager.focusedKey; const attrText = functionArguments.map(arg => arg.attribute).join(','); @@ -58,13 +98,16 @@ export function ArithmeticTokenFunction({ return ( - {token.function} + + {token.function} + @@ -92,10 +135,7 @@ function ArgumentsGrid({ const fieldDefinition = getFieldDefinition(functionToken.function)?.parameters?.[ index ]; - if (fieldDefinition?.kind === 'column') { - return fieldDefinition?.defaultLabel ?? fallbackLabel; - } - return fallbackLabel; + return resolveArgumentDisplayLabel(fieldDefinition, fallbackLabel); }, [getFieldDefinition, functionToken] ); @@ -251,6 +291,7 @@ function InternalInput({ argumentsListState, argumentItem, argument, + argumentRef, arguments: functionArguments, onArgumentsChange, }: InternalInputProps) { @@ -271,6 +312,7 @@ function InternalInput({ dispatch, functionArguments: builderFunctionArguments, getFieldDefinition, + getFilterTagValues, getSuggestedKey, } = useArithmeticBuilder(); @@ -281,9 +323,7 @@ function InternalInput({ const resolveDisplayLabel = useCallback( (fallback: string): string => - parameterDefinition?.kind === 'column' && parameterDefinition.defaultLabel - ? parameterDefinition.defaultLabel - : fallback, + resolveArgumentDisplayLabel(parameterDefinition, fallback), [parameterDefinition] ); @@ -292,15 +332,34 @@ function InternalInput({ const [inputValue, setInputValue] = useState(''); const [currentValue, setCurrentValue] = useState(initialLabel); const [isCurrentlyEditing, setIsCurrentlyEditing] = useState(false); - const [_selectionIndex, setSelectionIndex] = useState(0); // TODO - const [_isOpen, setIsOpen] = useState(false); // TODO + const [selectionIndex, setSelectionIndex] = useState(0); + + const isFilterParameter = isSearchFilterParameter(parameterDefinition); - const filterValue = inputValue.trim(); const displayValue = isCurrentlyEditing ? inputValue : currentValue; - const updateSelectionIndex = useCallback(() => { - setSelectionIndex(inputRef.current?.selectionStart ?? 0); - }, [setSelectionIndex]); + const { + comboBoxFilterValue, + editPhase, + items: filterItems, + } = useConditionalFilterAutocomplete({ + enabled: isFilterParameter && isCurrentlyEditing, + filterValue: inputValue, + functionArguments: builderFunctionArguments, + getFilterTagValues, + selectionIndex, + }); + + const shouldFilterComboBoxResults = !( + isFilterParameter && + editPhase === 'value' && + getFilterTagValues + ); + + const updateSelectionIndex = useCallback((input?: HTMLInputElement | null) => { + const target = input ?? inputRef.current; + setSelectionIndex(target?.selectionStart ?? 0); + }, []); const resetInputValue = useCallback(() => { setInputValue(''); @@ -348,6 +407,12 @@ function InternalInput({ const attributeItems = useAttributeItems(allowedAttributes); const items = useMemo(() => { + if (isFilterParameter) { + return filterItems; + } + + const filterValue = inputValue.trim(); + if (parameterDefinition?.kind === 'value' && parameterDefinition.options) { return parameterDefinition.options .filter( @@ -393,13 +458,23 @@ function InternalInput({ } return result; - }, [parameterDefinition, filterValue, attributeItems]); + }, [attributeItems, filterItems, inputValue, isFilterParameter, parameterDefinition]); const shouldCloseOnInteractOutside = useCallback((el: Element) => { return !gridCellRef.current?.contains(el); }, []); - const onClick = useCallback(() => { + const onClick = useCallback( + (evt: MouseEvent) => { + const input = evt.currentTarget; + requestAnimationFrame(() => { + updateSelectionIndex(input); + }); + }, + [updateSelectionIndex] + ); + + const onKeyUp = useCallback(() => { updateSelectionIndex(); }, [updateSelectionIndex]); @@ -410,6 +485,9 @@ function InternalInput({ const resolveValue = useCallback( (raw: string): string => { + if (isSearchFilterParameter(parameterDefinition)) { + return ensureSearchFilterArgument(raw); + } if ( parameterDefinition?.kind === 'column' && parameterDefinition.defaultLabel && @@ -423,34 +501,100 @@ function InternalInput({ [parameterDefinition] ); - const onTextInputBlur = useCallback(() => { - if (inputValue) { - onArgumentsChange(argumentIndex, inputValue); - dispatch({ - text: `${functionToken.function}(${updateAttrsWith(inputValue)})`, - type: 'REPLACE_TOKEN', - token: functionToken, - focusOverride: { - itemKey: nextTokenKeyOfKind( - functionListState, - functionToken, - TokenKind.FREE_TEXT - ), - }, - }); - } - resetInputValue(); - setIsCurrentlyEditing(false); - }, [ - argumentIndex, - dispatch, - functionListState, - functionToken, - inputValue, - onArgumentsChange, - resetInputValue, - updateAttrsWith, - ]); + // Persist free-text filter edits on blur. Skip REPLACE_TOKEN while focus stays inside + // the arguments grid — that remounts the function and steals focus from the next arg. + const onFilterInputBlur = useCallback( + (evt?: FocusEvent) => { + const value = inputValue ? ensureSearchFilterArgument(inputValue) : null; + if (value) { + setCurrentValue(unwrapSearchFilterArgument(value)); + onArgumentsChange(argumentIndex, value); + } + resetInputValue(); + setIsCurrentlyEditing(false); + + if (!value || value === functionToken.attributes[argumentIndex]?.text) { + return; + } + + const commitFilter = () => { + dispatch({ + text: `${functionToken.function}(${updateAttrsWith(value)})`, + type: 'REPLACE_TOKEN', + token: functionToken, + }); + }; + + if (evt) { + const argsGrid = evt.currentTarget.closest('[role="grid"]'); + const related = evt.relatedTarget; + const stayingInArgs = Boolean( + argsGrid && related instanceof Node && argsGrid.contains(related) + ); + if (!stayingInArgs) { + commitFilter(); + } + return; + } + + // Click-outside closes the menu without a focus event; check after focus settles. + window.setTimeout(() => { + const stayingInArgs = Boolean( + document.activeElement && argumentRef.current?.contains(document.activeElement) + ); + if (!stayingInArgs) { + commitFilter(); + } + }, 0); + }, + [ + argumentIndex, + argumentRef, + dispatch, + functionToken, + inputValue, + onArgumentsChange, + resetInputValue, + updateAttrsWith, + ] + ); + + // Non-filter free-text values (e.g. apdex threshold) still use InputBox and can use the + // relatedTarget check to REPLACE only when leaving the arguments grid. + const onTextInputBlur = useCallback( + (evt: FocusEvent) => { + if (inputValue) { + onArgumentsChange(argumentIndex, inputValue); + + const argsGrid = evt.currentTarget.closest('[role="grid"]'); + const related = evt.relatedTarget; + const stayingInArgs = Boolean( + related instanceof Node && argsGrid?.contains(related) + ); + if ( + !stayingInArgs && + inputValue !== functionToken.attributes[argumentIndex]?.text + ) { + dispatch({ + text: `${functionToken.function}(${updateAttrsWith(inputValue)})`, + type: 'REPLACE_TOKEN', + token: functionToken, + }); + } + } + resetInputValue(); + setIsCurrentlyEditing(false); + }, + [ + argumentIndex, + dispatch, + functionToken, + inputValue, + onArgumentsChange, + resetInputValue, + updateAttrsWith, + ] + ); const onInputChange = useCallback( (evt: ChangeEvent) => { @@ -523,6 +667,19 @@ function InternalInput({ [argumentItem.key, argumentsListState, resetInputValue] ); + // Free-text value args (e.g. `_if` filters) should keep their current text on focus so + // the user can edit it. ComboBox clears on focus to type a new filter query. + const onTextInputFocus = useCallback( + (evt: FocusEvent) => { + evt.stopPropagation(); + focusTarget(argumentsListState, argumentItem.key); + setIsCurrentlyEditing(true); + setInputValue(currentValue); + updateSelectionIndex(evt.currentTarget); + }, + [argumentItem.key, argumentsListState, currentValue, updateSelectionIndex] + ); + const onKeyDownCapture = useCallback( (evt: React.KeyboardEvent) => { // At start and pressing left arrow, focus the previous full token @@ -612,6 +769,30 @@ function InternalInput({ const onOptionSelected = useCallback( (option: SelectOptionWithKey) => { + if (isFilterParameter) { + const {newValue, newCursorIndex} = replaceConditionalFilterClause( + inputValue, + selectionIndex, + option.value + ); + setCurrentValue(newValue); + setInputValue(newValue); + setIsCurrentlyEditing(true); + setSelectionIndex(newCursorIndex); + + if (isFilterKeySuggestion(option.value)) { + requestAnimationFrame(() => { + const input = inputRef.current; + if (!input) { + return; + } + input.setSelectionRange(newCursorIndex, newCursorIndex); + input.focus(); + }); + } + return; + } + setCurrentValue(resolveDisplayLabel(prettifyTagKey(option.value))); if (hasNextArgument) { focusTarget( @@ -636,6 +817,7 @@ function InternalInput({ resetInputValue(); }, [ + isFilterParameter, hasNextArgument, resolveDisplayLabel, resetInputValue, @@ -647,6 +829,8 @@ function InternalInput({ functionToken, updateAttrsWith, functionListState, + inputValue, + selectionIndex, ] ); @@ -654,8 +838,11 @@ function InternalInput({ // TODO }, []); + // Free-text value args with no options (e.g. apdex threshold) use a plain input. + // `_if` filter args use ComboBox below for attribute-key autocomplete. if ( parameterDefinition?.kind === 'value' && + !isFilterParameter && (!defined(parameterDefinition.options) || !parameterDefinition.options.length) ) { return ( @@ -671,11 +858,10 @@ function InternalInput({ onInputChange={onInputChange} onInputCommit={onInputCommit} onInputEscape={onInputEscape} - onInputFocus={onInputFocus} + onInputFocus={onTextInputFocus} onKeyDown={onKeyDown} onKeyDownCapture={onKeyDownCapture} /> - {argumentIndex < functionToken.attributes.length - 1 && ','} ); @@ -688,30 +874,37 @@ function InternalInput({ items={items} ref={inputRef} placeholder={ - parameterDefinition?.kind === 'value' && 'placeholder' in parameterDefinition - ? (argument.label ?? parameterDefinition.placeholder) - : resolveDisplayLabel(argument.label) + isFilterParameter + ? resolveDisplayLabel(argument.label) + : parameterDefinition?.kind === 'value' && + 'placeholder' in parameterDefinition + ? (argument.label ?? parameterDefinition.placeholder) + : resolveDisplayLabel(argument.label) } inputLabel={ - parameterDefinition?.kind === 'column' - ? t('Select an attribute') - : t('Select an option') + isFilterParameter + ? t('Add a filter') + : parameterDefinition?.kind === 'column' + ? t('Select an attribute') + : t('Select an option') } inputValue={displayValue} - filterValue={filterValue} + filterValue={comboBoxFilterValue} + keepMenuOpenOnSelect={option => isFilterKeySuggestion(option.value)} + shouldFilterResults={shouldFilterComboBoxResults} tabIndex={ argumentItem.key === argumentsListState.selectionManager.focusedKey ? 0 : -1 } shouldCloseOnInteractOutside={shouldCloseOnInteractOutside} onClick={onClick} - onInputBlur={onInputBlur} + onInputBlur={isFilterParameter ? onFilterInputBlur : onInputBlur} onInputChange={onInputChange} onInputCommit={onInputCommit} onInputEscape={onInputEscape} - onInputFocus={onInputFocus} + onInputFocus={isFilterParameter ? onTextInputFocus : onInputFocus} onKeyDown={onKeyDown} onKeyDownCapture={onKeyDownCapture} - onOpenChange={setIsOpen} + onKeyUp={isFilterParameter ? onKeyUp : undefined} onOptionSelected={onOptionSelected} onPaste={onPaste} data-test-id={ diff --git a/static/app/components/arithmeticBuilder/token/grid.tsx b/static/app/components/arithmeticBuilder/token/grid.tsx index b679d67c727c..32dfd0a8ef12 100644 --- a/static/app/components/arithmeticBuilder/token/grid.tsx +++ b/static/app/components/arithmeticBuilder/token/grid.tsx @@ -1,4 +1,4 @@ -import {useLayoutEffect, useMemo, useRef} from 'react'; +import {useCallback, useLayoutEffect, useMemo, useRef, type PointerEvent} from 'react'; import styled from '@emotion/styled'; import type {AriaGridListOptions} from '@react-aria/gridlist'; import {Item} from '@react-stately/collections'; @@ -25,6 +25,8 @@ import {ArithmeticTokenParenthesis} from 'sentry/components/arithmeticBuilder/to import {ArithmeticBuilderTokenReference} from 'sentry/components/arithmeticBuilder/token/reference'; import {computeNextAllowedTokenKinds} from 'sentry/components/arithmeticBuilder/validator'; import {useGridList} from 'sentry/components/tokenizedInput/grid/useGridList'; +import {focusTarget} from 'sentry/components/tokenizedInput/grid/utils'; +import {shiftFocusToChild} from 'sentry/components/tokenizedInput/token/utils'; import {t} from 'sentry/locale'; import {defined} from 'sentry/utils/defined'; @@ -107,13 +109,46 @@ function GridList({showPlaceholder, ...props}: GridListProps) { useApplyFocusOverride(state); + const onGridPaddingPointerDown = useCallback( + (evt: PointerEvent) => { + if (evt.target !== evt.currentTarget) { + gridProps.onPointerDown?.(evt); + return; + } + + // Padding clicks would otherwise focus the grid itself, which has no caret. + evt.preventDefault(); + + const rect = evt.currentTarget.getBoundingClientRect(); + const closerToStart = evt.clientY < rect.top + rect.height / 2; + const key = closerToStart + ? state.collection.getFirstKey() + : state.collection.getLastKey(); + if (!key) { + return; + } + + focusTarget(state, key); + + const item = state.collection.getItem(key); + const rows = Array.from( + evt.currentTarget.querySelectorAll('[role="row"]') + ).filter(row => row.closest('[role="grid"]') === evt.currentTarget); + const row = closerToStart ? rows.at(0) : rows.at(-1); + if (row && item) { + shiftFocusToChild(row, item, state); + } + }, + [gridProps, state] + ); + const nextAllowedTokenKindsAtIndex = useMemo(() => { const tokens = Array.from(state.collection, item => item.value); return computeNextAllowedTokenKinds(tokens); }, [state.collection]); return ( - + {Array.from(state.collection, (item, i) => { const token = item.value; @@ -197,11 +232,15 @@ function GridList({showPlaceholder, ...props}: GridListProps) { } const TokenGridWrapper = styled('div')` - padding: ${p => p.theme.space.sm}; + box-sizing: border-box; + width: 100%; + min-height: 100%; + padding: ${p => p.theme.space.lg} ${p => p.theme.space.sm}; display: flex; align-items: center; row-gap: ${p => p.theme.space.xs}; flex-wrap: wrap; + cursor: text; &:focus { outline: none; diff --git a/static/app/components/arithmeticBuilder/token/index.spec.tsx b/static/app/components/arithmeticBuilder/token/index.spec.tsx index c77368bfaf72..7c0a86da6852 100644 --- a/static/app/components/arithmeticBuilder/token/index.spec.tsx +++ b/static/app/components/arithmeticBuilder/token/index.spec.tsx @@ -2,6 +2,7 @@ import type {Dispatch} from 'react'; import {useCallback} from 'react'; import { + fireEvent, render, screen, userEvent, @@ -18,7 +19,8 @@ import { TokenKind, } from 'sentry/components/arithmeticBuilder/token'; import {TokenGrid} from 'sentry/components/arithmeticBuilder/token/grid'; -import {FieldKind, getFieldDefinition} from 'sentry/utils/fields'; +import type {GetTagValues} from 'sentry/components/searchQueryBuilder'; +import {FieldKind, getExploreEquationFieldDefinition} from 'sentry/utils/fields'; const aggregations = ['avg', 'avg_if', 'sum', 'epm', 'count', 'count_unique', 'count_if']; @@ -34,7 +36,7 @@ const getSpanFieldDefinition = (key: string) => { functionArgument => functionArgument.name === key ); - return getFieldDefinition(key, 'span', argument?.kind); + return getExploreEquationFieldDefinition(key, argument?.kind, true); }; const getSuggestedKey = (key: string) => { @@ -52,6 +54,7 @@ const getSuggestedKey = (key: string) => { interface TokensProp { expression: string; dispatch?: Dispatch; + getFilterTagValues?: GetTagValues; references?: Set; } @@ -77,6 +80,7 @@ function Tokens(props: TokensProp) { aggregations, functionArguments, getFieldDefinition: getSpanFieldDefinition, + getFilterTagValues: props.getFilterTagValues, getSuggestedKey, references: props.references, }} @@ -95,6 +99,13 @@ function getLastInput() { } describe('token', () => { + it('focuses the last input when clicking empty space in the field', async () => { + render(); + + await userEvent.click(screen.getByRole('grid', {name: 'Enter an equation'})); + + expect(getLastInput()).toHaveFocus(); + }); describe('ArithmeticTokenFreeText', () => { it('renders default place holder', async () => { render(); @@ -171,7 +182,7 @@ describe('token', () => { expect( await screen.findByRole('row', { - name: 'avg_if(span.duration,span.op,equals,db)', + name: 'avg_if(``,span.duration)', }) ).toBeInTheDocument(); }); @@ -804,12 +815,338 @@ describe('token', () => { }); }); + it('keeps filter text when focusing an _if filter argument', async () => { + render(); + + expect( + await screen.findByRole('row', { + name: 'avg_if(`span.op:db`,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + expect(filterArg).toHaveValue('span.op:db'); + + await userEvent.click(filterArg); + expect(filterArg).toHaveValue('span.op:db'); + }); + + it('autocompletes attribute keys in an _if filter argument', async () => { + render(); + + expect( + await screen.findByRole('row', { + name: 'avg_if(``,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + + await userEvent.click(filterArg); + await userEvent.type(filterArg, 'span.op'); + expect(screen.getByRole('option', {name: 'span.op:'})).toBeInTheDocument(); + await userEvent.click(screen.getByRole('option', {name: 'span.op:'})); + + expect(filterArg).toHaveValue('span.op:'); + expect(filterArg).toHaveFocus(); + }); + + it('switches to value suggestions after selecting a filter key', async () => { + const getFilterTagValues = jest.fn().mockResolvedValue([{value: 'db'}]); + + render( + + ); + + expect( + await screen.findByRole('row', { + name: 'avg_if(``,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + + await userEvent.click(filterArg); + await userEvent.type(filterArg, 'span.op'); + await userEvent.click(screen.getByRole('option', {name: 'span.op:'})); + + expect(filterArg).toHaveValue('span.op:'); + await waitFor(() => { + expect(getFilterTagValues).toHaveBeenCalled(); + }); + expect(await screen.findByRole('option', {name: 'db'})).toBeInTheDocument(); + }); + + it('autocompletes tag values in an _if filter argument', async () => { + const getFilterTagValues = jest.fn(({tag, searchQuery}) => { + if (tag.key === 'span.op') { + return Promise.resolve( + [{value: 'db'}, {value: 'http'}].filter( + item => !searchQuery || item.value.includes(searchQuery) + ) + ); + } + return Promise.resolve([]); + }); + + render( + + ); + + expect( + await screen.findByRole('row', { + name: 'avg_if(``,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + + await userEvent.click(filterArg); + await userEvent.type(filterArg, 'span.op:'); + + await waitFor(() => { + expect(getFilterTagValues).toHaveBeenCalled(); + }); + expect(await screen.findByRole('option', {name: 'db'})).toBeInTheDocument(); + await userEvent.click(screen.getByRole('option', {name: 'db'})); + + expect(filterArg).toHaveValue('span.op:db'); + expect(filterArg).toHaveFocus(); + await waitFor(() => { + expect(screen.queryByRole('option')).not.toBeInTheDocument(); + }); + }); + + it('continues value autocomplete inside an unclosed quoted value', async () => { + const getFilterTagValues = jest.fn(({searchQuery}) => { + return Promise.resolve( + [{value: 'hello world'}, {value: 'hello there'}].filter( + item => !searchQuery || item.value.includes(searchQuery) + ) + ); + }); + + render( + + ); + + expect( + await screen.findByRole('row', { + name: 'avg_if(``,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + + await userEvent.click(filterArg); + await userEvent.type(filterArg, 'span.description:"hello '); + + await waitFor(() => { + expect(getFilterTagValues).toHaveBeenCalledWith( + expect.objectContaining({searchQuery: 'hello '}) + ); + }); + expect( + await screen.findByRole('option', {name: 'hello world'}) + ).toBeInTheDocument(); + await userEvent.click(screen.getByRole('option', {name: 'hello world'})); + expect(filterArg).toHaveValue('span.description:"hello world"'); + }); + + it('shows key autocomplete after a completed value and trailing space', async () => { + const getFilterTagValues = jest.fn().mockResolvedValue([{value: 'db'}]); + + render( + + ); + + expect( + await screen.findByRole('row', { + name: 'avg_if(``,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + + await userEvent.click(filterArg); + await userEvent.type(filterArg, 'span.op:db '); + + expect( + await screen.findByRole('option', {name: 'span.description:'}) + ).toBeInTheDocument(); + expect(screen.queryByRole('option', {name: 'db'})).not.toBeInTheDocument(); + await userEvent.click(screen.getByRole('option', {name: 'span.description:'})); + expect(filterArg).toHaveValue('span.op:db span.description:'); + }); + + it('autocompletes keys after a boolean operator in a compound filter', async () => { + render(); + + expect( + await screen.findByRole('row', { + name: 'avg_if(``,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + + await userEvent.click(filterArg); + await userEvent.type(filterArg, 'span.op:db and '); + + expect( + await screen.findByRole('option', {name: 'span.description:'}) + ).toBeInTheDocument(); + await userEvent.click(screen.getByRole('option', {name: 'span.description:'})); + + expect(filterArg).toHaveValue('span.op:db and span.description:'); + }); + + it('does not show a values dropdown when there are no matching values', async () => { + const getFilterTagValues = jest.fn().mockResolvedValue([]); + + render( + + ); + + expect( + await screen.findByRole('row', { + name: 'avg_if(``,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + + await userEvent.click(filterArg); + await userEvent.type(filterArg, 'span.description'); + expect(screen.getByRole('option', {name: 'span.description:'})).toBeInTheDocument(); + + await userEvent.type(filterArg, ':'); + await waitFor(() => { + expect(getFilterTagValues).toHaveBeenCalled(); + expect(screen.queryByRole('option')).not.toBeInTheDocument(); + }); + expect(screen.queryByText('No options found')).not.toBeInTheDocument(); + }); + + it('shows filter key suggestions when editing an existing _if filter', async () => { + const getFilterTagValues = jest.fn().mockResolvedValue([{value: 'db'}]); + + render( + + ); + + expect( + await screen.findByRole('row', { + name: 'avg_if(`span.op:db`,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + + await userEvent.click(filterArg); + filterArg.setSelectionRange(0, 0); + fireEvent.keyUp(filterArg, {key: 'ArrowLeft', code: 'ArrowLeft'}); + await waitFor(() => { + expect(screen.getByRole('option', {name: 'span.op:'})).toBeInTheDocument(); + }); + expect(screen.queryByRole('option', {name: 'db'})).not.toBeInTheDocument(); + }); + + it('shows filter value suggestions when the cursor is after the colon', async () => { + const getFilterTagValues = jest.fn().mockResolvedValue([{value: 'db'}]); + + render( + + ); + + expect( + await screen.findByRole('row', { + name: 'avg_if(`span.op:db`,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + + await userEvent.click(filterArg); + filterArg.setSelectionRange(9, 9); + fireEvent.keyUp(filterArg, {key: 'ArrowLeft', code: 'ArrowLeft'}); + + await waitFor(() => { + expect(getFilterTagValues).toHaveBeenCalled(); + }); + expect(await screen.findByRole('option', {name: 'db'})).toBeInTheDocument(); + expect(screen.queryByRole('option', {name: 'span.op:'})).not.toBeInTheDocument(); + }); + + it('does not open filter autocomplete when clicking the function name', async () => { + const getFilterTagValues = jest.fn().mockResolvedValue([{value: 'db'}]); + + render( + + ); + + const functionRow = await screen.findByRole('row', { + name: 'avg_if(`span.op:db`,span.duration)', + }); + + await userEvent.click(within(functionRow).getByText('avg_if')); + + expect( + within(functionRow).getByRole('combobox', {name: 'Add a filter'}) + ).not.toHaveFocus(); + expect(screen.queryByRole('option', {name: 'span.op:'})).not.toBeInTheDocument(); + }); + it('renders multi-argument function and allows navigating between arguments', async () => { - render(); + render(); expect( await screen.findByRole('row', { - name: 'count_if(span.op,equals,browser)', + name: 'avg_if(`span.op:db`,span.duration)', }) ).toBeInTheDocument(); @@ -817,134 +1154,137 @@ describe('token', () => { screen.getByRole('grid', {name: 'Enter arguments'}) ).queryAllByRole('gridcell'); - expect(args).toHaveLength(3); + expect(args).toHaveLength(2); - const firstArg = within( + const filterArg = within( + screen.getByRole('grid', {name: 'Enter arguments'}) + ).getByRole('combobox', {name: 'Add a filter'}); + // Filter args are shown without backticks; wrapping is applied on commit. + expect(filterArg).toHaveValue('span.op:db'); + + const columnArg = within( screen.getByRole('grid', {name: 'Enter arguments'}) ).getByRole('combobox', {name: 'Select an attribute'}); + expect(columnArg).toHaveAttribute('placeholder', 'span.duration'); - expect(firstArg).toHaveAttribute('placeholder', 'span.op'); + await userEvent.click(columnArg); + await userEvent.type(columnArg, 'span.self_time'); + expect(screen.getByRole('option', {name: 'span.self_time'})).toBeInTheDocument(); + await userEvent.click(screen.getByRole('option', {name: 'span.self_time'})); - const secondArg = within( - screen.getByRole('grid', {name: 'Enter arguments'}) - ).getByRole('combobox', {name: 'Select an option'}); - expect(secondArg).toHaveAttribute('placeholder', 'equals'); + await waitFor(() => { + expect(getLastInput()).toHaveFocus(); + }); + expect( + screen.queryByRole('option', {name: 'span.self_time'}) + ).not.toBeInTheDocument(); + }); + + it('commits _if filter on blur when leaving the equation', async () => { + const dispatch = jest.fn(); + render(); - const thirdArg = within( + expect( + await screen.findByRole('row', { + name: 'avg_if(``,span.duration)', + }) + ).toBeInTheDocument(); + + const filterArg = within( screen.getByRole('grid', {name: 'Enter arguments'}) - ).getByRole('textbox', {name: 'Add a value'}); - expect(thirdArg).toHaveValue('browser'); + ).getByRole('combobox', {name: 'Add a filter'}); - await userEvent.click(firstArg); - await userEvent.type(firstArg, 'span.description'); - expect(screen.getByRole('option', {name: 'span.description'})).toBeInTheDocument(); - await userEvent.click(screen.getByRole('option', {name: 'span.description'})); + await userEvent.click(filterArg); + await userEvent.type(filterArg, 'span.op:db'); + await userEvent.click(getLastInput()); await waitFor(() => { - expect(secondArg).toHaveFocus(); + expect(dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'REPLACE_TOKEN', + text: 'avg_if(`span.op:db`,span.duration)', + }) + ); }); + }); - expect( - screen.queryByRole('option', {name: 'span.description'}) - ).not.toBeInTheDocument(); + it('does not rewrite the function when moving from filter to another argument', async () => { + const dispatch = jest.fn(); + render(); - await userEvent.keyboard('not'); - await userEvent.click(screen.getByRole('option', {name: 'is not equal to'})); + const argsGrid = await screen.findByRole('grid', {name: 'Enter arguments'}); + const filterArg = within(argsGrid).getByRole('combobox', {name: 'Add a filter'}); + const columnArg = within(argsGrid).getByRole('combobox', { + name: 'Select an attribute', + }); + + await userEvent.click(filterArg); + await userEvent.type(filterArg, 'span.op:db'); + await userEvent.click(columnArg); await waitFor(() => { - expect(thirdArg).toHaveFocus(); + expect(columnArg).toHaveFocus(); }); - await userEvent.keyboard('db'); - expect(thirdArg).toHaveValue('db'); + expect(dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({type: 'REPLACE_TOKEN'}) + ); }); it('suggests attributes for each argument of avg_if', async () => { - render(); + render(); const argumentsGrid = await screen.findByRole('grid', {name: 'Enter arguments'}); - const [numberArg, stringArg] = within(argumentsGrid).getAllByRole('combobox', { - name: 'Select an attribute', + const filterArg = within(argumentsGrid).getByRole('combobox', { + name: 'Add a filter', }); - const conditionArg = within(argumentsGrid).getByRole('combobox', { - name: 'Select an option', - }); - const valueArg = within(argumentsGrid).getByRole('textbox', { - name: 'Add a value', + const columnArg = within(argumentsGrid).getByRole('combobox', { + name: 'Select an attribute', }); - await userEvent.click(numberArg!); + expect(filterArg).toHaveValue('span.op:db'); + + await userEvent.click(columnArg); expect(screen.getAllByRole('option').map(option => option.textContent)).toEqual([ 'span.duration', 'span.self_time', ]); - - await userEvent.click(stringArg!); - expect(screen.getAllByRole('option').map(option => option.textContent)).toEqual([ - 'span.op', - 'span.description', - ]); - - await userEvent.click(conditionArg); - expect(screen.getAllByRole('option').map(option => option.textContent)).toEqual([ - 'is equal to', - 'is not equal to', - ]); - - expect(valueArg).toHaveValue('queue.process'); }); }); it('shifts focus between args correctly', async () => { - render(); + render(); expect( await screen.findByRole('row', { - name: 'count_if(span.op,equals,browser)', + name: 'avg_if(`span.op:db`,span.duration)', }) ).toBeInTheDocument(); - const args = within( - screen.getByRole('grid', {name: 'Enter arguments'}) - ).queryAllByRole('gridcell'); - - expect(args).toHaveLength(3); - - const firstArg = within( - screen.getByRole('grid', {name: 'Enter arguments'}) - ).getByRole('combobox', {name: 'Select an attribute'}); - - const secondArg = within( - screen.getByRole('grid', {name: 'Enter arguments'}) - ).getByRole('combobox', {name: 'Select an option'}); + const argsGrid = screen.getByRole('grid', {name: 'Enter arguments'}); + expect(within(argsGrid).queryAllByRole('gridcell')).toHaveLength(2); - await userEvent.click(firstArg); - await userEvent.type(firstArg, 'span.description'); - expect(screen.getByRole('option', {name: 'span.description'})).toBeInTheDocument(); - await userEvent.click(screen.getByRole('option', {name: 'span.description'})); + const filterArg = within(argsGrid).getByRole('combobox', { + name: 'Add a filter', + }); + const columnArg = within(argsGrid).getByRole('combobox', { + name: 'Select an attribute', + }); + await userEvent.click(filterArg); await waitFor(() => { - expect(secondArg).toHaveFocus(); + expect(filterArg).toHaveFocus(); }); - expect( - screen.queryByRole('option', {name: 'span.description'}) - ).not.toBeInTheDocument(); - - await userEvent.click(firstArg); - + await userEvent.click(columnArg); await waitFor(() => { - expect(firstArg).toHaveFocus(); + expect(columnArg).toHaveFocus(); }); - await userEvent.type(firstArg, 'span.op'); - expect(screen.getByRole('option', {name: 'span.op'})).toBeInTheDocument(); - await userEvent.click(screen.getByRole('option', {name: 'span.op'})); + await userEvent.click(filterArg); await waitFor(() => { - expect(secondArg).toHaveFocus(); + expect(filterArg).toHaveFocus(); }); - - expect(screen.queryByRole('option', {name: 'span.op'})).not.toBeInTheDocument(); }); describe('ArithmeticTokenLiteral', () => { diff --git a/static/app/components/arithmeticBuilder/token/styles.tsx b/static/app/components/arithmeticBuilder/token/styles.tsx index 0de75c4ddffb..05a471ff8c71 100644 --- a/static/app/components/arithmeticBuilder/token/styles.tsx +++ b/static/app/components/arithmeticBuilder/token/styles.tsx @@ -1,7 +1,9 @@ import {css} from '@emotion/react'; import styled from '@emotion/styled'; -export const Row = styled('div')<{withBorder?: boolean}>` +export const Row = styled('div', { + shouldForwardProp: prop => prop !== 'collapsed' && prop !== 'withBorder', +})<{collapsed?: boolean; withBorder?: boolean}>` position: relative; display: flex; align-items: stretch; @@ -15,8 +17,28 @@ export const Row = styled('div')<{withBorder?: boolean}>` border-radius: ${p.theme.radius.md}; `} + /* Empty spacers must not consume a flex line. A wide _if function is + max-width 100%, so even a few pixels of leading/trailing free text wrap + onto their own row. Last-child still grows to fill leftover space. */ + ${p => + p.collapsed && + css` + width: 0; + min-width: 0; + flex-grow: 0; + flex-shrink: 0; + flex-basis: 0; + overflow: visible; + `} + &:last-child { flex-grow: 1; + min-width: 0; + max-width: none; + flex-basis: 0; + align-self: stretch; + height: auto; + min-height: 24px; } &[aria-invalid='true'] { diff --git a/static/app/components/tokenizedInput/token/comboBox.spec.tsx b/static/app/components/tokenizedInput/token/comboBox.spec.tsx index e8040bfeb7a9..1a7dafb154a0 100644 --- a/static/app/components/tokenizedInput/token/comboBox.spec.tsx +++ b/static/app/components/tokenizedInput/token/comboBox.spec.tsx @@ -1,7 +1,7 @@ import type {ComponentProps} from 'react'; import {Item} from '@react-stately/collections'; -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; import {ComboBox} from 'sentry/components/tokenizedInput/token/comboBox'; @@ -65,6 +65,8 @@ describe('ComboBox', () => { await userEvent.click(screen.getByRole('combobox')); expect(onOpenChange).not.toHaveBeenCalledWith(true); + expect(screen.queryByRole('option')).not.toBeInTheDocument(); + expect(screen.queryByText('No options found')).not.toBeInTheDocument(); }); it('does not open the menu when every option is filtered out', async () => { @@ -87,4 +89,48 @@ describe('ComboBox', () => { expect(onOpenChange).not.toHaveBeenCalledWith(true); }); + + it('closes the menu after selecting when keepMenuOpenOnSelect is configured', async () => { + render( + ({ + key: item, + label: item, + value: item, + }))} + keepMenuOpenOnSelect={() => false} + /> + ); + + await userEvent.click(screen.getByRole('combobox')); + await userEvent.click(screen.getByRole('option', {name: 'foo'})); + + await waitFor(() => { + expect(screen.queryByRole('option')).not.toBeInTheDocument(); + }); + }); + + it('keeps the menu open when keepMenuOpenOnSelect returns true', async () => { + render( + ({ + key: item, + label: item, + value: item, + }))} + keepMenuOpenOnSelect={() => true} + /> + ); + + await userEvent.click(screen.getByRole('combobox')); + await userEvent.click(screen.getByRole('option', {name: 'foo'})); + + expect(screen.getByRole('option', {name: 'bar'})).toBeInTheDocument(); + }); }); diff --git a/static/app/components/tokenizedInput/token/comboBox.tsx b/static/app/components/tokenizedInput/token/comboBox.tsx index ae29cff3a6ad..4e18cbad6316 100644 --- a/static/app/components/tokenizedInput/token/comboBox.tsx +++ b/static/app/components/tokenizedInput/token/comboBox.tsx @@ -1,6 +1,7 @@ import type { ChangeEventHandler, ClipboardEvent, + FocusEvent, FocusEventHandler, MouseEventHandler, Ref, @@ -36,8 +37,13 @@ interface ComboBoxProps { inputValue: string; items: Array>; ['data-test-id']?: string; + /** + * Keep the suggestion menu open after selecting an option. Useful when the + * user still needs to pick a follow-up value (e.g. filter key → filter value). + */ + keepMenuOpenOnSelect?: boolean | ((option: SelectOptionWithKey) => boolean); onClick?: MouseEventHandler; - onInputBlur?: () => void; + onInputBlur?: (evt?: FocusEvent) => void; onInputChange?: ChangeEventHandler; onInputCommit?: (value: string) => void; onInputEscape?: () => void; @@ -55,6 +61,10 @@ interface ComboBoxProps { * other elements. */ shouldCloseOnInteractOutside?: (interactedElement: Element) => boolean; + /** + * When false, all items from `items` are shown and filtering is left to the caller. + */ + shouldFilterResults?: boolean; tabIndex?: number; } @@ -112,18 +122,63 @@ export function ComboBox({ placeholder, tabIndex, ref, + keepMenuOpenOnSelect, + shouldFilterResults = true, }: ComboBoxProps) { const inputRef = useRef(null); const listBoxRef = useRef(null); const popoverRef = useRef(null); + const openMenuRef = useRef<(() => void) | null>(null); + const closeMenuRef = useRef<(() => void) | null>(null); + const suppressAutoOpenRef = useRef(false); const {hiddenOptions, disabledKeys} = useHiddenItems({ items, filterValue, maxOptions: 50, - shouldFilterResults: true, + shouldFilterResults, + }); + + const hasVisibleItems = items.some(item => { + if (itemIsSectionWithKey(item)) { + return item.options.some(option => !hiddenOptions.has(option.key)); + } + return !hiddenOptions.has(item.key); }); + const shouldKeepMenuOpenOnSelect = useCallback( + (option: SelectOptionWithKey) => { + if (typeof keepMenuOpenOnSelect === 'function') { + return keepMenuOpenOnSelect(option); + } + return keepMenuOpenOnSelect ?? false; + }, + [keepMenuOpenOnSelect] + ); + + const applyOptionSelection = useCallback( + (option: SelectOptionWithKey) => { + onOptionSelected?.(option); + if (shouldKeepMenuOpenOnSelect(option)) { + openMenuRef.current?.(); + return; + } + if (keepMenuOpenOnSelect === undefined) { + return; + } + // Selecting closes the menu and briefly suppresses auto-open so focus/click + // returning to the input does not immediately reopen it. + suppressAutoOpenRef.current = true; + requestAnimationFrame(() => { + closeMenuRef.current?.(); + requestAnimationFrame(() => { + suppressAutoOpenRef.current = false; + }); + }); + }, + [keepMenuOpenOnSelect, onOptionSelected, shouldKeepMenuOpenOnSelect] + ); + const handleValueChange = useCallback( (key: Key | null) => { if (!key) { @@ -134,18 +189,16 @@ export function ComboBox({ if (itemIsSectionWithKey(item)) { const option = item.options.find(child => child.key === key); if (option) { - onOptionSelected?.(option); - break; - } - } else { - if (item.key === key) { - onOptionSelected?.(item); + applyOptionSelection(option); break; } + } else if (item.key === key) { + applyOptionSelection(item); + break; } } }, - [items, onOptionSelected] + [applyOptionSelection, items] ); const comboBoxProps: Partial>> = @@ -168,10 +221,15 @@ export function ComboBox({ shouldCloseOnBlur: false, ...comboBoxProps, }); + openMenuRef.current = () => state.open(); + closeMenuRef.current = () => state.close(); const handleComboBoxFocus: FocusEventHandler = useCallback( evt => { onInputFocus?.(evt); + if (suppressAutoOpenRef.current) { + return; + } state.open(); }, [onInputFocus, state] @@ -182,19 +240,15 @@ export function ComboBox({ if (evt.relatedTarget && !shouldCloseOnInteractOutside?.(evt.relatedTarget)) { return; } - onInputBlur?.(); + onInputBlur?.(evt); state.close(); }, [onInputBlur, shouldCloseOnInteractOutside, state] ); - const totalOptions = items.reduce( - (acc, item) => acc + (itemIsSectionWithKey(item) ? item.options.length : 1), - 0 - ); - // Showing the overlay with nothing to select renders as an empty grey bar - const isOpen = state.isOpen && totalOptions > hiddenOptions.size; + const isOpen = state.isOpen && hasVisibleItems; + const isMenuVisible = isOpen; const handleComboBoxKeyDown = useCallback( (evt: KeyboardEvent) => { @@ -292,6 +346,9 @@ export function ComboBox({ evt.stopPropagation(); inputProps.onClick?.(evt); onClick?.(evt); + if (suppressAutoOpenRef.current) { + return; + } state.open(); }, [inputProps, state, onClick] @@ -327,7 +384,15 @@ export function ComboBox({ onKeyDownCapture={onKeyDownCapture} data-test-id={dataTestId} /> - +