diff --git a/site/src/components/Dialog/DeleteDialog/DeleteConfirmationField.tsx b/site/src/components/Dialog/DeleteDialog/DeleteConfirmationField.tsx new file mode 100644 index 0000000000..374b6ffdf1 --- /dev/null +++ b/site/src/components/Dialog/DeleteDialog/DeleteConfirmationField.tsx @@ -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) => { + setValue(event.target.value); + setHasSubmittedInvalid(false); + }, + onFocus: () => setIsFocused(true), + onBlur: () => setIsFocused(false), + onKeyDown: (event: KeyboardEvent) => { + // 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; + label: ReactNode; + entity: string; +}; + +export const DeleteConfirmationField: FC = ({ + confirmation, + label, + entity, +}) => { + const inputId = useId(); + const errorId = `${inputId}-error`; + const { name, value, showError, inputProps } = confirmation; + + return ( +
+ + + {showError && ( + + “{value}” does not match the name of this {entity} + + )} +
+ ); +}; diff --git a/site/src/components/Dialog/DeleteDialog/DeleteDialog.stories.tsx b/site/src/components/Dialog/DeleteDialog/DeleteDialog.stories.tsx index 9c877fd0cc..1745890c4d 100644 --- a/site/src/components/Dialog/DeleteDialog/DeleteDialog.stories.tsx +++ b/site/src/components/Dialog/DeleteDialog/DeleteDialog.stories.tsx @@ -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}", + ); }, }; diff --git a/site/src/components/Dialog/DeleteDialog/DeleteDialog.test.tsx b/site/src/components/Dialog/DeleteDialog/DeleteDialog.test.tsx new file mode 100644 index 0000000000..47a79842a0 --- /dev/null +++ b/site/src/components/Dialog/DeleteDialog/DeleteDialog.test.tsx @@ -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 = ({ onConfirm, onCancel, closeOnConfirm }) => { + const [isOpen, setIsOpen] = useState(true); + return ( + <> + + { + 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( + , + ); + + 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(); + }); +}); diff --git a/site/src/components/Dialog/DeleteDialog/DeleteDialog.tsx b/site/src/components/Dialog/DeleteDialog/DeleteDialog.tsx index e633ef374b..c3db7addf3 100644 --- a/site/src/components/Dialog/DeleteDialog/DeleteDialog.tsx +++ b/site/src/components/Dialog/DeleteDialog/DeleteDialog.tsx @@ -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 = ({ 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) => { event.preventDefault(); - if (deletionConfirmed && !confirmLoading) { + if (confirmation.confirmed && !confirmLoading) { onConfirm(); } }; @@ -93,30 +81,11 @@ export const DeleteDialog: FC = ({
-
- - 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 && ( - - {userConfirmationText} does not match the name of this {entity} - - )} -
+ + { + 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( + , + ); + + return { onConfirm, onCancel }; +}; + +const getInput = () => screen.getByLabelText("Workspace name"); + +const reopen = async (user: ReturnType) => { + 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(); + }); +}); diff --git a/site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceDeleteDialog.tsx b/site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceDeleteDialog.tsx index 513512f258..c7755dd163 100644 --- a/site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceDeleteDialog.tsx +++ b/site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceDeleteDialog.tsx @@ -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 = ({ 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(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 = ({ hideCancel={false} open={isOpen} title="Delete Workspace" - onConfirm={() => onConfirm(orphanWorkspace)} + onConfirm={confirm} onClose={onCancel} - disabled={!deletionConfirmed} + disabled={!confirmation.confirmed} description={ <>
@@ -95,27 +101,11 @@ export const WorkspaceDeleteDialog: FC = ({

- - 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 && ( - - {userConfirmationText} does not match the name of this workspace - - )} {canOrphan && (
diff --git a/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.stories.tsx b/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.stories.tsx index 93b4c4b701..7760c8d51c 100644 --- a/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.stories.tsx +++ b/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.stories.tsx @@ -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 & { onRowClick: () => void; }; diff --git a/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx b/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx index 941da4eb8a..50e5fb2534 100644 --- a/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx +++ b/site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx @@ -34,9 +34,9 @@ export const WorkspaceOutdatedTooltip: FC = ({ }) => { 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 = ({ size="small" hoverEffect={false} onClick={stopPropagation} - onKeyDown={stopPropagation} - onKeyUp={stopPropagation} > Outdated info diff --git a/site/src/pages/AISettingsPage/ModelsPage/components/ModelRow.tsx b/site/src/pages/AISettingsPage/ModelsPage/components/ModelRow.tsx index d812989942..097360b191 100644 --- a/site/src/pages/AISettingsPage/ModelsPage/components/ModelRow.tsx +++ b/site/src/pages/AISettingsPage/ModelsPage/components/ModelRow.tsx @@ -47,7 +47,7 @@ export const ModelRow: FC = ({ ? "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 = ({ variant="warning" className="shrink-0" onClick={stopPropagation} - onKeyDown={stopPropagation} - onKeyUp={stopPropagation} > diff --git a/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderRow.tsx b/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderRow.tsx index 80703630c5..45a417d93d 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderRow.tsx +++ b/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderRow.tsx @@ -32,10 +32,8 @@ export const ProviderRow: React.FC = ({ 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 = ({ {AgentsUnsupportedProviderTypes.some((t) => t === provider.type) && ( - + @@ -109,13 +101,7 @@ export const ProviderRow: React.FC = ({ {provider.status?.warnings && provider.status.warnings.length > 0 && ( - +