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
10 changes: 10 additions & 0 deletions site/src/components/Avatar/AvatarData.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,13 @@ export const WithImage: Story = {
src: "https://avatars.githubusercontent.com/u/95932066?s=200&v=4",
},
};

export const WithLongTitle: Story = {
args: {
truncate: true,
title: "a-workspace-with-an-unreasonably-long-name-that-should-be-clipped",
subtitle:
"and-an-even-longer-organization-or-template-subtitle-that-truncates",
},
decorators: [(Story) => <div style={{ maxWidth: 240 }}>{Story()}</div>],
};
27 changes: 24 additions & 3 deletions site/src/components/Avatar/AvatarData.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { FC, ReactNode } from "react";
import { Avatar } from "#/components/Avatar/Avatar";
import { cn } from "#/utils/cn";

interface AvatarDataProps {
title: ReactNode;
Expand All @@ -15,6 +16,13 @@ interface AvatarDataProps {
* from the title prop if it is a string.
*/
imgFallbackText?: string;

/**
* When true, the title and subtitle clip with an ellipsis if they overflow
* the available width. Off by default because callers that pass non-text
* nodes (icons, badges) as `title` would otherwise clip silently.
*/
truncate?: boolean;
}

export const AvatarData: FC<AvatarDataProps> = ({
Expand All @@ -23,6 +31,7 @@ export const AvatarData: FC<AvatarDataProps> = ({
src,
imgFallbackText,
avatar,
truncate = false,
}) => {
if (!avatar) {
avatar = (
Expand All @@ -38,12 +47,24 @@ export const AvatarData: FC<AvatarDataProps> = ({
<div className="flex items-center gap-3">
{avatar}

<div className="flex flex-col">
<span className="text-sm font-semibold text-content-primary">
<div
className={cn("flex flex-col", truncate && "flex-1 overflow-hidden")}
>
<span
className={cn(
"text-sm font-semibold text-content-primary",
truncate && "truncate",
)}
>
{title}
</span>
{subtitle && (
<span className="text-content-secondary text-xs font-medium">
<span
className={cn(
"text-content-secondary text-xs font-medium",
truncate && "truncate",
)}
>
{subtitle}
</span>
)}
Expand Down
160 changes: 160 additions & 0 deletions site/src/components/FormField/FormField.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import type { Meta, StoryObj } from "@storybook/react-vite";
import { useFormik } from "formik";
import type { FC } from "react";
import { expect, within } from "storybook/test";
import { FormField } from "./FormField";

interface ExampleFormFieldProps {
id?: string;
label: string;
description?: string;
helperText?: string;
required?: boolean;
error?: string;
value?: string;
}

const ExampleFormField: FC<ExampleFormFieldProps> = ({
id,
label,
description,
helperText,
required,
error,
value = "",
}) => {
const form = useFormik({
initialValues: { value },
onSubmit: () => {},
});

return (
<FormField
id={id}
field={{
name: "value",
id: "value",
value: form.values.value,
onChange: form.handleChange,
onBlur: form.handleBlur,
error: Boolean(error),
helperText: error ?? helperText,
}}
label={label}
description={description}
required={required}
/>
);
};

const meta: Meta<typeof ExampleFormField> = {
title: "components/FormField",
component: ExampleFormField,
args: {
id: "story-field",
label: "Provider name",
},
};

export default meta;
type Story = StoryObj<typeof ExampleFormField>;

export const Default: Story = {
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
const input = canvas.getByRole("textbox", { name: /Provider name/ });
await expect(input).not.toHaveAttribute("aria-describedby");
await expect(input).not.toHaveAttribute("aria-invalid", "true");
await expect(canvas.queryByText("*")).not.toBeInTheDocument();
},
};

export const Required: Story = {
args: {
required: true,
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await expect(canvas.getByText("*")).toBeVisible();
},
};

export const WithDescription: Story = {
args: {
description: "Shown to users when selecting this provider.",
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
const input = canvas.getByRole("textbox", { name: /Provider name/ });
await expect(input).toHaveAttribute(
"aria-describedby",
"story-field-description",
);
const description = canvas.getByText(
"Shown to users when selecting this provider.",
);
await expect(description).toHaveAttribute("id", "story-field-description");
},
};

export const WithHelperText: Story = {
args: {
helperText: "Lowercase letters and dashes only.",
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
const input = canvas.getByRole("textbox", { name: /Provider name/ });
await expect(input).toHaveAttribute(
"aria-describedby",
"story-field-helper",
);
},
};

export const WithError: Story = {
args: {
error: "Provider name is required.",
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
const input = canvas.getByRole("textbox", { name: /Provider name/ });
await expect(input).toHaveAttribute(
"aria-describedby",
"story-field-error",
);
await expect(input).toHaveAttribute("aria-invalid", "true");
await expect(canvas.getByText("Provider name is required.")).toBeVisible();
},
};

export const WithDescriptionAndError: Story = {
args: {
description: "Shown to users when selecting this provider.",
error: "Provider name is required.",
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
const input = canvas.getByRole("textbox", { name: /Provider name/ });
await expect(input).toHaveAttribute(
"aria-describedby",
"story-field-description story-field-error",
);
await expect(input).toHaveAttribute("aria-invalid", "true");
},
};

export const RequiredWithDescription: Story = {
args: {
required: true,
description: "Shown to users when selecting this provider.",
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
const input = canvas.getByRole("textbox", { name: /Provider name/ });
await expect(canvas.getByText("*")).toBeVisible();
await expect(input).toHaveAttribute(
"aria-describedby",
"story-field-description",
);
},
};
31 changes: 27 additions & 4 deletions site/src/components/FormField/FormField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,47 @@ import type { FormHelpers } from "#/utils/formUtils";
type FormFieldProps = React.ComponentPropsWithRef<"input"> & {
field: FormHelpers;
label: ReactNode;
description?: ReactNode;
};

export const FormField: FC<FormFieldProps> = ({
field,
label,
description,
className,
...inputProps
}) => {
const generatedId = useId();
const id = inputProps.id ?? generatedId;
const errorId = `${id}-error`;
const helperId = `${id}-helper`;
const descriptionId = `${id}-description`;
const describedBy = [
description ? descriptionId : null,
field.error ? errorId : field.helperText ? helperId : null,
]
.filter(Boolean)
.join(" ");
const required = inputProps.required ?? false;

return (
<div className="flex flex-col gap-2">
<Label htmlFor={id}>{label}</Label>
<Label htmlFor={id}>
{label}
{required && (
<>
{" "}
<span className="text-xs font-bold text-content-destructive">
*
</span>
</>
)}
</Label>
{description && (
<div id={descriptionId} className="text-xs text-content-secondary">
{description}
</div>
)}
<Input
name={field.name}
value={field.value}
Expand All @@ -31,9 +56,7 @@ export const FormField: FC<FormFieldProps> = ({
{...inputProps}
id={id}
aria-invalid={field.error}
aria-describedby={
field.error ? errorId : field.helperText ? helperId : undefined
}
aria-describedby={describedBy || undefined}
className={cn(field.error && "border-border-destructive", className)}
/>
{field.error ? (
Expand Down
50 changes: 40 additions & 10 deletions site/src/components/PageHeader/PageHeader.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { FC, PropsWithChildren, ReactNode } from "react";
import type React from "react";

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.

extreme nit but is this import necessary?

import type { FC, ReactNode } from "react";
import { cn } from "#/utils/cn";

interface PageHeaderProps {
Expand Down Expand Up @@ -31,32 +32,61 @@ export const PageHeader: FC<PageHeaderProps> = ({
);
};

export const PageHeaderTitle: FC<PropsWithChildren> = ({ children }) => {
type PageHeaderTitleProps = React.ComponentPropsWithRef<"h1">;

export const PageHeaderTitle: FC<PageHeaderTitleProps> = ({
children,
className,
...props
}) => {
return (
<h1 className="text-3xl font-semibold m-0 flex items-center leading-snug">
<h1
className={cn(
"text-3xl font-semibold m-0 flex items-center leading-snug",
className,
)}
{...props}
>
{children}
</h1>
);
};

interface PageHeaderSubtitleProps {
children?: ReactNode;
condensed?: boolean;
}
type PageHeaderSubtitleProps = React.ComponentPropsWithRef<"h2">;

export const PageHeaderSubtitle: FC<PageHeaderSubtitleProps> = ({
children,
className,
...props
}) => {
Comment on lines 57 to 61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Omit condensed before spreading subtitle props

PageHeaderSubtitle now forwards ...props to the <h2>, but condensed is a component-only prop and is not removed first. Existing call sites already pass <PageHeaderSubtitle condensed> (for example StarterTemplatePageView and TemplatePageHeader), so this change starts emitting an invalid DOM attribute (condensed) and React non-boolean-attribute warnings at runtime. Destructure condensed out before spreading the remaining intrinsic props.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the amended commit (force-pushed). PageHeaderSubtitle now destructures condensed out before spreading the remaining intrinsic props onto the <h2>. Manually verified the existing <PageHeaderSubtitle condensed> callers in StarterTemplatePageView and TemplatePageHeader no longer leak the attribute to the DOM.

Reply from Coder Agents on behalf of Jake Howell.

return (
<h2 className="text-sm text-content-secondary font-normal block m-0 leading-snug">
<h2
className={cn(
"text-sm text-content-secondary font-normal block m-0 leading-snug",
className,
)}
{...props}
>
{children}
</h2>
);
};

export const PageHeaderCaption: FC<PropsWithChildren> = ({ children }) => {
type PageHeaderCaptionProps = React.ComponentPropsWithRef<"span">;

export const PageHeaderCaption: FC<PageHeaderCaptionProps> = ({
children,
className,
...props
}) => {
return (
<span className="text-sm text-content-secondary font-medium uppercase tracking-widest">
<span
className={cn(
"text-sm text-content-secondary font-medium uppercase tracking-widest",
className,
)}
{...props}
>
{children}
</span>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const StarterTemplatePageView: FC<StarterTemplatePageViewProps> = ({
</div>
<div>
<PageHeaderTitle>{starterTemplate.name}</PageHeaderTitle>
<PageHeaderSubtitle condensed>
<PageHeaderSubtitle>
{starterTemplate.description}
</PageHeaderSubtitle>
</div>
Expand Down
6 changes: 2 additions & 4 deletions site/src/pages/TemplatePage/TemplatePageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -267,16 +267,14 @@ export const TemplatePageHeader: FC<TemplatePageHeaderProps> = ({
</div>

{template.deprecation_message !== "" ? (
<PageHeaderSubtitle condensed>
<PageHeaderSubtitle>
<MemoizedInlineMarkdown>
{template.deprecation_message}
</MemoizedInlineMarkdown>
</PageHeaderSubtitle>
) : (
template.description !== "" && (
<PageHeaderSubtitle condensed>
{template.description}
</PageHeaderSubtitle>
<PageHeaderSubtitle>{template.description}</PageHeaderSubtitle>
)
)}
</div>
Expand Down
Loading
Loading