Skip to content

refactor: use the new button component on forms and dialogs #16050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Jan 7, 2025
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
16 changes: 8 additions & 8 deletions site/e2e/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export const createWorkspace = async (
await popup.waitForSelector("text=You are now authenticated.");
}

await page.getByTestId("form-submit").click();
await page.getByRole("button", { name: /create workspace/i }).click();

const user = currentUser(page);

Expand Down Expand Up @@ -276,7 +276,7 @@ export const createTemplate = async (

const name = randomName();
await page.getByLabel("Name *").fill(name);
await page.getByTestId("form-submit").click();
await page.getByRole("button", { name: /save/i }).click();
await expectUrl(page).toHavePathName(
organizationsEnabled
? `/templates/${orgName}/${name}/files`
Expand All @@ -298,7 +298,7 @@ export const createGroup = async (page: Page): Promise<string> => {

const name = randomName();
await page.getByLabel("Name", { exact: true }).fill(name);
await page.getByTestId("form-submit").click();
await page.getByRole("button", { name: /save/i }).click();
await expectUrl(page).toHavePathName(`/groups/${name}`);
return name;
};
Expand Down Expand Up @@ -982,7 +982,7 @@ export const updateTemplateSettings = async (
await page.getByLabel(labelText, { exact: true }).fill(value);
}

await page.getByTestId("form-submit").click();
await page.getByRole("button", { name: /save/i }).click();

const name = templateSettingValues.name ?? templateName;
await expectUrl(page).toHavePathNameEndingWith(`/${name}`);
Expand All @@ -1003,7 +1003,7 @@ export const updateWorkspace = async (
await page.getByTestId("confirm-button").click();

await fillParameters(page, richParameters, buildParameters);
await page.getByTestId("form-submit").click();
await page.getByRole("button", { name: /update parameters/i }).click();

await page.waitForSelector("*[data-testid='build-status'] >> text=Running", {
state: "visible",
Expand All @@ -1024,7 +1024,7 @@ export const updateWorkspaceParameters = async (
);

await fillParameters(page, richParameters, buildParameters);
await page.getByTestId("form-submit").click();
await page.getByRole("button", { name: /submit and restart/i }).click();

await page.waitForSelector("*[data-testid='build-status'] >> text=Running", {
state: "visible",
Expand Down Expand Up @@ -1091,7 +1091,7 @@ export async function createUser(
// as the label for the currently active option.
const passwordField = page.locator("input[name=password]");
await passwordField.fill(password);
await page.getByRole("button", { name: "Create user" }).click();
await page.getByRole("button", { name: /save/i }).click();
await expect(page.getByText("Successfully created user.")).toBeVisible();

await expect(page).toHaveTitle("Users - Coder");
Expand Down Expand Up @@ -1123,7 +1123,7 @@ export async function createOrganization(page: Page): Promise<{
const description = `Org description ${name}`;
await page.getByLabel("Description").fill(description);
await page.getByLabel("Icon", { exact: true }).fill("/emojis/1f957.png");
await page.getByRole("button", { name: "Submit" }).click();
await page.getByRole("button", { name: /save/i }).click();

await expectUrl(page).toHavePathName(`/organizations/${name}`);
await expect(page.getByText("Organization created.")).toBeVisible();
Expand Down
4 changes: 2 additions & 2 deletions site/e2e/tests/deployment/idpOrgSync.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ test.describe("IdpOrgSyncPage", () => {
const syncField = page.getByRole("textbox", {
name: "Organization sync field",
});
const saveButton = page.getByRole("button", { name: "Save" }).first();
const saveButton = page.getByRole("button", { name: /save/i }).first();

await expect(saveButton).toBeDisabled();

await syncField.fill("test-field");
await expect(saveButton).toBeEnabled();

await page.getByRole("button", { name: "Save" }).click();
await page.getByRole("button", { name: /save/i }).click();

await expect(
page.getByText("Organization sync settings updated."),
Expand Down
2 changes: 1 addition & 1 deletion site/e2e/tests/groups/createGroup.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ test("create group", async ({ page, baseURL }) => {
await page.getByLabel("Name", { exact: true }).fill(groupValues.name);
await page.getByLabel("Display Name").fill(groupValues.displayName);
await page.getByLabel("Avatar URL").fill(groupValues.avatarURL);
await page.getByRole("button", { name: "Submit" }).click();
await page.getByRole("button", { name: /save/i }).click();

await expect(page).toHaveTitle(`${groupValues.displayName} - Coder`);
await expect(page.getByText(groupValues.displayName)).toBeVisible();
Expand Down
4 changes: 2 additions & 2 deletions site/e2e/tests/organizationGroups.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ test("create group", async ({ page }) => {
const displayName = `Group ${name}`;
await page.getByLabel("Display Name").fill(displayName);
await page.getByLabel("Avatar URL").fill("/emojis/1f60d.png");
await page.getByRole("button", { name: "Submit" }).click();
await page.getByRole("button", { name: /save/i }).click();

await expectUrl(page).toHavePathName(
`/organizations/${org.name}/groups/${name}`,
Expand Down Expand Up @@ -91,7 +91,7 @@ test("change quota settings", async ({ page }) => {

// Update Quota
await page.getByLabel("Quota Allowance").fill("100");
await page.getByRole("button", { name: "Submit" }).click();
await page.getByRole("button", { name: /save/i }).click();

// We should get sent back to the group page afterwards
expectUrl(page).toHavePathName(
Expand Down
4 changes: 2 additions & 2 deletions site/e2e/tests/organizations.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ test("create and delete organization", async ({ page }) => {
await page.getByLabel("Display name").fill(`Org ${name}`);
await page.getByLabel("Description").fill(`Org description ${name}`);
await page.getByLabel("Icon", { exact: true }).fill("/emojis/1f957.png");
await page.getByRole("button", { name: "Submit" }).click();
await page.getByRole("button", { name: /save/i }).click();

// Expect to be redirected to the new organization
await expectUrl(page).toHavePathName(`/organizations/${name}`);
Expand All @@ -32,7 +32,7 @@ test("create and delete organization", async ({ page }) => {
const newName = randomName();
await page.getByLabel("Slug").fill(newName);
await page.getByLabel("Description").fill(`Org description ${newName}`);
await page.getByRole("button", { name: "Submit" }).click();
await page.getByRole("button", { name: /save/i }).click();

// Expect to be redirected when renaming the organization
await expectUrl(page).toHavePathName(`/organizations/${newName}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ test.describe("CustomRolesPage", () => {
await expect(organizationMemberCheckbox).toBeVisible();
await organizationMemberCheckbox.click();

const saveButton = page.getByRole("button", { name: "Save" }).first();
const saveButton = page.getByRole("button", { name: /save/i }).first();
await expect(saveButton).toBeVisible();
await saveButton.click();

Expand Down
2 changes: 1 addition & 1 deletion site/e2e/tests/templates/updateTemplateSchedule.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ test("update template schedule settings without override other settings", async
waitUntil: "domcontentloaded",
});
await page.getByLabel("Default autostop (hours)").fill("48");
await page.getByRole("button", { name: "Submit" }).click();
await page.getByRole("button", { name: /save/i }).click();
await expect(page.getByText("Template updated successfully")).toBeVisible();

const updatedTemplate = await API.getTemplate(template.id);
Expand Down
2 changes: 1 addition & 1 deletion site/e2e/tests/updateTemplate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ test("require latest version", async ({ page }) => {
await expectUrl(page).toHavePathName(`/templates/${templateName}/settings`);
let checkbox = await page.waitForSelector("#require_active_version");
await checkbox.click();
await page.getByTestId("form-submit").click();
await page.getByRole("button", { name: /save/i }).click();

await page.goto(`/templates/${templateName}/settings`, {
waitUntil: "domcontentloaded",
Expand Down
63 changes: 49 additions & 14 deletions site/src/components/Button/Button.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Meta, StoryObj } from "@storybook/react";
import { Trash } from "lucide-react";
import { PlusIcon } from "lucide-react";
import { Button } from "./Button";

const meta: Meta<typeof Button> = {
Expand All @@ -8,7 +8,7 @@ const meta: Meta<typeof Button> = {
args: {
children: (
<>
<Trash />
<PlusIcon />
Button
</>
),
Expand All @@ -20,34 +20,41 @@ type Story = StoryObj<typeof Button>;

export const Default: Story = {};

export const Outline: Story = {
export const DefaultDisabled: Story = {
args: {
variant: "outline",
disabled: true,
},
};

export const Subtle: Story = {
export const DefaultSmall: Story = {
args: {
variant: "subtle",
size: "sm",
},
};

export const Warning: Story = {
export const Outline: Story = {
args: {
variant: "warning",
variant: "outline",
},
};

export const DefaultDisabled: Story = {
export const OutlineDisabled: Story = {
args: {
variant: "outline",
disabled: true,
},
};

export const OutlineDisabled: Story = {
export const OutlineSmall: Story = {
args: {
variant: "outline",
disabled: true,
size: "sm",
},
};

export const Subtle: Story = {
args: {
variant: "subtle",
},
};

Expand All @@ -58,23 +65,51 @@ export const SubtleDisabled: Story = {
},
};

export const SubtleSmall: Story = {
args: {
variant: "subtle",
size: "sm",
},
};

export const Destructive: Story = {
args: {
variant: "destructive",
children: "Delete",
},
};

export const DestructiveDisabled: Story = {
args: {
...Destructive.args,
disabled: true,
},
};

export const DestructiveSmall: Story = {
args: {
...Destructive.args,
size: "sm",
},
};

export const IconButtonDefault: Story = {
args: {
variant: "default",
children: <Trash />,
children: <PlusIcon />,
},
};

export const IconButtonOutline: Story = {
args: {
variant: "outline",
children: <Trash />,
children: <PlusIcon />,
},
};

export const IconButtonSubtle: Story = {
args: {
variant: "subtle",
children: <Trash />,
children: <PlusIcon />,
},
};
23 changes: 9 additions & 14 deletions site/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,38 +8,33 @@ import { forwardRef } from "react";
import { cn } from "utils/cn";

export const buttonVariants = cva(
`inline-flex items-center justify-center gap-2 whitespace-nowrap
`inline-flex items-center justify-center gap-1 whitespace-nowrap
border-solid rounded-md transition-colors
text-sm font-semibold font-medium cursor-pointer
text-sm font-semibold font-medium cursor-pointer no-underline
focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-content-link
disabled:pointer-events-none disabled:text-content-disabled
[&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0
px-3 py-2`,
[&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg]:p-[2px]`,
{
variants: {
variant: {
default:
"bg-surface-invert-primary text-content-invert hover:bg-surface-invert-secondary border-none disabled:bg-surface-secondary",
"bg-surface-invert-primary text-content-invert hover:bg-surface-invert-secondary border-none disabled:bg-surface-secondary font-semibold",
outline:
"border border-border-default text-content-primary bg-transparent hover:bg-surface-secondary",
subtle:
"border-none bg-transparent text-content-secondary hover:text-content-primary",
warning:
"border border-border-error text-content-primary bg-surface-error hover:bg-transparent",
ghost:
"text-content-primary bg-transparent border-0 hover:bg-surface-secondary",
destructive:
"border border-border-destructive text-content-primary bg-surface-destructive hover:bg-transparent disabled:bg-transparent disabled:text-content-disabled font-semibold",
},

size: {
lg: "h-10",
default: "h-9",
sm: "h-8 px-2 py-1.5 text-xs",
icon: "h-10 w-10",
lg: "h-10 px-3 py-2 [&_svg]:size-icon-lg",
sm: "h-[30px] px-2 py-1.5 text-xs [&_svg]:size-icon-sm",
},
},
defaultVariants: {
variant: "default",
size: "default",
size: "lg",
},
},
);
Expand Down
24 changes: 21 additions & 3 deletions site/src/components/Dialogs/DeleteDialog/DeleteDialog.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { action } from "@storybook/addon-actions";
import type { Meta, StoryObj } from "@storybook/react";
import { userEvent } from "@storybook/test";
import { within } from "@testing-library/react";
import { DeleteDialog } from "./DeleteDialog";

const meta: Meta<typeof DeleteDialog> = {
Expand All @@ -19,12 +21,28 @@ export default meta;

type Story = StoryObj<typeof DeleteDialog>;

const Example: Story = {};
export const Idle: Story = {};

export const FilledSuccessfully: Story = {
play: async ({ canvasElement }) => {
const user = userEvent.setup();
const body = within(canvasElement.ownerDocument.body);
const input = await body.findByLabelText("Name of the foo to delete");
await user.type(input, "MyFoo");
},
};

export const FilledWrong: Story = {
play: async ({ canvasElement }) => {
const user = userEvent.setup();
const body = within(canvasElement.ownerDocument.body);
const input = await body.findByLabelText("Name of the foo to delete");
await user.type(input, "InvalidFooName");
},
};

export const Loading: Story = {
args: {
confirmLoading: true,
},
};

export { Example as DeleteDialog };
Loading
Loading