Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/search-bar-drop-where-label.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@hyperdx/app': patch
---

fix: drop the search bar WHERE label and `/` keycap overlay

The `WHERE` label repeated the SQL placeholder, and the `/` hint clipped long queries. `/` and `s` still focus the input; the overlay is gone.
1 change: 0 additions & 1 deletion packages/app/src/DBDashboardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3117,7 +3117,6 @@ function DBDashboardPage({
onLanguageChange={(lang: 'sql' | 'lucene') =>
setValue('whereLanguage', lang)
}
label="WHERE"
enableHotkey
allowMultiline
minWidth={300}
Expand Down
1 change: 0 additions & 1 deletion packages/app/src/SessionSubpanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,6 @@ export default function SessionSubpanel({
control={control}
name="where"
size="xs"
showLabel={false}
enableHotkey
onSubmit={handleSubmit(handleWhereSubmit)}
onLanguageChange={onLanguageChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ export function ChartEditorControls({
onLanguageChange={(lang: 'sql' | 'lucene') =>
setValue('whereLanguage', lang)
}
showLabel={false}
enableVariables
/>
</Flex>
Expand Down Expand Up @@ -562,7 +561,6 @@ export function ChartEditorControls({
onLanguageChange={(lang: 'sql' | 'lucene') =>
setValue('whereLanguage', lang)
}
showLabel={false}
enableVariables
/>
</Flex>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,6 @@ export function ChartSeriesEditor({
control={control}
name={`${namePrefix}aggCondition`}
onSubmit={onSubmit}
showLabel={false}
additionalSuggestions={attributeSuggestions}
data-testid="series-where-input"
enableVariables
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export function HeatmapSeriesEditor({
onLanguageChange={(lang: 'sql' | 'lucene') =>
setValue('whereLanguage', lang)
}
showLabel={false}
enableVariables
/>
<Divider />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ function SearchOnClickFields({ control }: { control: DrawerControl }) {
name="onClick.whereTemplate"
languageName="onClick.whereLanguage"
allowMultiline
showLabel={false}
sqlPlaceholder="ServiceName = '{{ServiceName}}'"
lucenePlaceholder="ServiceName:{{ServiceName}}"
/>
Expand Down Expand Up @@ -162,7 +161,6 @@ function DashboardOnClickFields({
name="onClick.whereTemplate"
languageName="onClick.whereLanguage"
allowMultiline
showLabel={false}
sqlPlaceholder="ServiceName = '{{ServiceName}}'"
lucenePlaceholder="ServiceName:{{ServiceName}}"
/>
Expand Down
2 changes: 0 additions & 2 deletions packages/app/src/components/DBTraceWaterfallChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1288,7 +1288,6 @@ export function DBTraceWaterfallChartContainer({
languageName="traceWhereLanguage"
control={control}
size="xs"
showLabel={false}
allowMultiline={false}
onSubmit={handleSubmit(onSubmitFilters)}
onLanguageChange={lang =>
Expand Down Expand Up @@ -1327,7 +1326,6 @@ export function DBTraceWaterfallChartContainer({
languageName="logWhereLanguage"
control={control}
size="xs"
showLabel={false}
allowMultiline={false}
onSubmit={handleSubmit(onSubmitFilters)}
onLanguageChange={lang =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ export const QueryExpressionFilterEditForm = ({
control={control}
name="where"
languageName="whereLanguage"
showLabel={false}
allowMultiline={true}
sqlPlaceholder="Filter for dropdown values"
lucenePlaceholder="Filter for dropdown values"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
}

.inputWrapper {
position: relative;
flex: 1;
min-width: 0;

Expand Down Expand Up @@ -69,28 +68,3 @@
flex-shrink: 0;
height: calc(var(--editor-base-height) - var(--editor-border-inset, 2px));
}

.shortcutHint {
position: absolute;
top: 50%;
right: 8px;
transform: translateY(-50%);
display: flex;
align-items: center;
justify-content: center;
pointer-events: none;

/* Perfect square Kbd: Mantine Kbd renders as <kbd> */
/* stylelint-disable-next-line selector-pseudo-class-no-unknown */
:global(kbd) {
width: 22px;
height: 22px;
min-width: 22px;
min-height: 22px;
padding: 0;
display: inline-flex;
align-items: center;
justify-content: center;
box-sizing: border-box;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@ const mockTableConnection = {
function SearchWhereInputWrapper({
defaultLanguage = 'lucene',
width,
showLabel,
allowMultiline,
}: {
defaultLanguage?: 'sql' | 'lucene';
width?: string;
showLabel?: boolean;
allowMultiline?: boolean;
}) {
const { control } = useForm({
Expand All @@ -46,7 +44,6 @@ function SearchWhereInputWrapper({
name="where"
enableHotkey
width={width}
showLabel={showLabel}
allowMultiline={allowMultiline}
/>
</Stack>
Expand All @@ -67,11 +64,6 @@ export const CustomWidth = () => (
);
CustomWidth.storyName = 'Custom Width (50%)';

export const NoLabel = () => (
<SearchWhereInputWrapper defaultLanguage="sql" showLabel={false} />
);
NoLabel.storyName = 'SQL Without Label';

export const NoMultiline = () => (
<SearchWhereInputWrapper defaultLanguage="sql" allowMultiline={false} />
);
Expand Down
22 changes: 1 addition & 21 deletions packages/app/src/components/SearchInput/SearchWhereInput.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FieldPath, useController, UseControllerProps } from 'react-hook-form';
import { TableConnectionChoice } from '@hyperdx/common-utils/dist/core/metadata';
import { ActionIcon, Box, Flex, Kbd, Tooltip } from '@mantine/core';
import { ActionIcon, Box, Flex, Tooltip } from '@mantine/core';
import { useDisclosure } from '@mantine/hooks';
import { IconHelp } from '@tabler/icons-react';

Expand Down Expand Up @@ -59,14 +59,6 @@ export type SearchWhereInputProps = {
* Size of the input
*/
size?: 'xs' | 'sm';
/**
* Show label on SQL input (default true). Use label to customize text (e.g. "GLOBAL WHERE").
*/
showLabel?: boolean;
/**
* Label text when showLabel is true (default "WHERE")
*/
label?: string;
/**
* Enable multiline for SQL input
*/
Expand Down Expand Up @@ -155,8 +147,6 @@ export default function SearchWhereInput({
onLanguageChange,
enableHotkey,
size = 'sm',
showLabel = true,
label: labelText = 'WHERE',
allowMultiline = true,
sqlQueryHistoryType,
luceneQueryHistoryType,
Expand Down Expand Up @@ -242,7 +232,6 @@ export default function SearchWhereInput({
name={name}
placeholder={sqlPlaceholder}
onSubmit={onSubmit}
label={showLabel ? labelText : undefined}
queryHistoryType={sqlQueryHistoryType}
enableHotkey={enableHotkey}
allowMultiline={allowMultiline}
Expand Down Expand Up @@ -271,15 +260,6 @@ export default function SearchWhereInput({
enableVariables={enableVariables}
/>
)}
{enableHotkey && (
<Box
className={styles.shortcutHint}
title="Press / or s to focus search"
aria-hidden
>
<Kbd size="xs">/</Kbd>
</Box>
)}
</Box>
</Box>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,17 @@ describe('SearchWhereInput', () => {

describe('Lucene Mode', () => {
it('renders Lucene input when whereLanguage is lucene', () => {
renderWithMantine(<TestWrapper defaultLanguage="lucene" />);
const { container } = renderWithMantine(
<TestWrapper defaultLanguage="lucene" />,
);

// Lucene mode uses a textarea from AutocompleteInput
const input = screen.getByPlaceholderText(
/Search your events w\/ Lucene/i,
);
expect(input).toBeInTheDocument();
// `/` and `s` still focus the input; the overlay keycap is gone.
expect(container.querySelector('kbd')).not.toBeInTheDocument();

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.

🔵 minor — The new assertion pins the keycap's absence, not the hotkey the PR claims still works

expect(container.querySelector('kbd')).not.toBeInTheDocument() is an absence proxy, and the comment beside it ("/ and s still focus the input") is the one thing nothing checks — a grep for press('/')/keyboard('/')/hotkey across packages/app/tests and the jest suites returns nothing, so deleting the useHotkeys(['/', 's'], …) call in SearchInputV2.tsx:128 or SQLEditor/SQLInlineEditor.tsx:228 keeps every test green while breaking the behaviour the changeset promises. Add a case that renders with enableHotkey, fires / on document.body, and asserts the Lucene textarea (and the SQL .cm-content) has focus.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

});

it('allows typing in Lucene mode', async () => {
Expand Down Expand Up @@ -135,10 +139,15 @@ describe('SearchWhereInput', () => {
});

describe('SQL Mode', () => {
it('renders SQL input with WHERE label when whereLanguage is sql', () => {
it('renders the SQL editor when whereLanguage is sql', () => {

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.

🔵 minor — The renamed SQL-mode test no longer pins the removal of the WHERE label

The two assertions that covered the label (getByText('WHERE') here, and the deleted hides label when showLabel is false case) were replaced with checks on the language select's value and the absence of the Lucene placeholder — neither relates to this change, and the next test (renders SQL placeholder) already proves the SQL editor rendered. Restoring label="WHERE" on the SQLInlineEditorControlled in SearchWhereInput.tsx:229 would keep every test green, because the only new assertion about removed chrome (container.querySelector('kbd'), line 92) lives in the Lucene test while the label was SQL-only. Add expect(screen.queryByText('WHERE')).not.toBeInTheDocument() to this SQL-mode test, and move (or duplicate) the kbd assertion into SQL mode as well since the keycap rendered in both languages.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

renderWithMantine(<TestWrapper defaultLanguage="sql" />);

expect(screen.getByText('WHERE')).toBeInTheDocument();
expect(
screen.getByRole('combobox', { name: 'Query language' }),
).toHaveValue('SQL');
expect(
screen.queryByPlaceholderText(/Search your events w\/ Lucene/i),
).not.toBeInTheDocument();
Comment on lines +142 to +150

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.

P2 Missing label regression check

The rewritten SQL-mode test confirms that the SQL editor renders, but it no longer checks the behavior this PR changes: that the WHERE label is absent. Add a negative assertion for the label so a future regression cannot reintroduce it unnoticed.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Conductor Fix in Cursor Fix in Codex

});

it('renders SQL placeholder', () => {
Expand Down Expand Up @@ -196,23 +205,6 @@ describe('SearchWhereInput', () => {
).toBeInTheDocument();
});

it('hides label when showLabel is false', () => {
renderWithMantine(
<TestWrapper defaultLanguage="sql">
{({ control }) => (
<SearchWhereInput
tableConnection={mockTableConnection}
control={control}
name="where"
showLabel={false}
/>
)}
</TestWrapper>,
);

expect(screen.queryByText('WHERE')).not.toBeInTheDocument();
});

it('uses custom placeholders when provided', () => {
renderWithMantine(
<TestWrapper defaultLanguage="lucene">
Expand Down
Loading