From f44ceeb46e33dc79751344752b4ca690bf5c7042 Mon Sep 17 00:00:00 2001 From: nsdeschenes Date: Thu, 8 Jan 2026 15:46:45 -0400 Subject: [PATCH 1/9] :necktie: Add paren key handler --- .../searchQueryBuilder/selectionKeyHandler.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/static/app/components/searchQueryBuilder/selectionKeyHandler.tsx b/static/app/components/searchQueryBuilder/selectionKeyHandler.tsx index c2a1570c1d73df..a9e80fc0e531e9 100644 --- a/static/app/components/searchQueryBuilder/selectionKeyHandler.tsx +++ b/static/app/components/searchQueryBuilder/selectionKeyHandler.tsx @@ -142,6 +142,18 @@ export function SelectionKeyHandler({ return; } + // Wrap selected tokens in parentheses when ( or ) is pressed + if ((e.key === '(' || e.key === ')') && selectedTokens.length > 0) { + e.preventDefault(); + e.stopPropagation(); + dispatch({ + type: 'WRAP_TOKENS_WITH_PARENTHESES', + tokens: selectedTokens, + }); + state.selectionManager.clearSelection(); + return; + } + // If the key pressed will generate a symbol, replace the selection with it if (/^.$/u.test(e.key)) { dispatch({ From 2561fe7e0439baf35cb5dc56e442e397b1a80397 Mon Sep 17 00:00:00 2001 From: nsdeschenes Date: Thu, 8 Jan 2026 15:47:32 -0400 Subject: [PATCH 2/9] :necktie: Update query builder state with wrapping tokens in parens action --- .../hooks/useQueryBuilderState.tsx | 58 ++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx index 4691b1dd2835d8..3f9a984b897ee3 100644 --- a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx +++ b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx @@ -206,6 +206,12 @@ type UpdateLogicOperatorAction = { value: string; }; +type WrapTokensWithParenthesesAction = { + tokens: ParseResultToken[]; + type: 'WRAP_TOKENS_WITH_PARENTHESES'; + focusOverride?: FocusOverride; +}; + type ResetClearAskSeerFeedbackAction = {type: 'RESET_CLEAR_ASK_SEER_FEEDBACK'}; type UpdateFreeTextActions = @@ -242,7 +248,8 @@ export type QueryBuilderActions = | UpdateAggregateArgsAction | MultiSelectFilterValueAction | ResetClearAskSeerFeedbackAction - | UpdateLogicOperatorAction; + | UpdateLogicOperatorAction + | WrapTokensWithParenthesesAction; function removeQueryTokensFromQuery( query: string, @@ -437,6 +444,53 @@ function removeExcessWhitespaceFromParts(...parts: string[]): string { .trim(); } +function wrapTokensWithParentheses( + state: QueryBuilderState, + action: WrapTokensWithParenthesesAction, + parseQuery: (query: string) => ParseResult | null +): QueryBuilderState { + if (action.tokens.length === 0) { + return state; + } + + const firstToken = action.tokens[0]!; + const lastToken = action.tokens[action.tokens.length - 1]!; + + const before = state.query.substring(0, firstToken.location.start.offset); + const middle = state.query.substring( + firstToken.location.start.offset, + lastToken.location.end.offset + ); + const after = state.query.substring(lastToken.location.end.offset); + + const newQuery = `${before}(${middle})${after}`.trim(); + const cursorPosition = firstToken.location.start.offset + middle.length + 2; + const newParsedQuery = parseQuery(newQuery); + + const focusedToken = newParsedQuery?.find( + (token: any) => + token.type === Token.FREE_TEXT && token.location.start.offset >= cursorPosition + ); + + const focusOverride = focusedToken + ? {itemKey: makeTokenKey(focusedToken, newParsedQuery)} + : newParsedQuery?.length + ? { + itemKey: makeTokenKey( + newParsedQuery[newParsedQuery.length - 1]!, + newParsedQuery + ), + } + : null; + + return { + ...state, + query: newQuery, + committedQuery: newQuery, + focusOverride, + }; +} + // Ensures that the replaced token is separated from the rest of the query // and cleans up any extra whitespace function replaceTokensWithPadding( @@ -988,6 +1042,8 @@ export function useQueryBuilderState({ return updateAggregateArgs(state, action, {getFieldDefinition}); case 'TOGGLE_FILTER_VALUE': return multiSelectTokenValue(state, action); + case 'WRAP_TOKENS_WITH_PARENTHESES': + return wrapTokensWithParentheses(state, action, parseQuery); case 'RESET_CLEAR_ASK_SEER_FEEDBACK': return {...state, clearAskSeerFeedback: false}; default: From 587106ac16e77079341363845e91d569bb9b1258 Mon Sep 17 00:00:00 2001 From: nsdeschenes Date: Thu, 8 Jan 2026 15:51:12 -0400 Subject: [PATCH 3/9] :white_check_mark: Add in tests --- .../searchQueryBuilder/index.spec.tsx | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/static/app/components/searchQueryBuilder/index.spec.tsx b/static/app/components/searchQueryBuilder/index.spec.tsx index 8b686f2530239d..d7893727d91a08 100644 --- a/static/app/components/searchQueryBuilder/index.spec.tsx +++ b/static/app/components/searchQueryBuilder/index.spec.tsx @@ -1803,6 +1803,106 @@ describe('SearchQueryBuilder', () => { expect(mockOnChange).toHaveBeenCalledWith('', expect.anything()); }); + it('wraps selected tokens in parentheses with ( key', async () => { + const mockOnChange = jest.fn(); + render( + + ); + + await userEvent.click(getLastInput()); + await userEvent.keyboard('{Control>}a{/Control}'); + await userEvent.keyboard('('); + + expect(mockOnChange).toHaveBeenCalledWith( + '(is:unresolved browser.name:chrome)', + expect.anything() + ); + }); + + it('wraps selected tokens in parentheses with ) key', async () => { + const mockOnChange = jest.fn(); + render( + + ); + + await userEvent.click(getLastInput()); + await userEvent.keyboard('{Control>}a{/Control}'); + await userEvent.keyboard(')'); + + expect(mockOnChange).toHaveBeenCalledWith( + '(is:unresolved browser.name:chrome)', + expect.anything() + ); + }); + + it('wraps single selected token in parentheses', async () => { + const mockOnChange = jest.fn(); + render( + + ); + + await userEvent.click(getLastInput()); + await userEvent.keyboard('{Shift>}{ArrowLeft}{/Shift}'); + await waitFor(() => { + expect(screen.getByRole('row', {name: 'is:unresolved'})).toHaveAttribute( + 'aria-selected', + 'true' + ); + }); + await userEvent.keyboard('('); + + expect(mockOnChange).toHaveBeenCalledWith('(is:unresolved)', expect.anything()); + }); + + it('does not wrap when nothing is selected', async () => { + render(); + + await userEvent.click(getLastInput()); + await userEvent.keyboard('('); + + expect(await screen.findByRole('row', {name: '('})).toBeInTheDocument(); + }); + + it('can undo wrapping with ctrl-z', async () => { + const mockOnChange = jest.fn(); + render( + + ); + + await userEvent.click(getLastInput()); + await userEvent.keyboard('{Control>}a{/Control}'); + await userEvent.keyboard('('); + + expect(mockOnChange).toHaveBeenCalledWith( + '(is:unresolved browser.name:chrome)', + expect.anything() + ); + + await userEvent.keyboard('{Control>}z{/Control}'); + + expect(await screen.findByRole('row', {name: 'is:unresolved'})).toBeInTheDocument(); + expect( + await screen.findByRole('row', {name: 'browser.name:chrome'}) + ).toBeInTheDocument(); + expect(screen.queryByText('(')).not.toBeInTheDocument(); + }); + it('can undo last action with ctrl-z', async () => { render( From 150445f18c1eb57da5753121bea98957deddf5a9 Mon Sep 17 00:00:00 2001 From: nsdeschenes Date: Thu, 8 Jan 2026 16:03:44 -0400 Subject: [PATCH 4/9] :necktie: Ensure there are spaces around the parens --- .../searchQueryBuilder/hooks/useQueryBuilderState.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx index 3f9a984b897ee3..1d717abaa72c00 100644 --- a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx +++ b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx @@ -463,7 +463,7 @@ function wrapTokensWithParentheses( ); const after = state.query.substring(lastToken.location.end.offset); - const newQuery = `${before}(${middle})${after}`.trim(); + const newQuery = `${before} ( ${middle} ) ${after}`.trim(); const cursorPosition = firstToken.location.start.offset + middle.length + 2; const newParsedQuery = parseQuery(newQuery); From 180513d39dba938e1031a80c0e86d61cb4364806 Mon Sep 17 00:00:00 2001 From: nsdeschenes Date: Fri, 9 Jan 2026 07:43:48 -0400 Subject: [PATCH 5/9] :white_check_mark: Fix up test issues from adding spaces around brackets --- static/app/components/searchQueryBuilder/index.spec.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/static/app/components/searchQueryBuilder/index.spec.tsx b/static/app/components/searchQueryBuilder/index.spec.tsx index d7893727d91a08..f390b7599b79e4 100644 --- a/static/app/components/searchQueryBuilder/index.spec.tsx +++ b/static/app/components/searchQueryBuilder/index.spec.tsx @@ -1818,7 +1818,7 @@ describe('SearchQueryBuilder', () => { await userEvent.keyboard('('); expect(mockOnChange).toHaveBeenCalledWith( - '(is:unresolved browser.name:chrome)', + '( is:unresolved browser.name:chrome )', expect.anything() ); }); @@ -1838,7 +1838,7 @@ describe('SearchQueryBuilder', () => { await userEvent.keyboard(')'); expect(mockOnChange).toHaveBeenCalledWith( - '(is:unresolved browser.name:chrome)', + '( is:unresolved browser.name:chrome )', expect.anything() ); }); @@ -1863,7 +1863,7 @@ describe('SearchQueryBuilder', () => { }); await userEvent.keyboard('('); - expect(mockOnChange).toHaveBeenCalledWith('(is:unresolved)', expect.anything()); + expect(mockOnChange).toHaveBeenCalledWith('( is:unresolved )', expect.anything()); }); it('does not wrap when nothing is selected', async () => { @@ -1890,7 +1890,7 @@ describe('SearchQueryBuilder', () => { await userEvent.keyboard('('); expect(mockOnChange).toHaveBeenCalledWith( - '(is:unresolved browser.name:chrome)', + '( is:unresolved browser.name:chrome )', expect.anything() ); From 40735cfb21d96dde763cc0154394d1b4800058ec Mon Sep 17 00:00:00 2001 From: nsdeschenes Date: Fri, 9 Jan 2026 09:46:21 -0400 Subject: [PATCH 6/9] :bug: Use whitespace util to ensure consistent whitespacing --- .../searchQueryBuilder/hooks/useQueryBuilderState.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx index 1d717abaa72c00..aa4051fbe240b9 100644 --- a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx +++ b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx @@ -463,7 +463,7 @@ function wrapTokensWithParentheses( ); const after = state.query.substring(lastToken.location.end.offset); - const newQuery = `${before} ( ${middle} ) ${after}`.trim(); + const newQuery = removeExcessWhitespaceFromParts(before, '(', middle, ')', after); const cursorPosition = firstToken.location.start.offset + middle.length + 2; const newParsedQuery = parseQuery(newQuery); From 5ce2774088109e18ead1a657935d6ae027a86c18 Mon Sep 17 00:00:00 2001 From: nsdeschenes Date: Fri, 9 Jan 2026 09:49:14 -0400 Subject: [PATCH 7/9] :bug: Dynamically calculate cursor position after query has had parens added --- .../hooks/useQueryBuilderState.tsx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx index aa4051fbe240b9..19a3fa545f6cfa 100644 --- a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx +++ b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx @@ -464,7 +464,21 @@ function wrapTokensWithParentheses( const after = state.query.substring(lastToken.location.end.offset); const newQuery = removeExcessWhitespaceFromParts(before, '(', middle, ')', after); - const cursorPosition = firstToken.location.start.offset + middle.length + 2; + // Calculate cursor position in the new query: find the closing ')' that we just added. + const trimmedMiddle = middle.trim(); + let cursorPosition = newQuery.length; // Fallback: position at end of query + + if (trimmedMiddle.length > 0) { + const middleStartIndex = newQuery.indexOf(trimmedMiddle); + if (middleStartIndex >= 0) { + const middleEndIndex = middleStartIndex + trimmedMiddle.length; + // Find the ')' that comes after middle (there should be a space before it) + const closingParenIndex = newQuery.indexOf(')', middleEndIndex); + if (closingParenIndex >= 0) { + cursorPosition = closingParenIndex + 1; + } + } + } const newParsedQuery = parseQuery(newQuery); const focusedToken = newParsedQuery?.find( From 8432e82837219105c99aa3df7ea80a818f62550f Mon Sep 17 00:00:00 2001 From: nsdeschenes Date: Fri, 9 Jan 2026 10:18:46 -0400 Subject: [PATCH 8/9] :bug: More fixes to calculating cursor position --- .../hooks/useQueryBuilderState.tsx | 28 +++---- .../searchQueryBuilder/index.spec.tsx | 73 +++++++++++++++++++ 2 files changed, 88 insertions(+), 13 deletions(-) diff --git a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx index 19a3fa545f6cfa..a62a21c6577978 100644 --- a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx +++ b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx @@ -464,20 +464,22 @@ function wrapTokensWithParentheses( const after = state.query.substring(lastToken.location.end.offset); const newQuery = removeExcessWhitespaceFromParts(before, '(', middle, ')', after); - // Calculate cursor position in the new query: find the closing ')' that we just added. + + // Calculate cursor position: position right after the closing ')' we just added. + let cursorPosition: number; + const trimmedBefore = before.trim(); const trimmedMiddle = middle.trim(); - let cursorPosition = newQuery.length; // Fallback: position at end of query - - if (trimmedMiddle.length > 0) { - const middleStartIndex = newQuery.indexOf(trimmedMiddle); - if (middleStartIndex >= 0) { - const middleEndIndex = middleStartIndex + trimmedMiddle.length; - // Find the ')' that comes after middle (there should be a space before it) - const closingParenIndex = newQuery.indexOf(')', middleEndIndex); - if (closingParenIndex >= 0) { - cursorPosition = closingParenIndex + 1; - } - } + if (trimmedMiddle.length === 0) { + // Edge case: empty middle, position at end + cursorPosition = newQuery.length; + } else if (trimmedBefore.length > 0) { + // Format: "trimmedBefore ( trimmedMiddle ) ..." + // Position: trimmedBefore + " " + "(" + " " + trimmedMiddle + " " + ")" = len + 4 + len + 1 + cursorPosition = trimmedBefore.length + trimmedMiddle.length + 5; + } else { + // Format: "( trimmedMiddle ) ..." + // Position: "(" + " " + trimmedMiddle + " " + ")" = 2 + len + 2 + cursorPosition = trimmedMiddle.length + 4; } const newParsedQuery = parseQuery(newQuery); diff --git a/static/app/components/searchQueryBuilder/index.spec.tsx b/static/app/components/searchQueryBuilder/index.spec.tsx index f390b7599b79e4..3d547c41585ed2 100644 --- a/static/app/components/searchQueryBuilder/index.spec.tsx +++ b/static/app/components/searchQueryBuilder/index.spec.tsx @@ -1875,6 +1875,79 @@ describe('SearchQueryBuilder', () => { expect(await screen.findByRole('row', {name: '('})).toBeInTheDocument(); }); + it('wraps selected tokens correctly when existing parentheses are present', async () => { + const mockOnChange = jest.fn(); + render( + + ); + + await userEvent.click(getLastInput()); + // Select only the browser.name token (shift+left) + await userEvent.keyboard('{Shift>}{ArrowLeft}{/Shift}'); + await waitFor(() => { + expect(screen.getByRole('row', {name: 'browser.name:chrome'})).toHaveAttribute( + 'aria-selected', + 'true' + ); + }); + await userEvent.keyboard('('); + + // Should wrap only the selected token, preserving existing parens + expect(mockOnChange).toHaveBeenCalledWith( + '( is:unresolved ) ( browser.name:chrome )', + expect.anything() + ); + }); + + it('wraps selected tokens correctly when duplicate content appears earlier', async () => { + const mockOnChange = jest.fn(); + render( + + ); + + await userEvent.click(getLastInput()); + // Select only the last browser.name token (shift+left) + await userEvent.keyboard('{Shift>}{ArrowLeft}{/Shift}'); + await waitFor(() => { + // Both have the same name, so just check something is selected + const rows = screen.getAllByRole('row', {name: 'browser.name:firefox'}); + expect(rows[1]).toHaveAttribute('aria-selected', 'true'); + }); + await userEvent.keyboard('('); + + // Should wrap the second occurrence correctly + expect(mockOnChange).toHaveBeenCalledWith( + 'browser.name:firefox ( browser.name:firefox )', + expect.anything() + ); + }); + + it('places focus after the closing paren when wrapping', async () => { + render( + + ); + + await userEvent.click(getLastInput()); + await userEvent.keyboard('{Control>}a{/Control}'); + await userEvent.keyboard('('); + + // After wrapping, focus should be at the end (last input) + await waitFor(() => { + expect(getLastInput()).toHaveFocus(); + }); + }); + it('can undo wrapping with ctrl-z', async () => { const mockOnChange = jest.fn(); render( From a1b8bb99006a57192e8715b72e0abdc3fa252064 Mon Sep 17 00:00:00 2001 From: nsdeschenes Date: Fri, 9 Jan 2026 10:21:29 -0400 Subject: [PATCH 9/9] :art: Tidy up logic around setting focus override --- .../hooks/useQueryBuilderState.tsx | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx index a62a21c6577978..1169b5a0b47200 100644 --- a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx +++ b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx @@ -488,16 +488,15 @@ function wrapTokensWithParentheses( token.type === Token.FREE_TEXT && token.location.start.offset >= cursorPosition ); - const focusOverride = focusedToken - ? {itemKey: makeTokenKey(focusedToken, newParsedQuery)} - : newParsedQuery?.length - ? { - itemKey: makeTokenKey( - newParsedQuery[newParsedQuery.length - 1]!, - newParsedQuery - ), - } - : null; + let focusOverride: FocusOverride | null = null; + if (focusedToken) { + focusOverride = {itemKey: makeTokenKey(focusedToken, newParsedQuery)}; + } else if (newParsedQuery?.[newParsedQuery.length - 1]) { + focusOverride = { + // We know that the last token exists because of the check above, but TS isn't happy + itemKey: makeTokenKey(newParsedQuery[newParsedQuery.length - 1]!, newParsedQuery), + }; + } return { ...state,