feat(site): add UI primitives for the AI settings stack by jakehwll · Pull Request #25579 · coder/coder

jakehwll · 2026-05-21T16:11:40Z

🤖 This PR was modified by Coder Agents on behalf of Jake Howell.

Linear: DEVEX-355

First PR in a 5-PR stack splitting #25328. Adds the small UI primitives the AI settings stack depends on.

FormField accepts a description prop and renders a required marker. aria-describedby is composed from the description, helper, and error IDs.
PageHeader title, subtitle, and caption forward className and other intrinsic h1/h2/span props to their root elements.
AvatarData gains an opt-in truncate prop that clips overflowing title and subtitle with an ellipsis. Off by default so existing consumers passing non-text nodes (icons, badges) do not clip silently.
Bundles the Vercel provider icon (vercel.svg) and registers it in icons.json and externalImages.ts.

No new pages or routes here; later PRs in the stack consume these primitives.

Stack

jakehwll/DEVEX-355/01-primitives, primitives (this PR)
jakehwll/DEVEX-355/02-api, API client and query layer
jakehwll/DEVEX-355/03-components, provider form components
jakehwll/DEVEX-355/04-pages, pages and routes
jakehwll/DEVEX-355/05-section, section reshuffle

Replaces #25328 once the stack lands.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66f0b253fd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-21T16:15:14Z

 export const PageHeaderSubtitle: FC<PageHeaderSubtitleProps> = ({
 	children,
+	className,
+	...props
 }) => {


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 👍 / 👎.

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.

jakehwll · 2026-05-21T16:16:30Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

aslilac · 2026-05-22T05:41:23Z


 	return (
-		<div className="flex items-center gap-3">
+		<div className="flex min-w-0 items-center gap-3">


Suggested change

<div className="flex min-w-0 items-center gap-3">

<div className="flex items-center gap-3">

doubt this does anything

Applied. You're right, the outer container is a flex container, not a flex child, so min-w-0 was a no-op in every current call site. Reverted to the original flex items-center gap-3.

Reply from Coder Agents on behalf of Jake Howell.

aslilac · 2026-05-22T05:41:44Z


-			<div className="flex flex-col">
-				<span className="text-sm font-semibold text-content-primary">
+			<div className="flex min-w-0 flex-1 flex-col overflow-hidden">


Suggested change

<div className="flex min-w-0 flex-1 flex-col overflow-hidden">

<div className="flex flex-1 flex-col overflow-hidden">

same

Applied. overflow-hidden already sets the flex item's automatic min-size to 0 (per the CSS Flexbox spec on min-width: auto), so min-w-0 was redundant here. Removed it and kept flex flex-1 flex-col overflow-hidden. Truncation still works because Avatar itself carries shrink-0.

Reply from Coder Agents on behalf of Jake Howell.

aslilac · 2026-05-22T05:43:25Z

 			data-testid="header"
 		>
-			<hgroup className="flex flex-col gap-2">{children}</hgroup>
+			<hgroup className="flex flex-col gap-2 min-w-0">{children}</hgroup>


Suggested change

<hgroup className="flex flex-col gap-2 min-w-0">{children}</hgroup>

<hgroup className="flex flex-col gap-2">{children}</hgroup>

why so eager to add this everywhere? what does it do?

Applied. Same story, nothing inside the hgroup truncates so the class was load-bearing for nothing. Removed.

You caught me being defensive about min-w-0. I was carrying it over from earlier flex-shrink work without checking whether the surrounding container actually needed it.

Reply from Coder Agents on behalf of Jake Howell.

aslilac · 2026-05-22T05:43:59Z

 	children,
+	className,
+	// Component-only prop. Pull it out so it isn't forwarded to the DOM.
+	condensed: _condensed,


if it's unused then let's just remove it from the props type

Applied. Dropped condensed?: boolean from PageHeaderSubtitleProps entirely and removed the now-unused destructure. Also removed the dead <PageHeaderSubtitle condensed> usages from StarterTemplatePageView and TemplatePageHeader so the callers stop passing a prop that was never read.

Reply from Coder Agents on behalf of Jake Howell.

aslilac · 2026-05-22T05:45:01Z

 	"typescript.svg",
 	"ubuntu.svg",
 	"vault.svg",
+	"vercel.svg",


needs an entry in externalImages.ts

Added ["/icon/vercel.svg", "whiteWithColor"] to defaultParametersForBuiltinIcons. The logo is a single white triangle, so it'd disappear on light themes without the inversion filter.

Reply from Coder Agents on behalf of Jake Howell.

aslilac · 2026-05-22T05:45:15Z

 	babel({ presets: [compilerPreset] }),
 	checker({
 		typescript: true,
+		overlay: false,


Suggested change

overlay: false,

Removed. The override was inherited from the source PR and isn't necessary here. Back to vite-plugin-checker's default overlay: true.

Reply from Coder Agents on behalf of Jake Howell.

jakehwll · 2026-05-22T08:29:43Z

/coder-agents-review

coder-agents-review

Clean decomposition of UI primitives. The code changes are tight and purposeful: PageHeader prop spreading, FormField description/required additions, AvatarData overflow handling, dead condensed prop removal, and Vercel icon registration. Good use of cn() for className merging and proper aria-describedby wiring.

4 P2, 3 P3, 2 Nit.

The AvatarData truncation P2 is a real behavioral regression for WorkspacesTable (icons/badges get hard-clipped). The FormField story gap is the kind of debt that compounds across the 5-PR stack.

"Oh, this component. It renders labels, inputs, errors, helper text, and now it also composes aria-describedby from up to three conditional IDs, renders a required asterisk, and shows a description block. Lovely features. Zero tests." (Bisky)

site/src/components/PageHeader/PageHeader.tsx:76

P3 [DEREM-8] PageHeaderCaption remains FC<PropsWithChildren> while PageHeaderTitle and PageHeaderSubtitle are now ComponentPropsWithRef<"h1"/"h2"> with className merging and prop spreading. A consumer who discovers className works on the title tries the same on the caption and gets a TypeScript error. Since this PR's purpose is "add UI primitives," shipping a family where 2/3 accept className and 1/3 doesn't is a rough edge in the primitive API.

(Gon)

🤖

🤖 This review was automatically generated with Coder Agents.

coder-agents-review · 2026-05-22T08:52:45Z

+export const PageHeaderTitle: FC<PageHeaderTitleProps> = ({
+	children,
+	className,
+	title,


P3 [DEREM-1] title is destructured from props but never referenced in the function body. The effect is that the native HTML title attribute (tooltip) is silently stripped from {...props} and dropped. PageHeaderSubtitle (line 58) does not do the same extraction, so the behavior is inconsistent between siblings. If stripping the tooltip is intentional, prefix with _title and add a comment. If not, remove the destructure so the title attribute passes through to the <h1>. (Netero)

🤖

Removed the title destructure so the HTML title attribute flows through ...props to the <h1>. No more silent stripping, and the behavior is now consistent with PageHeaderSubtitle and PageHeaderCaption.

Reply from Coder Agents on behalf of Jake Howell.

coder-agents-review · 2026-05-22T08:52:45Z

 		<div className="flex flex-col gap-2">
-			<Label htmlFor={id}>{label}</Label>
+			<Label htmlFor={id}>
+				{label}{" "}


Nit [DEREM-2] {label}{" "} renders a trailing space inside the <Label> even when required is false and no asterisk follows. Move the space inside the conditional:

{required && ( <>{" "}<span className="text-xs font-bold text-content-destructive">*</span></> )}

(Netero, Chopper)

🤖

Applied. Moved the space inside the required conditional so the trailing whitespace only renders alongside the asterisk.

Reply from Coder Agents on behalf of Jake Howell.

coder-agents-review · 2026-05-22T08:52:45Z

 		<div className="flex flex-col gap-2">
-			<Label htmlFor={id}>{label}</Label>
+			<Label htmlFor={id}>
+				{label}{" "}


P2 [DEREM-3] FormField has zero Storybook stories, and this PR extends its untested surface with description, required asterisk rendering, and a multi-source aria-describedby composition that joins descriptionId, errorId, and helperId. The composition has enough branching (description present/absent x error/helper present/absent) that a wrong join silently points a screen reader at a nonexistent element. No visual indicator of the failure.

Stories needed at minimum:

Default (label + input, no error, no description)

Required (asterisk renders)

WithDescription (description div renders with correct id)

WithError (aria-describedby points at error)

WithDescriptionAndError (both IDs in aria-describedby)

(Bisky P2, Mafuuu P3, Chopper P3, Nami Nit)

🤖

Added FormField.stories.tsx with seven stories driven by a Formik-backed example wrapper: Default, Required, WithDescription, WithHelperText, WithError, WithDescriptionAndError, and RequiredWithDescription. Each story uses a play function to assert the aria-describedby composition (single ID, joined IDs, absent), aria-invalid, and the required asterisk. pnpm test:storybook src/components/FormField/FormField.stories.tsx passes all seven.

Reply from Coder Agents on behalf of Jake Howell.

coder-agents-review · 2026-05-22T08:52:45Z


-			<div className="flex flex-col">
-				<span className="text-sm font-semibold text-content-primary">
+			<div className="flex flex-1 flex-col overflow-hidden">


P2 [DEREM-4] Adding truncate to the title <span> clips ReactNode children without showing an ellipsis. text-overflow: ellipsis only applies to inline text overflow, not to block or flex children. WorkspacesTable passes a flex <div> as title containing the workspace name, a favorite star icon, an outdated tooltip, and Task/Chat badges. When the column is narrower than the content, overflow: hidden clips the flex div and the icons/badges disappear without any visual trace. The user sees a name but not the status indicators that were cut off.

This is a cross-cutting change to a shared component used in ~25 consumers. Could an opt-in truncate prop on AvatarData be safer, set only in the new AI settings pages where truncation is needed?

(Hisoka)

🤖

Made truncation opt-in via a new truncate?: boolean prop that defaults to false. When truncate is unset, AvatarData keeps its previous DOM (no flex-1, no overflow-hidden, no truncate on the spans), so existing consumers like WorkspacesTable keep their icons and badges. The AI settings rows opt in explicitly. Added a JSDoc on the prop explaining why it isn't on by default.

Reply from Coder Agents on behalf of Jake Howell.

coder-agents-review · 2026-05-22T08:52:45Z


-			<div className="flex flex-col">
-				<span className="text-sm font-semibold text-content-primary">
+			<div className="flex flex-1 flex-col overflow-hidden">


P3 [DEREM-5] The existing AvatarData stories use short strings ("coder", "coder@coder.com") that never trigger truncation. This PR adds truncate, flex-1, and overflow-hidden but no story demonstrates the new behavior. A WithLongTitle story inside a width-constrained container would serve as a regression baseline.

(Bisky P3, Zoro P3)

🤖

Added a WithLongTitle story that sets truncate: true and renders inside a max-w-240px decorator with a deliberately long title and subtitle, so the ellipsis behavior is visible as a regression baseline.

Reply from Coder Agents on behalf of Jake Howell.

coder-agents-review · 2026-05-22T08:52:45Z


-			<div className="flex flex-col">
-				<span className="text-sm font-semibold text-content-primary">
+			<div className="flex flex-1 flex-col overflow-hidden">


P2 [DEREM-6] The PR description claims two deliverables not present in the diff: "AvatarData tweaks (hideAvatar prop)" and "allows importing SVGs as URLs in vite.config.mts." hideAvatar does not exist anywhere in the codebase. The diff touches zero lines in vite.config.mts. Two of four bullet points describe work that was not delivered. A reviewer reading the description will look for code that isn't there. Fix: rewrite the two bullets to match the actual diff (AvatarData truncation classes, PageHeader prop extensibility).

(Mafu-san P2, Pariston P3, Leorio P3)

🤖

Rewrote the PR description bullets to match what actually ships:

The phantom hideAvatar bullet is replaced by the opt-in AvatarData.truncate change.

The phantom vite.config.mts bullet is dropped; the icon bullet now correctly mentions both icons.json and externalImages.ts.

FormField and PageHeader bullets describe the actual prop changes (description + required marker, intrinsic prop passthrough).

Reply from Coder Agents on behalf of Jake Howell.

coder-agents-review · 2026-05-22T08:52:45Z


-			<div className="flex flex-col">
-				<span className="text-sm font-semibold text-content-primary">
+			<div className="flex flex-1 flex-col overflow-hidden">


P2 [DEREM-7] The commit body contains two factual errors. (1) "PageHeader gains min-w-0 wrapping to support truncation." No min-w-0 appears anywhere in this diff. The actual change is className passthrough via cn(). (2) "Pure additive. No existing call sites change behavior." False. AvatarData's truncate and overflow-hidden classes change sizing and clipping for every consumer. A developer git-blaming this file a year from now reads "no existing call sites change behavior" and moves on to the wrong commit. Fix: rewrite to describe what actually shipped.

(Leorio)

🤖

Rewrote the commit body in the amended commit. Dropped the bogus "min-w-0 wrapping" line (never shipped) and the inaccurate "Pure additive. No existing call sites change behavior" claim. The new body describes the actual deliverables: FormField description + required marker + aria-describedby composition, PageHeader className passthrough, opt-in AvatarData.truncate, and the Vercel icon registration in both icons.json and externalImages.ts.

Reply from Coder Agents on behalf of Jake Howell.

coder-agents-review · 2026-05-22T08:52:45Z


-			<div className="flex flex-col">
-				<span className="text-sm font-semibold text-content-primary">
+			<div className="flex flex-1 flex-col overflow-hidden">


Nit [DEREM-9] Commit scope site/src/components does not contain all changed files. The diff spans site/src/components/, site/src/pages/, site/src/theme/, and site/static/. Project convention: "When you use a scope, it must be a real filesystem path containing every changed file." The correct scope is site or omitted.

(Mafu-san)

🤖

Broadened the scope to feat(site): ... so it covers every changed path (site/src/components, site/src/pages, site/src/theme, site/static). Per the convention in AGENTS.md, the scope must be a real path containing every changed file.

Reply from Coder Agents on behalf of Jake Howell.

coder-tasks · 2026-05-26T15:08:45Z

Documentation Check

New Documentation Needed

site/src/components/FormField/FormField.stories.tsx - Add Storybook story for FormField component with label, optional description, error handling, and required field indicator
site/src/components/Avatar/AvatarData.stories.tsx - Add story for new truncate prop (WithLongTitle story)
Update site/src/components/PageHeader/PageHeader.stories.tsx - Add story demonstrating new className and HTML props support for PageHeaderTitle and PageHeaderSubtitle

⚠️ Checked but no corresponding documentation changes found in this PR

Automated review via Coder Agents

Adds reusable primitives that the AI settings pages compose: - FormField accepts a description and renders a required marker. aria-describedby is composed from description, helper, and error IDs. - PageHeader title, subtitle, and caption forward className and other intrinsic h1/h2/span props to their root elements. - AvatarData gains an opt-in `truncate` prop that clips overflowing title and subtitle with an ellipsis. Off by default so consumers passing non-text nodes (icons, badges) do not clip silently. - icons.json and externalImages.ts register a new Vercel icon.

jeremyruppel · 2026-05-26T16:00:00Z

@@ -1,4 +1,5 @@
-import type { FC, PropsWithChildren, ReactNode } from "react";
+import type React from "react";


extreme nit but is this import necessary?

This was referenced May 21, 2026

feat(site): add AI provider API client and query layer #25580

Merged

feat(site): add AI settings provider form components #25581

Merged

github-actions Bot assigned jakehwll May 21, 2026

This was referenced May 21, 2026

feat(site): add AI settings providers pages and routes #25583

Merged

feat(site): promote AI settings to a top-level section #25582

Merged

feat: implement AI settings page with providers management UI #25328

Closed

chatgpt-codex-connector Bot reviewed May 21, 2026

View reviewed changes

jakehwll requested review from aslilac and jeremyruppel May 21, 2026 16:18

jakehwll changed the title ~~feat(site/src/components): add primitives for AI settings form layout~~ feat(site): add primitives for AI settings form layout May 21, 2026

jakehwll changed the title ~~feat(site): add primitives for AI settings form layout~~ feat(site): add UI primitives for the AI settings stack May 21, 2026

jakehwll force-pushed the jakehwll/DEVEX-355/01-primitives branch from 66f0b25 to e0448f5 Compare May 21, 2026 16:52

aslilac reviewed May 22, 2026

View reviewed changes

jakehwll force-pushed the jakehwll/DEVEX-355/01-primitives branch from e0448f5 to faeb9d9 Compare May 22, 2026 08:15

ibetitsmike approved these changes May 22, 2026

View reviewed changes

coder-agents-review Bot reviewed May 22, 2026

View reviewed changes

jakehwll force-pushed the jakehwll/DEVEX-355/01-primitives branch from 3146544 to faa9eb3 Compare May 26, 2026 15:22

jeremyruppel approved these changes May 26, 2026

View reviewed changes

jakehwll merged commit 8ae7320 into main May 26, 2026
32 of 34 checks passed

jakehwll deleted the jakehwll/DEVEX-355/01-primitives branch May 26, 2026 16:01

github-actions Bot locked and limited conversation to collaborators May 26, 2026

	<div className="flex min-w-0 items-center gap-3">
	<div className="flex items-center gap-3">

	<div className="flex min-w-0 flex-1 flex-col overflow-hidden">
	<div className="flex flex-1 flex-col overflow-hidden">

	<hgroup className="flex flex-col gap-2 min-w-0">{children}</hgroup>
	<hgroup className="flex flex-col gap-2">{children}</hgroup>

		@@ -1,4 +1,5 @@
		import type { FC, PropsWithChildren, ReactNode } from "react";
		import type React from "react";

Conversation

jakehwll commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakehwll commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakehwll commented May 22, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakehwll commented May 21, 2026 •

edited

Loading

jakehwll commented May 21, 2026 •

edited

Loading

coder-tasks Bot commented May 26, 2026 •

edited

Loading