mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site): prevent chat search filter pills from overflowing the dialog (#26095)
Long filter values (for example diff URLs) were widening the chat search dialog instead of staying inside the search box, and some passthrough filters did not round-trip cleanly through the backend query parser. This fixes the filter pills so long values truncate inside the search box (with the full value available via a hover tooltip), and normalizes passthrough filters so values containing spaces or colons are quoted and re-parse identically. closes CODAGT-556
This commit is contained in:
+95
@@ -85,6 +85,8 @@ const cappedMockChats: Chat[] = Array.from(
|
||||
diff_status: undefined,
|
||||
}),
|
||||
);
|
||||
const longDiffURL =
|
||||
"github.com/coder/coder/pull/26016/files/1234567890abcdef1234567890abcdef1234567890abcdef";
|
||||
|
||||
const meta: Meta<typeof ChatSearchDialog> = {
|
||||
title: "pages/AgentsPage/ChatSearchDialog",
|
||||
@@ -123,6 +125,42 @@ type Story = StoryObj<typeof ChatSearchDialog>;
|
||||
|
||||
export const EmptyState: Story = {};
|
||||
|
||||
export const IconInputAlignment: Story = {
|
||||
play: async () => {
|
||||
const body = within(document.body);
|
||||
const searchInput = await body.findByRole("combobox", {
|
||||
name: "Search chats",
|
||||
});
|
||||
const toggleButton = await body.findByRole("button", {
|
||||
name: "Toggle filters",
|
||||
});
|
||||
|
||||
const container = toggleButton.parentElement;
|
||||
if (!container) {
|
||||
throw new Error("Expected the toggle button to have a parent container");
|
||||
}
|
||||
const searchIcon = container.querySelector("svg");
|
||||
const filterIcon = toggleButton.querySelector("svg");
|
||||
if (!searchIcon || !filterIcon) {
|
||||
throw new Error("Expected the search and filter icons to render");
|
||||
}
|
||||
|
||||
const verticalCenter = (element: Element) => {
|
||||
const rect = element.getBoundingClientRect();
|
||||
return rect.top + rect.height / 2;
|
||||
};
|
||||
await waitFor(() => {
|
||||
const inputCenter = verticalCenter(searchInput);
|
||||
expect(
|
||||
Math.abs(verticalCenter(searchIcon) - inputCenter),
|
||||
).toBeLessThanOrEqual(1);
|
||||
expect(
|
||||
Math.abs(verticalCenter(filterIcon) - inputCenter),
|
||||
).toBeLessThanOrEqual(1);
|
||||
});
|
||||
},
|
||||
};
|
||||
|
||||
export const LoadingState: Story = {
|
||||
beforeEach: () => {
|
||||
spyOn(API.experimental, "getChats").mockImplementation(
|
||||
@@ -461,6 +499,63 @@ export const ParameterizedFilterPill: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
export const DiffURLFilterPill: Story = {
|
||||
beforeEach: () => {
|
||||
spyOn(API.experimental, "getChats").mockResolvedValue(mockChats);
|
||||
},
|
||||
play: async () => {
|
||||
const body = within(document.body);
|
||||
const searchInput = body.getByRole("combobox", { name: "Search chats" });
|
||||
const toggleButton = body.getByRole("button", { name: "Toggle filters" });
|
||||
|
||||
await userEvent.click(toggleButton);
|
||||
await userEvent.click(await body.findByText("Diff URL"));
|
||||
|
||||
await expect(await body.findByText("diff_url:")).toBeInTheDocument();
|
||||
|
||||
await userEvent.click(searchInput);
|
||||
await userEvent.type(searchInput, `${longDiffURL} `);
|
||||
|
||||
const diffURLPill = await body.findByText(`diff_url:${longDiffURL}`);
|
||||
await expect(diffURLPill).toBeInTheDocument();
|
||||
await expect(diffURLPill).toHaveAttribute(
|
||||
"title",
|
||||
`diff_url:${longDiffURL}`,
|
||||
);
|
||||
await expect(searchInput).toBeVisible();
|
||||
|
||||
const searchContainer = searchInput.parentElement;
|
||||
const searchWrapper = searchContainer?.parentElement;
|
||||
if (!searchContainer || !searchWrapper) {
|
||||
throw new Error(
|
||||
"Expected search input to render inside nested containers",
|
||||
);
|
||||
}
|
||||
|
||||
const dialog = searchWrapper.closest('[role="dialog"]');
|
||||
if (!dialog) {
|
||||
throw new Error("Expected the search input to render inside a dialog");
|
||||
}
|
||||
|
||||
await waitFor(() => {
|
||||
const dialogRight = Math.ceil(dialog.getBoundingClientRect().right);
|
||||
expect(
|
||||
Math.ceil(searchWrapper.getBoundingClientRect().right),
|
||||
).toBeLessThanOrEqual(dialogRight);
|
||||
expect(
|
||||
Math.ceil(diffURLPill.getBoundingClientRect().right),
|
||||
).toBeLessThanOrEqual(dialogRight);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(API.experimental.getChats).toHaveBeenCalledWith({
|
||||
limit: CHAT_SEARCH_LIMIT,
|
||||
q: `diff_url:"https://${longDiffURL}"`,
|
||||
});
|
||||
});
|
||||
},
|
||||
};
|
||||
|
||||
export const ParameterizedFilterPillEnterCommit: Story = {
|
||||
beforeEach: () => {
|
||||
spyOn(API.experimental, "getChats").mockResolvedValue(mockChats);
|
||||
|
||||
@@ -400,7 +400,7 @@ const ChatSearchDialogContent: FC<ChatSearchDialogContentProps> = ({
|
||||
the dropdown, but clicks within the dropdown (which is
|
||||
inside the same container) don't trigger blur. */}
|
||||
<div
|
||||
className="relative"
|
||||
className="relative w-full min-w-0 max-w-full"
|
||||
onBlur={(e) => {
|
||||
if (!e.currentTarget.contains(e.relatedTarget)) {
|
||||
setIsDropdownOpen(false);
|
||||
|
||||
@@ -45,56 +45,63 @@ export const ChatSearchInput: FC<ChatSearchInputProps> = ({
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
"flex min-h-10 w-full items-center gap-1.5 rounded-md border border-solid border-border-default bg-surface-primary px-3",
|
||||
"flex min-h-10 w-full min-w-0 items-start gap-1.5 rounded-md border border-solid border-border-default bg-surface-primary px-3 py-2",
|
||||
"focus-within:ring-2 focus-within:ring-content-link",
|
||||
)}
|
||||
>
|
||||
<SearchIcon className="size-4 shrink-0 text-content-secondary" />
|
||||
{completedFilters.map((f) => (
|
||||
<span
|
||||
key={f.key}
|
||||
className="inline-flex shrink-0 items-center gap-1 rounded-md border border-solid border-border bg-surface-secondary px-2 py-0.5 text-xs text-content-secondary"
|
||||
>
|
||||
<span>
|
||||
{f.key}:{f.value}
|
||||
</span>
|
||||
<button
|
||||
type="button"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
onRemoveFilter(f.key);
|
||||
}}
|
||||
className="inline-flex cursor-pointer items-center border-none bg-transparent p-0 text-content-secondary hover:text-content-primary"
|
||||
aria-label={`Remove ${f.key} filter`}
|
||||
<div className="flex h-7 shrink-0 items-center">
|
||||
<SearchIcon className="size-4 text-content-secondary" />
|
||||
</div>
|
||||
<div className="flex min-w-0 flex-1 flex-wrap items-center gap-1.5">
|
||||
{completedFilters.map((f) => (
|
||||
<span
|
||||
key={f.key}
|
||||
className="inline-flex max-w-full min-w-0 items-center gap-1 rounded-md border border-solid border-border bg-surface-secondary px-2 py-0.5 text-xs text-content-secondary"
|
||||
>
|
||||
<XIcon className="size-3" />
|
||||
</button>
|
||||
</span>
|
||||
))}
|
||||
{incompleteFilter && (
|
||||
<span className="inline-flex shrink-0 items-center rounded-md border border-dashed border-border bg-surface-secondary px-2 py-0.5 text-xs text-content-secondary">
|
||||
{incompleteFilter.key}:
|
||||
</span>
|
||||
)}
|
||||
<input
|
||||
ref={inputRef}
|
||||
value={value}
|
||||
onChange={onChange}
|
||||
onKeyDown={onKeyDown}
|
||||
placeholder={filters.length > 0 ? "" : "Search chats..."}
|
||||
className="min-w-[60px] flex-1 border-none bg-transparent py-2 text-sm text-content-primary outline-none placeholder:text-content-disabled"
|
||||
aria-label="Search chats"
|
||||
role="combobox"
|
||||
aria-controls={hasResults ? listboxId : undefined}
|
||||
aria-expanded={hasResults}
|
||||
aria-haspopup="listbox"
|
||||
aria-activedescendant={activeResultId}
|
||||
/>
|
||||
<span
|
||||
className="block min-w-0 truncate"
|
||||
title={`${f.key}:${f.value}`}
|
||||
>
|
||||
{f.key}:{f.value}
|
||||
</span>
|
||||
<button
|
||||
type="button"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
onRemoveFilter(f.key);
|
||||
}}
|
||||
className="inline-flex shrink-0 cursor-pointer items-center border-none bg-transparent p-0 text-content-secondary hover:text-content-primary"
|
||||
aria-label={`Remove ${f.key} filter`}
|
||||
>
|
||||
<XIcon className="size-3" />
|
||||
</button>
|
||||
</span>
|
||||
))}
|
||||
{incompleteFilter && (
|
||||
<span className="inline-flex shrink-0 items-center rounded-md border border-dashed border-border bg-surface-secondary px-2 py-0.5 text-xs text-content-secondary">
|
||||
{incompleteFilter.key}:
|
||||
</span>
|
||||
)}
|
||||
<input
|
||||
ref={inputRef}
|
||||
value={value}
|
||||
onChange={onChange}
|
||||
onKeyDown={onKeyDown}
|
||||
placeholder={filters.length > 0 ? "" : "Search chats..."}
|
||||
className="min-w-[60px] flex-1 basis-[60px] border-none bg-transparent py-0.5 text-sm text-content-primary outline-none placeholder:text-content-disabled"
|
||||
aria-label="Search chats"
|
||||
role="combobox"
|
||||
aria-controls={hasResults ? listboxId : undefined}
|
||||
aria-expanded={hasResults}
|
||||
aria-haspopup="listbox"
|
||||
aria-activedescendant={activeResultId}
|
||||
/>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
onClick={onToggleDropdown}
|
||||
className={cn(
|
||||
"inline-flex shrink-0 cursor-pointer items-center border-none bg-transparent p-0 text-content-secondary hover:text-content-primary",
|
||||
"inline-flex h-7 shrink-0 cursor-pointer items-center border-none bg-transparent p-0 text-content-secondary hover:text-content-primary",
|
||||
isDropdownOpen && "text-content-primary",
|
||||
)}
|
||||
aria-label="Toggle filters"
|
||||
|
||||
@@ -7,7 +7,7 @@ describe("normalizeChatSearchInput", () => {
|
||||
expect(normalizeChatSearchInput(" ")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("keeps key:value filters unchanged", () => {
|
||||
it("normalizes key:value filters", () => {
|
||||
expect(normalizeChatSearchInput("has_unread:true")).toBe("has_unread:true");
|
||||
expect(normalizeChatSearchInput('title:"chat title" archived:true')).toBe(
|
||||
'title:"chat title" archived:true',
|
||||
@@ -20,6 +20,25 @@ describe("normalizeChatSearchInput", () => {
|
||||
'diff_url:"https://github.com/coder/coder/pull/25391"',
|
||||
),
|
||||
).toBe('diff_url:"https://github.com/coder/coder/pull/25391"');
|
||||
expect(
|
||||
normalizeChatSearchInput(
|
||||
"diff_url:https://github.com/coder/coder/pull/26016",
|
||||
),
|
||||
).toBe('diff_url:"https://github.com/coder/coder/pull/26016"');
|
||||
expect(
|
||||
normalizeChatSearchInput("diff_url:github.com/coder/coder/pull/26016"),
|
||||
).toBe('diff_url:"https://github.com/coder/coder/pull/26016"');
|
||||
expect(
|
||||
normalizeChatSearchInput('diff_url:"github.com/coder/coder/pull/26016"'),
|
||||
).toBe('diff_url:"https://github.com/coder/coder/pull/26016"');
|
||||
});
|
||||
|
||||
it("re-quotes passthrough values containing spaces so the result round-trips", () => {
|
||||
const normalized = normalizeChatSearchInput('pr_status:"open merged"');
|
||||
expect(normalized).toBe('pr_status:"open merged"');
|
||||
expect(normalizeChatSearchInput(normalized ?? "")).toBe(
|
||||
'pr_status:"open merged"',
|
||||
);
|
||||
});
|
||||
|
||||
it("converts bare search text into a title filter", () => {
|
||||
@@ -40,6 +59,11 @@ describe("normalizeChatSearchInput", () => {
|
||||
expect(normalizeChatSearchInput("fix has_unread:true auth")).toBe(
|
||||
'has_unread:true title:"fix auth"',
|
||||
);
|
||||
expect(
|
||||
normalizeChatSearchInput(
|
||||
"diff_url:https://github.com/coder/coder/pull/26016 fix",
|
||||
),
|
||||
).toBe('diff_url:"https://github.com/coder/coder/pull/26016" title:"fix"');
|
||||
expect(
|
||||
normalizeChatSearchInput('archived:true title:"chat title" fix'),
|
||||
).toBe('archived:true title:"chat title fix"');
|
||||
|
||||
@@ -6,6 +6,10 @@ const sanitizeChatSearchValue = (value: string): string => {
|
||||
return value.replaceAll('"', "");
|
||||
};
|
||||
|
||||
const addDefaultURLScheme = (value: string): string => {
|
||||
return /^[a-z][a-z\d+\-.]*:\/\//i.test(value) ? value : `https://${value}`;
|
||||
};
|
||||
|
||||
// Filter keys that may pass through to the backend unchanged. `title` is not
|
||||
// listed here because bare text and `title:` filters are merged into a single
|
||||
// title filter; see the title-handling branch in normalizeChatSearchInput.
|
||||
@@ -62,7 +66,7 @@ const getKeyValueDelimiterIndex = (token: string): number | undefined => {
|
||||
|
||||
const getKeyValuePair = (
|
||||
token: string,
|
||||
): { key: string; value: string } | undefined => {
|
||||
): { key: string; rawKey: string; value: string } | undefined => {
|
||||
const delimiterIndex = getKeyValueDelimiterIndex(token);
|
||||
if (
|
||||
delimiterIndex === undefined ||
|
||||
@@ -72,18 +76,40 @@ const getKeyValuePair = (
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const rawKey = token.slice(0, delimiterIndex).replaceAll('"', "");
|
||||
return {
|
||||
key: token.slice(0, delimiterIndex).replaceAll('"', "").toLowerCase(),
|
||||
key: rawKey.toLowerCase(),
|
||||
rawKey,
|
||||
value: token.slice(delimiterIndex + 1).replace(/^"|"$/g, ""),
|
||||
};
|
||||
};
|
||||
|
||||
// The backend splits on unquoted whitespace and colons, so values containing
|
||||
// either (e.g. a diff URL) must be quoted.
|
||||
const normalizePassthroughChatSearchFilter = ({
|
||||
key,
|
||||
rawKey,
|
||||
value,
|
||||
}: {
|
||||
readonly key: string;
|
||||
readonly rawKey: string;
|
||||
readonly value: string;
|
||||
}): string => {
|
||||
const sanitizedValue =
|
||||
key === "diff_url"
|
||||
? addDefaultURLScheme(sanitizeChatSearchValue(value))
|
||||
: sanitizeChatSearchValue(value);
|
||||
return sanitizedValue.includes(":") || sanitizedValue.includes(" ")
|
||||
? `${rawKey}:"${sanitizedValue}"`
|
||||
: `${rawKey}:${sanitizedValue}`;
|
||||
};
|
||||
|
||||
/**
|
||||
* Normalizes raw search input into a query string the chat search API accepts.
|
||||
*
|
||||
* Bare text and `title:` filters are merged into a single `title:"..."`
|
||||
* filter (the backend rejects a parameter that appears more than once).
|
||||
* Recognized `key:value` filters pass through unchanged.
|
||||
* Recognized `key:value` filters are normalized for backend syntax.
|
||||
*/
|
||||
export const normalizeChatSearchInput = (
|
||||
rawInput: string,
|
||||
@@ -94,7 +120,8 @@ export const normalizeChatSearchInput = (
|
||||
}
|
||||
|
||||
const tokens = splitSearchInput(trimmedInput);
|
||||
const keyValuePairs: string[] = [];
|
||||
const passthroughFilters: string[] = [];
|
||||
const normalizedTokens: string[] = [];
|
||||
const titleTerms: string[] = [];
|
||||
let hasBareTitleText = false;
|
||||
|
||||
@@ -107,6 +134,7 @@ export const normalizeChatSearchInput = (
|
||||
}
|
||||
|
||||
if (keyValuePair.key === "title") {
|
||||
normalizedTokens.push(token);
|
||||
titleTerms.push(keyValuePair.value);
|
||||
continue;
|
||||
}
|
||||
@@ -117,7 +145,9 @@ export const normalizeChatSearchInput = (
|
||||
continue;
|
||||
}
|
||||
|
||||
keyValuePairs.push(token);
|
||||
const normalizedFilter = normalizePassthroughChatSearchFilter(keyValuePair);
|
||||
passthroughFilters.push(normalizedFilter);
|
||||
normalizedTokens.push(normalizedFilter);
|
||||
}
|
||||
|
||||
// Multiple title values must be merged into a single title filter because
|
||||
@@ -127,11 +157,11 @@ export const normalizeChatSearchInput = (
|
||||
}
|
||||
|
||||
if (!hasBareTitleText) {
|
||||
return trimmedInput;
|
||||
return normalizedTokens.join(" ");
|
||||
}
|
||||
|
||||
return [
|
||||
...keyValuePairs,
|
||||
...passthroughFilters,
|
||||
`title:"${sanitizeChatSearchValue(titleTerms.join(" "))}"`,
|
||||
].join(" ");
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user