mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
fix(site/src): stop delete dialog keys and clicks from activating rows (#29868)
Fixes [DASH-106](https://linear.app/codercom/issue/DASH-106/show-validation-error-for-incorrect-workspace-deletion-confirmation). In the workspaces list, pressing Enter or Space in the delete confirmation input took you to the workspace page. Nothing was deleted and there was no explanation. Review turned up related problems in the same dialog, which this PR also fixes. **Root cause:** React sends events from portaled content (dialogs, menus) up through the component tree to parent components. The delete dialog is rendered inside a clickable workspace row. The row reacted to Enter and Space from the dialog's input as if the row itself had been activated, and it did the same for middle clicks. The actions cell already stopped normal clicks from reaching the row, but not key events or middle clicks. **Changes** - `useClickable` / `useClickableTableRow`: - Enter and Space only activate the row when the row itself has focus. - Clicks, middle-clicks, and double-clicks from portaled content are ignored (`isFromPortal`). A middle-click paste (Linux) into the dialog no longer opens the workspace in a new tab. - A nested native button still sends its own click to the row unless it stops click propagation, as the existing row buttons already do. - New `DeleteConfirmationField` + `useDeleteConfirmation`, shared by `WorkspaceDeleteDialog` and `DeleteDialog`. They previously duplicated this logic. The shared field provides: - Enter with a wrong name shows the error immediately, instead of only after the input loses focus. Screen readers announce it (`role="alert"`). - The typed text is quoted and keeps its whitespace, so a stray space is visible. - `WorkspaceDeleteDialog` clears the typed name, "Orphan Resources", and the error whenever the dialog closes, including after confirm (the parent closes it then). Before, typing the correct name, cancelling, and reopening showed an enabled Delete button, so one click deleted the workspace without retyping. - `DeleteDialog` (shared) clears the typed name whenever its dialog closes, including when the parent closes it, as the organization page does after a failed delete. Before, reopening showed Delete already enabled. Callers that keep the dialog open after a failed delete (templates, users) still keep the typed name so the user can retry. - `WorkspaceOutdatedTooltip`, `ProviderRow`, `ModelRow`: removed key handlers that only worked around the old row behavior, and corrected their comments. **Decisions** - The mismatch error appears below the input without a reserved slot, so the controls below move down when it shows (unchanged from `main`, by design decision). - Enter with an empty input stays silent on purpose. It doesn't confirm or navigate, and the disabled Delete button signals that a name is needed. - Workspace names can't contain spaces, so a typed space leads to the mismatch error. The input doesn't block spaces. **Verification** - Unit tests: - Enter and Space from inline and portaled inputs don't activate a clickable element. - Clicks and middle-clicks from portaled content don't activate the row. - Enter with a wrong name marks the input invalid and announces the error. Editing the name clears it. - A matching name confirms on Enter. - The workspace dialog reopens empty after cancel and after confirm. The shared dialog reopens empty after its parent closes it, and keeps the typed name while it stays open for a retry. - Each new test fails when the code it covers is removed. - Stories: new `FilledWrongSubmitted` (error shown while focused). - Related unit tests and stories pass for every clickable table page and every `DeleteDialog` caller (workspaces, templates, organizations, users, Agents). - Tested by hand by @tracyjohnsonux before the reset, shared-field, and click-guard changes. --- _Generated by Coder Agents on behalf of @tracyjohnsonux._
This commit is contained in:
@@ -0,0 +1,101 @@
|
||||
import {
|
||||
type ChangeEvent,
|
||||
type FC,
|
||||
type KeyboardEvent,
|
||||
type ReactNode,
|
||||
useId,
|
||||
useState,
|
||||
} from "react";
|
||||
import { Input } from "#/components/Input/Input";
|
||||
import { Label } from "#/components/Label/Label";
|
||||
|
||||
/**
|
||||
* State for a "type the name to confirm" field. It clears when `isOpen` becomes
|
||||
* false and is kept while the dialog stays open, so a failed delete can be
|
||||
* retried.
|
||||
*/
|
||||
export const useDeleteConfirmation = (name: string, isOpen: boolean) => {
|
||||
const [value, setValue] = useState("");
|
||||
const [isFocused, setIsFocused] = useState(false);
|
||||
const [hasSubmittedInvalid, setHasSubmittedInvalid] = useState(false);
|
||||
const [prevIsOpen, setPrevIsOpen] = useState(isOpen);
|
||||
|
||||
if (isOpen !== prevIsOpen) {
|
||||
setPrevIsOpen(isOpen);
|
||||
if (!isOpen) {
|
||||
setValue("");
|
||||
setIsFocused(false);
|
||||
setHasSubmittedInvalid(false);
|
||||
}
|
||||
}
|
||||
|
||||
const confirmed = value === name;
|
||||
const showError =
|
||||
!confirmed && value.length > 0 && (!isFocused || hasSubmittedInvalid);
|
||||
|
||||
return {
|
||||
name,
|
||||
value,
|
||||
confirmed,
|
||||
showError,
|
||||
inputProps: {
|
||||
value,
|
||||
onChange: (event: ChangeEvent<HTMLInputElement>) => {
|
||||
setValue(event.target.value);
|
||||
setHasSubmittedInvalid(false);
|
||||
},
|
||||
onFocus: () => setIsFocused(true),
|
||||
onBlur: () => setIsFocused(false),
|
||||
onKeyDown: (event: KeyboardEvent<HTMLInputElement>) => {
|
||||
// DeleteDialog disables its submit button for a wrong name, so its form
|
||||
// never submits and onSubmit cannot set this.
|
||||
if (event.key === "Enter" && !confirmed) {
|
||||
setHasSubmittedInvalid(true);
|
||||
}
|
||||
},
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
type DeleteConfirmationFieldProps = {
|
||||
confirmation: ReturnType<typeof useDeleteConfirmation>;
|
||||
label: ReactNode;
|
||||
entity: string;
|
||||
};
|
||||
|
||||
export const DeleteConfirmationField: FC<DeleteConfirmationFieldProps> = ({
|
||||
confirmation,
|
||||
label,
|
||||
entity,
|
||||
}) => {
|
||||
const inputId = useId();
|
||||
const errorId = `${inputId}-error`;
|
||||
const { name, value, showError, inputProps } = confirmation;
|
||||
|
||||
return (
|
||||
<div className="flex flex-col gap-2">
|
||||
<Label htmlFor={inputId}>{label}</Label>
|
||||
<Input
|
||||
{...inputProps}
|
||||
id={inputId}
|
||||
className="text-content-primary"
|
||||
name="confirmation"
|
||||
autoComplete="off"
|
||||
autoFocus
|
||||
placeholder={name}
|
||||
aria-invalid={showError}
|
||||
aria-describedby={showError ? errorId : undefined}
|
||||
data-testid="delete-dialog-name-confirmation"
|
||||
/>
|
||||
{showError && (
|
||||
<span
|
||||
id={errorId}
|
||||
role="alert"
|
||||
className="whitespace-pre-wrap text-xs text-content-destructive"
|
||||
>
|
||||
“{value}” does not match the name of this {entity}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
};
|
||||
@@ -48,11 +48,17 @@ export const FilledWrong: Story = {
|
||||
await user.type(input, "InvalidFooName");
|
||||
// Blur so the mismatch error becomes visible.
|
||||
await user.tab();
|
||||
},
|
||||
};
|
||||
|
||||
await expect(body.getByRole("button", { name: "Delete" })).toBeDisabled();
|
||||
await expect(
|
||||
body.getByText("InvalidFooName does not match the name of this foo"),
|
||||
).toBeVisible();
|
||||
// Shows the error while the input still has focus after pressing Enter.
|
||||
export const FilledWrongSubmitted: Story = {
|
||||
play: async ({ canvasElement }) => {
|
||||
const body = within(canvasElement.ownerDocument.body);
|
||||
await userEvent.type(
|
||||
await body.findByLabelText("Name of the foo to delete"),
|
||||
"InvalidFooName{Enter}",
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
import { screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { type FC, useState } from "react";
|
||||
import { render } from "#/testHelpers/renderHelpers";
|
||||
import { DeleteDialog } from "./DeleteDialog";
|
||||
|
||||
type HarnessProps = {
|
||||
onConfirm: () => void;
|
||||
onCancel: () => void;
|
||||
closeOnConfirm: boolean;
|
||||
};
|
||||
|
||||
// Callers either close the dialog after confirming or keep it open to retry.
|
||||
const Harness: FC<HarnessProps> = ({ onConfirm, onCancel, closeOnConfirm }) => {
|
||||
const [isOpen, setIsOpen] = useState(true);
|
||||
return (
|
||||
<>
|
||||
<button type="button" onClick={() => setIsOpen(true)}>
|
||||
Reopen
|
||||
</button>
|
||||
<DeleteDialog
|
||||
isOpen={isOpen}
|
||||
onConfirm={() => {
|
||||
if (closeOnConfirm) {
|
||||
setIsOpen(false);
|
||||
}
|
||||
onConfirm();
|
||||
}}
|
||||
onCancel={() => {
|
||||
setIsOpen(false);
|
||||
onCancel();
|
||||
}}
|
||||
entity="workspace"
|
||||
name="my-workspace"
|
||||
/>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
const renderDialog = ({ closeOnConfirm = false } = {}) => {
|
||||
const onConfirm = vi.fn();
|
||||
const onCancel = vi.fn();
|
||||
|
||||
render(
|
||||
<Harness
|
||||
onConfirm={onConfirm}
|
||||
onCancel={onCancel}
|
||||
closeOnConfirm={closeOnConfirm}
|
||||
/>,
|
||||
);
|
||||
|
||||
return {
|
||||
onConfirm,
|
||||
onCancel,
|
||||
input: screen.getByLabelText("Name of the workspace to delete"),
|
||||
};
|
||||
};
|
||||
|
||||
describe("DeleteDialog", () => {
|
||||
it("marks the input invalid instead of confirming when Enter is pressed with a wrong name", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { onConfirm, onCancel, input } = renderDialog();
|
||||
|
||||
await user.type(input, "wrong-name{Enter}");
|
||||
|
||||
expect(onConfirm).not.toHaveBeenCalled();
|
||||
expect(onCancel).not.toHaveBeenCalled();
|
||||
expect(input).toHaveFocus();
|
||||
expect(input).toHaveAttribute("aria-invalid", "true");
|
||||
expect(input).toHaveAccessibleDescription(
|
||||
"“wrong-name” does not match the name of this workspace",
|
||||
);
|
||||
expect(screen.getByRole("alert")).toHaveTextContent("does not match");
|
||||
});
|
||||
|
||||
it("clears the submitted error when the user edits the name", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { input } = renderDialog();
|
||||
|
||||
await user.type(input, "wrong-name{Enter}x");
|
||||
|
||||
expect(input).toHaveAttribute("aria-invalid", "false");
|
||||
expect(screen.queryByRole("alert")).toBeNull();
|
||||
});
|
||||
|
||||
it("confirms on Enter and keeps the typed name while the dialog stays open", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { onConfirm, input } = renderDialog();
|
||||
|
||||
await user.type(input, "my-workspace{Enter}");
|
||||
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1);
|
||||
expect(input).toHaveValue("my-workspace");
|
||||
});
|
||||
|
||||
it("starts empty when reopened after the parent closes it", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { onConfirm, input } = renderDialog({ closeOnConfirm: true });
|
||||
|
||||
await user.type(input, "my-workspace{Enter}");
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1);
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Reopen" }));
|
||||
|
||||
expect(
|
||||
await screen.findByLabelText("Name of the workspace to delete"),
|
||||
).toHaveValue("");
|
||||
expect(screen.getByRole("button", { name: "Delete" })).toBeDisabled();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { type FC, type FormEvent, useId, useState } from "react";
|
||||
import type { FC, FormEvent } from "react";
|
||||
import { Alert } from "#/components/Alert/Alert";
|
||||
import { Button } from "#/components/Button/Button";
|
||||
import {
|
||||
@@ -9,9 +9,11 @@ import {
|
||||
DialogHeader,
|
||||
DialogTitle,
|
||||
} from "#/components/Dialog/Dialog";
|
||||
import { Input } from "#/components/Input/Input";
|
||||
import { Label } from "#/components/Label/Label";
|
||||
import { Spinner } from "#/components/Spinner/Spinner";
|
||||
import {
|
||||
DeleteConfirmationField,
|
||||
useDeleteConfirmation,
|
||||
} from "./DeleteConfirmationField";
|
||||
|
||||
type DeleteDialogProps = {
|
||||
isOpen: boolean;
|
||||
@@ -41,31 +43,17 @@ export const DeleteDialog: FC<DeleteDialogProps> = ({
|
||||
label,
|
||||
confirmText = "Delete",
|
||||
}) => {
|
||||
const confirmId = useId();
|
||||
const errorId = `${confirmId}-error`;
|
||||
|
||||
const [userConfirmationText, setUserConfirmationText] = useState("");
|
||||
const [isFocused, setIsFocused] = useState(false);
|
||||
|
||||
const deletionConfirmed = name === userConfirmationText;
|
||||
const hasError = !deletionConfirmed && userConfirmationText.length > 0;
|
||||
const displayErrorMessage = hasError && !isFocused;
|
||||
|
||||
const resetConfirmation = () => {
|
||||
setUserConfirmationText("");
|
||||
setIsFocused(false);
|
||||
};
|
||||
const confirmation = useDeleteConfirmation(name, isOpen);
|
||||
|
||||
const handleOpenChange = (open: boolean) => {
|
||||
if (!open) {
|
||||
resetConfirmation();
|
||||
onCancel();
|
||||
}
|
||||
};
|
||||
|
||||
const onSubmit = (event: FormEvent<HTMLFormElement>) => {
|
||||
event.preventDefault();
|
||||
if (deletionConfirmed && !confirmLoading) {
|
||||
if (confirmation.confirmed && !confirmLoading) {
|
||||
onConfirm();
|
||||
}
|
||||
};
|
||||
@@ -93,30 +81,11 @@ export const DeleteDialog: FC<DeleteDialogProps> = ({
|
||||
</div>
|
||||
|
||||
<form className="flex flex-col gap-6" onSubmit={onSubmit}>
|
||||
<div className="flex flex-col gap-2">
|
||||
<Label htmlFor={confirmId}>
|
||||
{label ?? `Name of the ${entity} to delete`}
|
||||
</Label>
|
||||
<Input
|
||||
id={confirmId}
|
||||
name="confirmation"
|
||||
autoComplete="off"
|
||||
autoFocus
|
||||
placeholder={name}
|
||||
value={userConfirmationText}
|
||||
onChange={(event) => setUserConfirmationText(event.target.value)}
|
||||
onFocus={() => setIsFocused(true)}
|
||||
onBlur={() => setIsFocused(false)}
|
||||
aria-invalid={displayErrorMessage}
|
||||
aria-describedby={displayErrorMessage ? errorId : undefined}
|
||||
data-testid="delete-dialog-name-confirmation"
|
||||
/>
|
||||
{displayErrorMessage && (
|
||||
<span id={errorId} className="text-xs text-content-destructive">
|
||||
{userConfirmationText} does not match the name of this {entity}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<DeleteConfirmationField
|
||||
confirmation={confirmation}
|
||||
label={label ?? `Name of the ${entity} to delete`}
|
||||
entity={entity}
|
||||
/>
|
||||
|
||||
<DialogFooter>
|
||||
<Button
|
||||
@@ -130,7 +99,7 @@ export const DeleteDialog: FC<DeleteDialogProps> = ({
|
||||
<Button
|
||||
type="submit"
|
||||
variant="destructive"
|
||||
disabled={!deletionConfirmed || confirmLoading}
|
||||
disabled={!confirmation.confirmed || confirmLoading}
|
||||
data-testid="confirm-button"
|
||||
>
|
||||
<Spinner loading={confirmLoading} />
|
||||
|
||||
@@ -6,6 +6,7 @@ import type {
|
||||
MouseEventHandler,
|
||||
PropsWithChildren,
|
||||
} from "react";
|
||||
import { createPortal } from "react-dom";
|
||||
import { type ClickableAriaRole, useClickable } from "./useClickable";
|
||||
|
||||
/**
|
||||
@@ -86,7 +87,7 @@ describe(useClickable.name, () => {
|
||||
|
||||
await user.keyboard("[Tab][Tab]");
|
||||
await user.keyboard(" ");
|
||||
expect(mockCallback).toBeCalledTimes(1);
|
||||
expect(mockCallback).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("Allows an element to respond to clicks and Space/Enter, following all rules for native Button element interactions", async () => {
|
||||
@@ -97,13 +98,13 @@ describe(useClickable.name, () => {
|
||||
await user.click(document.body);
|
||||
await user.keyboard(" ");
|
||||
await user.keyboard("[Enter]");
|
||||
expect(mockCallback).not.toBeCalled();
|
||||
expect(mockCallback).not.toHaveBeenCalled();
|
||||
|
||||
const button = screen.getByRole("button");
|
||||
await user.click(button);
|
||||
await user.keyboard(" ");
|
||||
await user.keyboard("[Enter]");
|
||||
expect(mockCallback).toBeCalledTimes(3);
|
||||
expect(mockCallback).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("Will keep firing events if the Enter key is held down", async () => {
|
||||
@@ -115,7 +116,7 @@ describe(useClickable.name, () => {
|
||||
// (count determined by browser/library), and then release the Enter key
|
||||
const keydownCount = 5;
|
||||
await user.keyboard(`[Tab]{Enter>${keydownCount}}{/Enter}`);
|
||||
expect(mockCallback).toBeCalledTimes(keydownCount);
|
||||
expect(mockCallback).toHaveBeenCalledTimes(keydownCount);
|
||||
});
|
||||
|
||||
it("Will NOT keep firing events if the Space key is held down", async () => {
|
||||
@@ -125,11 +126,11 @@ describe(useClickable.name, () => {
|
||||
|
||||
// Focus over to element, and then hold down Space for 100 keydown cycles
|
||||
await user.keyboard("[Tab]{ >100}");
|
||||
expect(mockCallback).not.toBeCalled();
|
||||
expect(mockCallback).not.toHaveBeenCalled();
|
||||
|
||||
// Then explicitly release the space bar
|
||||
await user.keyboard("{/ }");
|
||||
expect(mockCallback).toBeCalledTimes(1);
|
||||
expect(mockCallback).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test("If focus is lost while Space is held down, then releasing the key will do nothing", async () => {
|
||||
@@ -148,6 +149,40 @@ describe(useClickable.name, () => {
|
||||
// Focus over to element, hold down Space for an indefinite amount of time,
|
||||
// move focus away from element, and then release Space
|
||||
await user.keyboard("[Tab]{ >}[Tab]{/ }");
|
||||
expect(mockCallback).not.toBeCalled();
|
||||
expect(mockCallback).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("Ignores Enter and Space from descendants, including portaled content", async () => {
|
||||
const mockCallback = vi.fn();
|
||||
const user = userEvent.setup();
|
||||
render(
|
||||
<NonNativeButton role="button" onInteraction={mockCallback}>
|
||||
<input aria-label="Inline input" />
|
||||
{createPortal(<input aria-label="Portaled input" />, document.body)}
|
||||
</NonNativeButton>,
|
||||
);
|
||||
|
||||
for (const name of ["Inline input", "Portaled input"]) {
|
||||
screen.getByLabelText(name).focus();
|
||||
await user.keyboard("a b{Enter}");
|
||||
}
|
||||
expect(mockCallback).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("Responds to clicks on inline descendants but not on portaled content", async () => {
|
||||
const mockCallback = vi.fn();
|
||||
const user = userEvent.setup();
|
||||
render(
|
||||
<NonNativeButton role="button" onInteraction={mockCallback}>
|
||||
<span>Inline content</span>
|
||||
{createPortal(<span>Portaled content</span>, document.body)}
|
||||
</NonNativeButton>,
|
||||
);
|
||||
|
||||
await user.click(screen.getByText("Portaled content"));
|
||||
expect(mockCallback).not.toHaveBeenCalled();
|
||||
|
||||
await user.click(screen.getByText("Inline content"));
|
||||
expect(mockCallback).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
type KeyboardEventHandler,
|
||||
type MouseEventHandler,
|
||||
type RefObject,
|
||||
type SyntheticEvent,
|
||||
useRef,
|
||||
} from "react";
|
||||
|
||||
@@ -31,6 +32,15 @@ export type UseClickableResult<
|
||||
onKeyUp: KeyboardEventHandler<TElement>;
|
||||
}>;
|
||||
|
||||
/**
|
||||
* True when the event's target is outside `currentTarget` in the DOM, as with
|
||||
* events React bubbles from portaled dialogs and menus.
|
||||
*/
|
||||
export const isFromPortal = (event: SyntheticEvent<unknown>): boolean =>
|
||||
event.currentTarget instanceof Node &&
|
||||
event.target instanceof Node &&
|
||||
!event.currentTarget.contains(event.target);
|
||||
|
||||
/**
|
||||
* Exposes props that let you turn traditionally non-interactive elements into
|
||||
* buttons.
|
||||
@@ -46,25 +56,32 @@ export const useClickable = <
|
||||
|
||||
return {
|
||||
ref,
|
||||
onClick,
|
||||
onClick: (event) => {
|
||||
if (!isFromPortal(event)) {
|
||||
onClick(event);
|
||||
}
|
||||
},
|
||||
tabIndex: 0,
|
||||
role: (role ?? "button") as TRole,
|
||||
|
||||
/*
|
||||
* Native buttons are programmed to handle both space and enter, but they're
|
||||
* each handled via different event handlers.
|
||||
*
|
||||
* 99% of the time, you shouldn't be able to tell the difference, but one
|
||||
* edge case behavior is that holding down Enter will continually fire
|
||||
* events, while holding down Space won't fire anything until you let go.
|
||||
* Mirrors native buttons: Enter activates on keydown (repeats while held),
|
||||
* Space on keyup. Keys typed into descendants, including portaled dialogs,
|
||||
* must not activate it.
|
||||
*/
|
||||
onKeyDown: (event) => {
|
||||
if (event.target !== event.currentTarget) {
|
||||
return;
|
||||
}
|
||||
if (event.key === "Enter") {
|
||||
ref.current?.click();
|
||||
event.stopPropagation();
|
||||
}
|
||||
},
|
||||
onKeyUp: (event) => {
|
||||
if (event.target !== event.currentTarget) {
|
||||
return;
|
||||
}
|
||||
if (event.key === " ") {
|
||||
ref.current?.click();
|
||||
event.stopPropagation();
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import type { FC, ReactNode } from "react";
|
||||
import { createPortal } from "react-dom";
|
||||
import { useClickableTableRow } from "./useClickableTableRow";
|
||||
|
||||
const Row: FC<{
|
||||
onClick: () => void;
|
||||
onMiddleClick: () => void;
|
||||
onDoubleClick?: () => void;
|
||||
children: ReactNode;
|
||||
}> = ({ onClick, onMiddleClick, onDoubleClick, children }) => {
|
||||
const { hover, ...rowProps } = useClickableTableRow({
|
||||
onClick,
|
||||
onMiddleClick,
|
||||
onDoubleClick,
|
||||
});
|
||||
return (
|
||||
<table>
|
||||
<tbody>
|
||||
<tr {...rowProps}>
|
||||
<td>{children}</td>
|
||||
</tr>
|
||||
</tbody>
|
||||
</table>
|
||||
);
|
||||
};
|
||||
|
||||
describe(useClickableTableRow.name, () => {
|
||||
it("handles double-clicks in the row but ignores double-clicks from portaled content", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onDoubleClick = vi.fn();
|
||||
render(
|
||||
<Row
|
||||
onClick={vi.fn()}
|
||||
onMiddleClick={vi.fn()}
|
||||
onDoubleClick={onDoubleClick}
|
||||
>
|
||||
<span>Row cell</span>
|
||||
{createPortal(<input aria-label="Dialog input" />, document.body)}
|
||||
</Row>,
|
||||
);
|
||||
|
||||
await user.dblClick(screen.getByLabelText("Dialog input"));
|
||||
expect(onDoubleClick).not.toHaveBeenCalled();
|
||||
|
||||
await user.dblClick(screen.getByText("Row cell"));
|
||||
expect(onDoubleClick).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("opens on middle-click in the row but ignores middle-clicks from portaled content", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onMiddleClick = vi.fn();
|
||||
render(
|
||||
<Row onClick={vi.fn()} onMiddleClick={onMiddleClick}>
|
||||
<span>Row cell</span>
|
||||
{createPortal(<input aria-label="Dialog input" />, document.body)}
|
||||
</Row>,
|
||||
);
|
||||
|
||||
await user.pointer({
|
||||
keys: "[MouseMiddle]",
|
||||
target: screen.getByLabelText("Dialog input"),
|
||||
});
|
||||
expect(onMiddleClick).not.toHaveBeenCalled();
|
||||
|
||||
await user.pointer({
|
||||
keys: "[MouseMiddle]",
|
||||
target: screen.getByText("Row cell"),
|
||||
});
|
||||
expect(onMiddleClick).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -18,6 +18,7 @@ import { cn } from "cn";
|
||||
import type { ComponentProps, MouseEventHandler } from "react";
|
||||
import {
|
||||
type ClickableAriaRole,
|
||||
isFromPortal,
|
||||
type UseClickableResult,
|
||||
useClickable,
|
||||
} from "./useClickable";
|
||||
@@ -61,8 +62,19 @@ export const useClickableTableRow = <
|
||||
"first:rounded-t-md last:rounded-b-md",
|
||||
]),
|
||||
hover: true,
|
||||
onDoubleClick,
|
||||
onDoubleClick:
|
||||
onDoubleClick &&
|
||||
((event) => {
|
||||
if (!isFromPortal(event)) {
|
||||
onDoubleClick(event);
|
||||
}
|
||||
}),
|
||||
onAuxClick: (event) => {
|
||||
// A middle-click paste (Linux) into a portaled dialog's input would
|
||||
// otherwise open the row's link in a new tab.
|
||||
if (isFromPortal(event)) {
|
||||
return;
|
||||
}
|
||||
// Regardless of which callback gets called, the hook won't stop the event
|
||||
// from bubbling further up the DOM
|
||||
const isMiddleMouseButton = event.button === 1;
|
||||
|
||||
+12
-12
@@ -1,5 +1,5 @@
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { expect, fn, userEvent, waitFor, within } from "storybook/test";
|
||||
import { expect, fn, userEvent, within } from "storybook/test";
|
||||
import { MockFailedWorkspace, MockWorkspace } from "#/testHelpers/entities";
|
||||
import { daysAgo } from "#/utils/time";
|
||||
import { WorkspaceDeleteDialog } from "./WorkspaceDeleteDialog";
|
||||
@@ -77,18 +77,18 @@ export const FilledWrong: Story = {
|
||||
const confirm = body.getByTestId("delete-dialog-name-confirmation");
|
||||
|
||||
await userEvent.type(confirm, "wrong-name");
|
||||
// Blur so the mismatch error becomes visible.
|
||||
await userEvent.tab();
|
||||
// The validation error renders asynchronously after blur, so wait for
|
||||
// visibility instead of asserting it once.
|
||||
await waitFor(
|
||||
() =>
|
||||
expect(
|
||||
body.getByText(
|
||||
"wrong-name does not match the name of this workspace",
|
||||
),
|
||||
).toBeVisible(),
|
||||
{ timeout: 5_000 },
|
||||
},
|
||||
};
|
||||
|
||||
// Shows the error while the input still has focus after pressing Enter.
|
||||
export const FilledWrongSubmitted: Story = {
|
||||
play: async ({ canvasElement }) => {
|
||||
const body = within(canvasElement.ownerDocument.body);
|
||||
await userEvent.type(
|
||||
body.getByTestId("delete-dialog-name-confirmation"),
|
||||
"wrong name{Enter}",
|
||||
);
|
||||
await expect(body.getByRole("button", { name: "Delete" })).toBeDisabled();
|
||||
},
|
||||
};
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
import { screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { type FC, useState } from "react";
|
||||
import type { Workspace } from "#/api/typesGenerated";
|
||||
import { MockFailedWorkspace, MockWorkspace } from "#/testHelpers/entities";
|
||||
import { render } from "#/testHelpers/renderHelpers";
|
||||
import { WorkspaceDeleteDialog } from "./WorkspaceDeleteDialog";
|
||||
|
||||
type HarnessProps = {
|
||||
workspace: Workspace;
|
||||
canDeleteFailedWorkspace: boolean;
|
||||
onConfirm: (orphan: boolean | undefined) => void;
|
||||
onCancel: () => void;
|
||||
};
|
||||
|
||||
// Like WorkspaceMoreActions, the dialog stays mounted while closed.
|
||||
const Harness: FC<HarnessProps> = ({ onConfirm, onCancel, ...props }) => {
|
||||
const [isOpen, setIsOpen] = useState(true);
|
||||
return (
|
||||
<>
|
||||
<button type="button" onClick={() => setIsOpen(true)}>
|
||||
Reopen
|
||||
</button>
|
||||
<WorkspaceDeleteDialog
|
||||
{...props}
|
||||
isOpen={isOpen}
|
||||
onCancel={() => {
|
||||
setIsOpen(false);
|
||||
onCancel();
|
||||
}}
|
||||
onConfirm={(orphan) => {
|
||||
setIsOpen(false);
|
||||
onConfirm(orphan);
|
||||
}}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
const renderDialog = (
|
||||
workspace: Workspace = MockWorkspace,
|
||||
canDeleteFailedWorkspace = false,
|
||||
) => {
|
||||
const onConfirm = vi.fn();
|
||||
const onCancel = vi.fn();
|
||||
|
||||
render(
|
||||
<Harness
|
||||
workspace={workspace}
|
||||
canDeleteFailedWorkspace={canDeleteFailedWorkspace}
|
||||
onConfirm={onConfirm}
|
||||
onCancel={onCancel}
|
||||
/>,
|
||||
);
|
||||
|
||||
return { onConfirm, onCancel };
|
||||
};
|
||||
|
||||
const getInput = () => screen.getByLabelText("Workspace name");
|
||||
|
||||
const reopen = async (user: ReturnType<typeof userEvent.setup>) => {
|
||||
await user.click(screen.getByRole("button", { name: "Reopen" }));
|
||||
return screen.findByLabelText("Workspace name");
|
||||
};
|
||||
|
||||
describe("WorkspaceDeleteDialog", () => {
|
||||
it("marks the input invalid instead of confirming when Enter is pressed with a wrong name", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { onConfirm, onCancel } = renderDialog();
|
||||
|
||||
const input = getInput();
|
||||
await user.type(input, "wrong name{Enter}");
|
||||
|
||||
expect(onConfirm).not.toHaveBeenCalled();
|
||||
expect(onCancel).not.toHaveBeenCalled();
|
||||
expect(input).toHaveFocus();
|
||||
expect(input).toHaveAttribute("aria-invalid", "true");
|
||||
expect(input).toHaveAccessibleDescription(
|
||||
"“wrong name” does not match the name of this workspace",
|
||||
);
|
||||
expect(screen.getByRole("alert")).toHaveTextContent("does not match");
|
||||
});
|
||||
|
||||
it("clears the submitted error when the user edits the name", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderDialog();
|
||||
|
||||
const input = getInput();
|
||||
await user.type(input, "wrong name{Enter}x");
|
||||
|
||||
expect(input).toHaveAttribute("aria-invalid", "false");
|
||||
expect(screen.queryByRole("alert")).toBeNull();
|
||||
});
|
||||
|
||||
it("starts empty when reopened after cancelling with the correct name typed", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { onCancel } = renderDialog();
|
||||
|
||||
await user.type(getInput(), MockWorkspace.name);
|
||||
await user.keyboard("{Escape}");
|
||||
expect(onCancel).toHaveBeenCalledTimes(1);
|
||||
|
||||
const input = await reopen(user);
|
||||
expect(input).toHaveValue("");
|
||||
expect(screen.getByRole("button", { name: "Delete" })).toBeDisabled();
|
||||
});
|
||||
|
||||
it("confirms on Enter and starts empty when reopened", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { onConfirm } = renderDialog();
|
||||
|
||||
await user.type(getInput(), `${MockWorkspace.name}{Enter}`);
|
||||
expect(onConfirm).toHaveBeenCalledExactlyOnceWith(false);
|
||||
|
||||
const input = await reopen(user);
|
||||
expect(input).toHaveValue("");
|
||||
expect(screen.getByRole("button", { name: "Delete" })).toBeDisabled();
|
||||
});
|
||||
|
||||
it("unchecks Orphan Resources when reopened", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderDialog(MockFailedWorkspace, true);
|
||||
|
||||
await user.click(
|
||||
screen.getByRole("checkbox", { name: /Orphan Resources/ }),
|
||||
);
|
||||
await user.keyboard("{Escape}");
|
||||
await reopen(user);
|
||||
|
||||
expect(
|
||||
screen.getByRole("checkbox", { name: /Orphan Resources/ }),
|
||||
).not.toBeChecked();
|
||||
});
|
||||
});
|
||||
@@ -6,8 +6,10 @@ import type {
|
||||
} from "#/api/typesGenerated";
|
||||
import { Checkbox } from "#/components/Checkbox/Checkbox";
|
||||
import { ConfirmDialog } from "#/components/Dialog/ConfirmDialog/ConfirmDialog";
|
||||
import { Input } from "#/components/Input/Input";
|
||||
import { Label } from "#/components/Label/Label";
|
||||
import {
|
||||
DeleteConfirmationField,
|
||||
useDeleteConfirmation,
|
||||
} from "#/components/Dialog/DeleteDialog/DeleteConfirmationField";
|
||||
import { Link } from "#/components/Link/Link";
|
||||
import { docs } from "#/utils/docs";
|
||||
|
||||
@@ -29,23 +31,27 @@ export const WorkspaceDeleteDialog: FC<WorkspaceDeleteDialogProps> = ({
|
||||
onCancel,
|
||||
onConfirm,
|
||||
}) => {
|
||||
const confirmId = useId();
|
||||
const errorId = `${confirmId}-error`;
|
||||
const orphanId = `${confirmId}-orphan`;
|
||||
const orphanId = useId();
|
||||
|
||||
const [userConfirmationText, setUserConfirmationText] = useState("");
|
||||
const confirmation = useDeleteConfirmation(workspace.name, isOpen);
|
||||
const [orphanWorkspace, setOrphanWorkspace] =
|
||||
useState<CreateWorkspaceBuildRequest["orphan"]>(false);
|
||||
const [isFocused, setIsFocused] = useState(false);
|
||||
const [prevIsOpen, setPrevIsOpen] = useState(isOpen);
|
||||
|
||||
const deletionConfirmed = workspace.name === userConfirmationText;
|
||||
const hasError = !deletionConfirmed && userConfirmationText.length > 0;
|
||||
const displayErrorMessage = hasError && !isFocused;
|
||||
// The dialog stays mounted while closed, so clear the checkbox on close.
|
||||
if (isOpen !== prevIsOpen) {
|
||||
setPrevIsOpen(isOpen);
|
||||
if (!isOpen) {
|
||||
setOrphanWorkspace(false);
|
||||
}
|
||||
}
|
||||
|
||||
const confirm = () => onConfirm(orphanWorkspace);
|
||||
|
||||
const onSubmit = (event: FormEvent) => {
|
||||
event.preventDefault();
|
||||
if (deletionConfirmed) {
|
||||
onConfirm(orphanWorkspace);
|
||||
if (confirmation.confirmed) {
|
||||
confirm();
|
||||
}
|
||||
};
|
||||
|
||||
@@ -68,9 +74,9 @@ export const WorkspaceDeleteDialog: FC<WorkspaceDeleteDialogProps> = ({
|
||||
hideCancel={false}
|
||||
open={isOpen}
|
||||
title="Delete Workspace"
|
||||
onConfirm={() => onConfirm(orphanWorkspace)}
|
||||
onConfirm={confirm}
|
||||
onClose={onCancel}
|
||||
disabled={!deletionConfirmed}
|
||||
disabled={!confirmation.confirmed}
|
||||
description={
|
||||
<>
|
||||
<div className="flex items-center justify-between rounded-md border border-solid border-border p-4 mb-5 leading-snug">
|
||||
@@ -95,27 +101,11 @@ export const WorkspaceDeleteDialog: FC<WorkspaceDeleteDialogProps> = ({
|
||||
</p>
|
||||
|
||||
<form className="mt-2 flex flex-col gap-2" onSubmit={onSubmit}>
|
||||
<Label htmlFor={confirmId}>Workspace name</Label>
|
||||
<Input
|
||||
id={confirmId}
|
||||
className="text-content-primary"
|
||||
name="confirmation"
|
||||
autoComplete="off"
|
||||
autoFocus
|
||||
placeholder={workspace.name}
|
||||
value={userConfirmationText}
|
||||
onChange={(event) => setUserConfirmationText(event.target.value)}
|
||||
onFocus={() => setIsFocused(true)}
|
||||
onBlur={() => setIsFocused(false)}
|
||||
aria-invalid={displayErrorMessage}
|
||||
aria-describedby={displayErrorMessage ? errorId : undefined}
|
||||
data-testid="delete-dialog-name-confirmation"
|
||||
<DeleteConfirmationField
|
||||
confirmation={confirmation}
|
||||
label="Workspace name"
|
||||
entity="workspace"
|
||||
/>
|
||||
{displayErrorMessage && (
|
||||
<span id={errorId} className="text-xs text-content-destructive">
|
||||
{userConfirmationText} does not match the name of this workspace
|
||||
</span>
|
||||
)}
|
||||
|
||||
{canOrphan && (
|
||||
<div className={warnBoxClassName}>
|
||||
|
||||
+2
-3
@@ -58,9 +58,8 @@ const Example: Story = {
|
||||
export { Example as WorkspaceOutdatedTooltip };
|
||||
|
||||
// Regression coverage for the `useClickableTableRow` usage on the workspaces
|
||||
// list. The trigger must stop click + keyboard propagation so the popover
|
||||
// opens instead of the parent row's onClick swallowing the activation and
|
||||
// navigating away.
|
||||
// list. The trigger must stop click propagation so the popover opens instead
|
||||
// of the parent row's onClick swallowing the activation and navigating away.
|
||||
type ClickableRowArgs = ComponentProps<typeof WorkspaceOutdatedTooltip> & {
|
||||
onRowClick: () => void;
|
||||
};
|
||||
|
||||
@@ -34,9 +34,9 @@ export const WorkspaceOutdatedTooltip: FC<WorkspaceOutdatedTooltipProps> = ({
|
||||
}) => {
|
||||
const [isOpen, setIsOpen] = useState(false);
|
||||
|
||||
// Stop activation from bubbling to a parent `useClickableTableRow` row,
|
||||
// which navigates on click, Enter (onKeyDown), and Space (onKeyUp). Radix
|
||||
// composes its own click handler, so the popover still opens.
|
||||
// Keep trigger clicks from triggering a parent clickable row's navigation.
|
||||
// Radix composes its own click handler, so the popover still opens. The
|
||||
// span's key handlers only satisfy Biome's lint/a11y/useKeyWithClickEvents.
|
||||
const stopPropagation = (event: React.SyntheticEvent) => {
|
||||
event.stopPropagation();
|
||||
};
|
||||
@@ -60,8 +60,6 @@ export const WorkspaceOutdatedTooltip: FC<WorkspaceOutdatedTooltipProps> = ({
|
||||
size="small"
|
||||
hoverEffect={false}
|
||||
onClick={stopPropagation}
|
||||
onKeyDown={stopPropagation}
|
||||
onKeyUp={stopPropagation}
|
||||
>
|
||||
<CircleAlertIcon className="text-content-secondary" />
|
||||
<span className="sr-only">Outdated info</span>
|
||||
|
||||
@@ -47,7 +47,7 @@ export const ModelRow: FC<ModelRowProps> = ({
|
||||
? "The provider connected to this model is disabled."
|
||||
: null;
|
||||
|
||||
// Keep tooltip activation from triggering the clickable row's navigation.
|
||||
// Keep tooltip clicks from triggering the clickable row's navigation.
|
||||
const stopPropagation = (event: React.SyntheticEvent) => {
|
||||
event.stopPropagation();
|
||||
};
|
||||
@@ -97,8 +97,6 @@ export const ModelRow: FC<ModelRowProps> = ({
|
||||
variant="warning"
|
||||
className="shrink-0"
|
||||
onClick={stopPropagation}
|
||||
onKeyDown={stopPropagation}
|
||||
onKeyUp={stopPropagation}
|
||||
>
|
||||
<button type="button">Unavailable</button>
|
||||
</Badge>
|
||||
|
||||
@@ -32,10 +32,8 @@ export const ProviderRow: React.FC<ProviderRowProps> = ({
|
||||
const displayName = provider.display_name || provider.name;
|
||||
const disabled = !provider.enabled;
|
||||
|
||||
// Stop activation from bubbling to a parent `useClickableTableRow`
|
||||
// row, which navigates on click, Enter (onKeyDown), and Space
|
||||
// (onKeyUp). Radix composes its own click handler, so the tooltip
|
||||
// still opens.
|
||||
// Keep tooltip clicks from triggering the clickable row's navigation. Radix
|
||||
// composes its own click handler, so the tooltip still opens.
|
||||
const stopPropagation = (event: React.SyntheticEvent) => {
|
||||
event.stopPropagation();
|
||||
};
|
||||
@@ -90,13 +88,7 @@ export const ProviderRow: React.FC<ProviderRowProps> = ({
|
||||
{AgentsUnsupportedProviderTypes.some((t) => t === provider.type) && (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Badge
|
||||
asChild
|
||||
variant="info"
|
||||
onClick={stopPropagation}
|
||||
onKeyDown={stopPropagation}
|
||||
onKeyUp={stopPropagation}
|
||||
>
|
||||
<Badge asChild variant="info" onClick={stopPropagation}>
|
||||
<button type="button">Not supported in Agents</button>
|
||||
</Badge>
|
||||
</TooltipTrigger>
|
||||
@@ -109,13 +101,7 @@ export const ProviderRow: React.FC<ProviderRowProps> = ({
|
||||
{provider.status?.warnings && provider.status.warnings.length > 0 && (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Badge
|
||||
asChild
|
||||
variant="warning"
|
||||
onClick={stopPropagation}
|
||||
onKeyDown={stopPropagation}
|
||||
onKeyUp={stopPropagation}
|
||||
>
|
||||
<Badge asChild variant="warning" onClick={stopPropagation}>
|
||||
<button
|
||||
type="button"
|
||||
aria-label={`Warning: ${provider.status.warnings.join("; ")}`}
|
||||
|
||||
Reference in New Issue
Block a user