From cb9958e4091589d1d02f6c71f8103aeab0e12266 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 14 Jul 2026 10:10:00 -0500 Subject: [PATCH 1/2] docs: add initial style guide docs sections --- .github/skills/style-guide/SKILL.md | 36 +- .../style-guide/docs/authoring-components.md | 119 ++++ .../style-guide/docs/component-props.md | 306 ++++++++++ .github/skills/style-guide/docs/css.md | 140 +++++ .../skills/style-guide/docs/managing-focus.md | 21 + .../skills/style-guide/docs/react-hooks.md | 102 ++++ .github/skills/style-guide/docs/storybook.md | 28 + .github/skills/style-guide/docs/testing.md | 65 +++ contributor-docs/style.md | 526 ------------------ 9 files changed, 810 insertions(+), 533 deletions(-) create mode 100644 .github/skills/style-guide/docs/authoring-components.md create mode 100644 .github/skills/style-guide/docs/component-props.md create mode 100644 .github/skills/style-guide/docs/css.md create mode 100644 .github/skills/style-guide/docs/managing-focus.md create mode 100644 .github/skills/style-guide/docs/react-hooks.md create mode 100644 .github/skills/style-guide/docs/storybook.md create mode 100644 .github/skills/style-guide/docs/testing.md delete mode 100644 contributor-docs/style.md diff --git a/.github/skills/style-guide/SKILL.md b/.github/skills/style-guide/SKILL.md index 7d9fb9348f6..b00465b16d2 100644 --- a/.github/skills/style-guide/SKILL.md +++ b/.github/skills/style-guide/SKILL.md @@ -1,18 +1,40 @@ --- name: style-guide -description: 'Use when: authoring, modifying, or reviewing Primer React components, hooks, utilities, or component APIs. Covers applying contributor-docs/style.md, including the spectrum of abstraction, presentational components, behavior hooks, config components, accessibility primitives, props, hooks, SSR, and CSS conventions.' +description: 'Use when: authoring, modifying, or reviewing Primer React components, hooks, utilities, or component APIs.' --- + + + +## Table of Contents + +- [Primer React Style Guide](#primer-react-style-guide) + - [Topics](#topics) + + + # Primer React Style Guide Use this skill when creating, modifying, or reviewing Primer React components, -hooks, utilities, or component APIs. - -Before authoring component code, read `contributor-docs/style.md` and apply the -guidelines that are relevant to the change. The style guide is the source of -truth for component API design and implementation conventions in this -repository. +hooks, utilities, or component APIs. This skill contains our style guide which +is the source of truth for component API design and implementation conventions +in this repository. When proposing, implementing, or reviewing a component change, explicitly apply the relevant style-guide principles to the design. If a change intentionally deviates from the style guide, call out the reason. + +Use the following topics and links to categorize what you're working on in order +to find the relevant guidance. Each guide contains a table of contents for quick +navigation to the relevant guidance. + +## Topics + +- [Authoring components](./docs/authoring-components.md) for our component authoring conventions +- [Component Props](./docs/component-props.md) for our Component prop + conventions +- [CSS](./docs/css.md) for our CSS conventions +- [React Hooks](./docs/react-hooks.md) for our React hook conventions +- [Managing focus](./docs/managing-focus.md) for our focus management conventions +- [Storybook](./docs/storybook.md) for our Storybook conventions +- [Testing](./docs/testing.md) for our testing conventions diff --git a/.github/skills/style-guide/docs/authoring-components.md b/.github/skills/style-guide/docs/authoring-components.md new file mode 100644 index 00000000000..1ec688c94ad --- /dev/null +++ b/.github/skills/style-guide/docs/authoring-components.md @@ -0,0 +1,119 @@ +# Authoring components + +Use these guidelines when authoring components. + + + + +## Table of Contents + +- [Overview](#overview) +- [Guidelines](#guidelines) + - [Prefer building components across a spectrum of abstraction](#prefer-building-components-across-a-spectrum-of-abstraction) + - [Config components](#config-components) + - [Presentational components](#presentational-components) + - [Base components](#base-components) + - [Utilities](#utilities) + + + + +## Overview + +React components are authored in `packages/react`. When adding a component, you +will add the following files: + +- `packages/react/src//index.ts` +- `packages/react/src//.tsx` +- `packages/react/src//.module.css` +- `packages/react/src//.test.tsx` +- `packages/react/src//.stories.tsx` +- `packages/react/src//.features.stories.tsx` +- `packages/react/src//.docs.json` + +## Guidelines + +### Prefer building components across a spectrum of abstraction + +When authoring Primer components, prefer starting with flexible presentational +components and companion behavior hooks. As opinions or defaults become +established, create config components that compose those presentational +components and hooks instead of duplicating their behavior. This keeps common +patterns easy to adopt while still giving consumers a clear path when they need +to customize appearance, content, semantics, or behavior. + +Use the following API types when designing component APIs: + +| API type | Use for | +| ------------------------- | ----------------------------------------------------------------------------------- | +| Config components | Ready-made, props-based components for stable product patterns and common use-cases | +| Presentational components | Styled pieces that consumers compose directly, often with companion behavior hooks | +| Base components | Unstyled primitives, often for accessibility structure or low-level behavior | +| Utilities | Hooks, state management, behaviors, and functions used to build components | + +#### Config components + +Config components are "all-in-one" APIs that let consumers describe intent +through props or data rather than composing markup directly. They should provide +opinionated defaults for structure, behavior, accessibility, styling, and +interactions so teams can implement established patterns quickly and correctly. + +Use config components when Primer understands the pattern well and expects many +teams to reuse it. Keep the supported customization surface explicit through +props, slots, render props, or configuration. Do not make config components +support every variation; if consumers need to change structure, semantics, or +behavior beyond the config API, they should be able to drop down to +presentational components and behavior hooks. + +#### Presentational components + +Presentational components are styled pieces that consumers compose directly. +They should let consumers control layout, ordering, conditional rendering, and +content while Primer still owns the styling, accessibility expectations, data +attributes, and component contracts for each piece. + +Prefer presentational components and behavior hooks for emerging patterns or +flexible APIs where a single high-level config API has not stabilized. Build +config components over time by composing those components and hooks after common +use-cases and defaults become clear. + +Do not add slots by default. Prefer normal React composition first, and only +introduce slots when a parent component must identify a specific child part or +the requested API explicitly needs child extraction. Do not reorder +consumer-authored children in flexible presentational components just to enforce +a preferred structure; document the recommended structure or add a dev warning +instead. + +#### Base components + +Base components are unstyled primitives used to build higher-level components. +Use them for accessibility primitives and low-level behaviors that need full +markup and style control. Accessibility primitives for established patterns, +such as ARIA Authoring Practices Guide patterns, should be consolidated and +reused rather than reimplemented across components. Before adding custom +behavior to a component, look for an existing base component, hook, utility, or +behavior that can provide the foundation. + +Prefer existing base primitives over recreating native elements and their reset +styles. For example, use shared button primitives such as `ButtonBase` when a +new component needs Primer-owned button semantics, interaction behavior, and +reset styling instead of hand-rolling a button with custom reset CSS. + +#### Utilities + +Utilities include hooks, functions, behaviors, and other reusable logic. Provide +utilities for established patterns so teams can build accessible experiences on +solid foundations. Examples include hooks such as `useMergedRefs`, +`useOnEscapePress`, and `useTimeout`, or lower-level packages such as +`@primer/behaviors`. + +### Server-side rendering + +All components should be able to be rendered on the server. This means that the +component must not access any JavaScript APIs that are only available in a +browser. + +In addition, authors should consider how components will be hydrated as a result +of being server-side rendered. In particular, components should not cause layout +shifts as a result of being hydrated. Instead components should progressively +enhance as JavaScript is loaded on the page. diff --git a/.github/skills/style-guide/docs/component-props.md b/.github/skills/style-guide/docs/component-props.md new file mode 100644 index 00000000000..6f1b2ac6918 --- /dev/null +++ b/.github/skills/style-guide/docs/component-props.md @@ -0,0 +1,306 @@ +# Component Props + +Use these conventions when authoring props for Primer React components. + + + + +## Table of Contents + +- [Naming conventions](#naming-conventions) + - [Prefer native names](#prefer-native-names) + - [Controlled and uncontrolled state](#controlled-and-uncontrolled-state) + - [Boolean props](#boolean-props) + - [Visual options](#visual-options) + - [Leading and trailing content](#leading-and-trailing-content) + - [Text, labels, and announcements](#text-labels-and-announcements) +- [Guidelines](#guidelines) + - [Prefer applying component rest parameters to the root element rendered by a component](#prefer-applying-component-rest-parameters-to-the-root-element-rendered-by-a-component) + - [Prefer authoring callback prop types with arguments that can be extended](#prefer-authoring-callback-prop-types-with-arguments-that-can-be-extended) + - [Prefer the `useControllableState` hook when authoring components that can be controlled or uncontrolled](#prefer-the-usecontrollablestate-hook-when-authoring-components-that-can-be-controlled-or-uncontrolled) + + + + +## Naming conventions + +Use these conventions when deciding on a prop name for a Primer React component. +Start from existing React, HTML, and ARIA names when they already describe the +behavior, then use Primer-specific names only when the prop represents a design +system concept. + +### Prefer native names + +Use native React and HTML prop names when the component exposes the same concept: + +- `children`, `className`, `style`, `id`, `ref` +- `as`, `href`, `target`, `rel` +- `disabled`, `required`, `checked`, `defaultChecked`, `value`, `defaultValue` +- `onChange`, `onClick`, `onKeyDown`, and other native event names + +Use real ARIA prop names instead of aliases: + +- Do: `'aria-label'`, `'aria-labelledby'`, `'aria-describedby'` +- Don't: `label`, `labelledBy`, `describedBy` when the value maps directly to an + ARIA attribute + +### Controlled and uncontrolled state + +For controlled state, pair the state prop with an `onXChange` callback: + +- `open` / `onOpenChange` +- `selected` / `onSelectedChange` +- `expanded` / `onExpandedChange` +- `checked` / `onChange` for native-like form controls + +For uncontrolled state, use `defaultX`: + +- `defaultOpen` +- `defaultSelected` +- `defaultExpanded` +- `defaultChecked` + +Prefer the state name directly (`open`, `selected`, `expanded`) over `isOpen`, +`isSelected`, or `isExpanded` for public component props. Existing `is*` props +are usually legacy APIs and should not be used as the default pattern for new +props. + +### Boolean props + +Use direct adjective or state names when the prop toggles behavior or appearance: + +- `disabled` +- `loading` +- `inactive` +- `selected` +- `current` +- `checked` +- `expanded` +- `open` +- `required` +- `indeterminate` +- `truncate` +- `inline` +- `block` +- `fullWidth` + +Use `has*` when the prop describes the presence of a structural visual element: + +- `hasBorder` + +Use `show*` or `hide*` when the prop explicitly controls visibility of optional +content: + +- `showSelectAll` +- `showSelectedOptionsFirst` +- `hideTitle` + +Use `disable*` sparingly for opt-outs where the default behavior stays enabled: + +- `disableFullscreenOnNarrow` + +### Visual options + +Use `variant` for visual style and `size` for scale. + +Examples: + +- `variant="default" | "primary" | "danger"` +- `size="small" | "medium" | "large"` + +When styling component options with data attributes, mirror the public prop name: + +- `variant` -> `data-variant` +- `size` -> `data-size` +- `loading` -> `data-loading` + +### Leading and trailing content + +Use logical position names for content that appears before or after the main +content: + +- `leadingVisual` +- `trailingVisual` +- `trailingAction` +- `leadingAction` +- `secondaryAction` + +Prefer `Visual` over `Icon` when the slot can contain any visual React node or +component, not only an Octicon. Older props such as `icon`, `leadingIcon`, and +`trailingIcon` are commonly deprecated in favor of `leadingVisual` or +`trailingVisual`. + +### Text, labels, and announcements + +Use `children` for primary visible content when possible. + +Use semantic names for specific visible text: + +- `title` +- `subtitle` +- `description` +- `placeholder` +- `caption` +- `text` + +Use purpose-specific names for assistive technology text that the component +renders or announces: + +- `srText` +- `loadingAnnouncement` +- `loadingLabel` + +Use ARIA attribute names directly when the consumer is providing the accessible +name or description for the underlying element: + +- `'aria-label'` +- `'aria-labelledby'` +- `'aria-describedby'` + +## Guidelines + +### Prefer applying component rest parameters to the root element rendered by a component + +Components may accept forwarding props to an underlying element through rest parameters. This strategy allows the component to accept common prop types like `className`, `data-testid`, and more without having to explicitly annotate each property in the component prop type definition. For example: + +```tsx +type Props = React.HTMLAttributes & { + variant?: 'primary' | 'secondary' +} + +function Example({variant = 'primary', ...rest}: Props) { + return ( +
+
{children}
+
+ ) +} +``` + +These rest parameters should be applied to the root element rendered by the +component. Tests must also make assertions these props are applied to the root +element. + + + + + +
UnpreferredPreferred
+ +```tsx +type Props = React.ComponentPropsWithoutRef<'div'> + +function Example({children, ...rest}: Props) { + return ( +
+
{children}
+
+ ) +} +``` + +		 + +```tsx +type Props = React.ComponentPropsWithoutRef<'div'> + +function Example({children, ...rest}: Props) { + return
{children}
+} +``` + +
+ +### Prefer authoring callback prop types with arguments that can be extended + +When authoring callback props, it is helpful to structure the types for these +functions in ways that allow for changes over time. For example, instead of +having a dedicated argument for a value consider using an object that contains +that value as a property. + +This structure allows additional properties to be added to the object without +having to update the function signature. In particular, this structure allows +us to more easily structure breaking changes if a property needs to be removed +from this type. + +This structure is also helpful for creating a consistent function signature +across callback props. Consumers only ever need to supply one argument and will +not need to worry about which position the argument they care about is in. + + + + + +
UnpreferredPreferred
+ +```tsx +type Props = { + onChange: (a: boolean, b: string, c: number) => void +} +``` + + + +```tsx +type Props = { + onChange: ({a, b, c}: {a: boolean; b: string; c: number}) => void +} +``` + +
+ +### Prefer the `useControllableState` hook when authoring components that can be controlled or uncontrolled + +Components may be authored in a way that allows state values to be controlled or +uncontrolled. For example: + +```tsx +type ExampleProps = { + value?: string + defaultValue?: string + onChange({value}: {value: string}): void +} + +function Example(props: ExampleProps) { + // ... +} + +function UncontrolledExample() { + return ( + { + console.log(value) + }} + /> + ) +} + +function ControlledExample() { + const [value, setValue] = React.useState('Example controlled value') + return ( + { + setValue(value) + }} + /> + ) +} +``` + +When building out components that can controlled or uncontrolled, prefer using +the `useControllableState` hook. With the `Example` component above, this +would look like: + +```tsx +function Example({defaultValue, onChange, value: controlledValue}: ExampleProps) { + const [value, setValue] = useControllableState({ + value: controlledValue, + defaultValue, + onChange: ({value}) => { + onChange({value}) + }, + }) +} +``` diff --git a/.github/skills/style-guide/docs/css.md b/.github/skills/style-guide/docs/css.md new file mode 100644 index 00000000000..fd6887b8a5d --- /dev/null +++ b/.github/skills/style-guide/docs/css.md @@ -0,0 +1,140 @@ +# CSS + +Use these guidelines when authoring CSS + + + + +## Table of Contents + +- [Overview](#overview) +- [Guidelines](#guidelines) + - [Prefer using `clsx` for conditional class names](#prefer-using-clsx-for-conditional-class-names) + - [Prefer using `data-*` attributes for states or modifiers instead of CSS class names](#prefer-using-data--attributes-for-states-or-modifiers-instead-of-css-class-names) + - [Prefer using CSS Custom Properties to bridge between CSS and JavaScript instead of inline styles](#prefer-using-css-custom-properties-to-bridge-between-css-and-javascript-instead-of-inline-styles) + + + + +## Overview + +Component styles are authored using CSS Modules in files that live beside the +corresponding component, for example: + +- `packages/react/src/Banner/Banner.tsx` +- `packages/react/src/Banner/Banner.module.css` + +## Guidelines + +### Prefer using `clsx` for conditional class names + +When authoring components, there may be situations where you want a class name +to only be applied if a condition is met. In these situations, prefer using the +`clsx` library to conditional apply class names. + + + + + +
UnpreferredPreferred
+ +```tsx +function Example({isActive}) { + return ( +
+
Example
+
+ ) +} +``` + +		 + +```tsx +function Example({isActive}) { + return ( +
+
Example
+
+ ) +} +``` + +
+ +### Prefer using `data-*` attributes for states or modifiers instead of CSS class names + + + + + +
UnpreferredPreferred
+ +```tsx +function Example({variant}: {variant: 'primary' | 'secondary'}) { + return ( +
+ Example +
+ ) +} +``` + +		 + +```tsx +function Example({variant}: {variant: 'primary' | 'secondary'}) { + return ( +
+ Example +
+ ) +} +``` + +
+ +### Prefer using CSS Custom Properties to bridge between CSS and JavaScript instead of inline styles + + + + + +
UnpreferredPreferred
+ +```tsx +function Example({variant}: {variant: 'primary' | 'secondary'}) { + return ( +
+ Example +
+ ) +} +``` + +		 + +```tsx +function Example({variant}: {variant: 'primary' | 'secondary'}) { + return ( +
+ Example +
+ ) +} +``` + +
diff --git a/.github/skills/style-guide/docs/managing-focus.md b/.github/skills/style-guide/docs/managing-focus.md new file mode 100644 index 00000000000..8020a269068 --- /dev/null +++ b/.github/skills/style-guide/docs/managing-focus.md @@ -0,0 +1,21 @@ +# Managing focus + +Use these guidelines when authoring components that manage focus. + + + + +## Table of Contents + + + + +## Guidelines + +### Prefer managing focus through event handlers instead of effects + +When managing focus within a component, prefer focusing elements within event +handlers as opposed to effects. This helps to ensure that focus is only being +moved as a result of a user interaction. When focus is managed in an effect, +there is a risk that focus will be moved due to an unrelated dependency in the +effect dependency array being updated. diff --git a/.github/skills/style-guide/docs/react-hooks.md b/.github/skills/style-guide/docs/react-hooks.md new file mode 100644 index 00000000000..0bdcb1c4b59 --- /dev/null +++ b/.github/skills/style-guide/docs/react-hooks.md @@ -0,0 +1,102 @@ +# React Hooks + +Use these guidelines when authoring hooks in React. + + + + +## Table of Contents + +- [Guidelines](#guidelines) + - [Prefer authoring hooks that accept a `ref` instead of returning a `ref` to apply](#prefer-authoring-hooks-that-accept-a-ref-instead-of-returning-a-ref-to-apply) + - [Prefer creating stable callbacks from hook arguments instead of adding them to dependency arrays](#prefer-creating-stable-callbacks-from-hook-arguments-instead-of-adding-them-to-dependency-arrays) + + + + +## Guidelines + +#### Prefer authoring hooks that accept a `ref` instead of returning a `ref` to apply + +When designing hooks that require a reference to a DOM node (using a `ref`) you +should design the hook to take in a `ref` as an argument instead of creating a +`ref` on behalf of the caller. +This is important when a caller decides to use multiple hooks that rely on a +`ref`. For example, + +```tsx +function MyComponent() { + const [ref1, isHovering] = useHover() + const [ref2, isDragging] = useDrag() + + // How should the caller merge these two refs? +} +``` + +If, instead, these hooks took in a `ref` we could have the caller manage the +`ref` and pass it into the hooks. + +```tsx +function MyComponent() { + const ref = useRef(null) + const isHovering = useHover(ref) + const isDragging = useDrag(ref) + + // Caller has to add `ref` to a node below +} +``` + +#### Prefer creating stable callbacks from hook arguments instead of adding them to dependency arrays + +A common practice when authoring hooks is to accept a `callback` as an argument +that is called when something is updated or changed. For example: + +```tsx +// Example hook +function useEvent(ref, eventName, callback) { + // +} + +function Example() { + const ref = React.useRef(null) + + useEvent(ref, 'click', () => { + // + }) +} +``` + +Instead of adding the callback to a dependency array, create a stable callback +that can be referenced. + +```tsx +function useEvent(ref, eventName, callback) { + const savedCallback = React.useRef(callback) + + React.useEffect(() => { + savedCallback.current = callback + }, [callback]) + + React.useEffect(() => { + const node = ref.current + + const eventListener = event => { + // Call the saved callback + savedCallback.current(event) + } + + node.addEventListener(eventName, eventListener) + return () => { + node.removeEventListener(eventName, eventListener) + } + }, [ref, eventName]) +} +``` + +Structuring hooks in this way allow us to decouple an effect from the identity +of the callback being passed in. This is helpful since the effect should only +synchronize with ref and event being passed in, not when the callback itself +changes. + +Authoring hooks in this way allow us to avoid propagating dependency arrays and +ensures that callbacks are always called with the latest value. diff --git a/.github/skills/style-guide/docs/storybook.md b/.github/skills/style-guide/docs/storybook.md new file mode 100644 index 00000000000..3ce3d775920 --- /dev/null +++ b/.github/skills/style-guide/docs/storybook.md @@ -0,0 +1,28 @@ +# Storybook + +Use these guidelines when authoring storybook stories. + + + + +## Table of Contents + +- [Overview](#overview) + + + + +## Overview + +Storybook stories live alongside the component they are documenting: + +- `packages/react/src/Banner/Banner.tsx` +- `packages/react/src/Banner/Banner.stories.tsx` +- `packages/react/src/Banner/Banner.features.stories.tsx` + +The `*.stories.tsx` file should contain two stories: + +- A `Default` story rendering the default case for the component +- A `Playground` story rendering a playground for the component, allowing users to interact with the component and change its props + +When additional functionality needs to be documented, a `*.features.stories.tsx` file should be created. This file should contain stories for each feature of the component, demonstrating how to use the feature and any relevant props. diff --git a/.github/skills/style-guide/docs/testing.md b/.github/skills/style-guide/docs/testing.md new file mode 100644 index 00000000000..5a5783b592f --- /dev/null +++ b/.github/skills/style-guide/docs/testing.md @@ -0,0 +1,65 @@ +# Testing + +Use these guidelines when authoring tests. + + + + +## Table of Contents + +- [Overview](#overview) +- [Components](#components) + - [Requirements](#requirements) + - [Use `@testing-library/react`](#use-testing-libraryreact) + - [Test that a components provides `className` support](#test-that-a-components-provides-classname-support) + - [Test that a component supports additional props on the root element](#test-that-a-component-supports-additional-props-on-the-root-element) + - [Guidelines](#guidelines) + - [Test the public API of a component](#test-the-public-api-of-a-component) + + + + +## Overview + +This project uses vitest as the test runner. Tests may live under the +`__tests__` directory or as `*.test.tsx?` files alongside the code they are testing. + +Prefer browser tests when possible. Use Node.js when working in an environment +that will never run in the browser. + +Shared config for testing lives in `packages/vitest-config`. + +## Components + +### Requirements + +#### Use `@testing-library/react` + +Render components using `@testing-library/react` and query for elements by +using `screen`. + +When interactivity is needed in a test, prefer `userEvent` from `vitest/browser`. If needed, fallback to `userEvent` from `@testing-library/react`. + +#### Test that a components provides `className` support + +Use the `implementsClassName` utility to test that a component supports the `className` prop. + +#### Test that a component supports additional props on the root element + +Provide an extra prop, like a `data-testid` to a component and assert that the +root element has that attribute. + +### Guidelines + +#### Test the public API of a component + +The most important thing to test in our component suite is the public API of a +component. This will allow us to safely make changes to the implementation of a +component without breaking consumers of the component. + +When evaluating the public API of a component, consider: + +- What props does the component have? What combinations can they be used in? +- What are the default values of props? +- For callback props, what arguments are passed to the callback when it is called? +- What accessibility guarantees or semantics does the component provide? diff --git a/contributor-docs/style.md b/contributor-docs/style.md deleted file mode 100644 index 42b67a5defb..00000000000 --- a/contributor-docs/style.md +++ /dev/null @@ -1,526 +0,0 @@ - - -# Primer Style Guide - - - - -## Table of Contents - -- [React](#react) - - [Server-side rendering](#server-side-rendering) - - [Prefer managing focus through event handlers instead of effects](#prefer-managing-focus-through-event-handlers-instead-of-effects) - - [Prefer the `useControllableState` hook when authoring components that can be controlled or uncontrolled](#prefer-the-usecontrollablestate-hook-when-authoring-components-that-can-be-controlled-or-uncontrolled) - - [Prefer building components across a spectrum of abstraction](#prefer-building-components-across-a-spectrum-of-abstraction) - - [Config components](#config-components) - - [Presentational components](#presentational-components) - - [Base components](#base-components) - - [Utilities](#utilities) - - [Props](#props) - - [Prefer applying component rest parameters to the root element rendered by a component](#prefer-applying-component-rest-parameters-to-the-root-element-rendered-by-a-component) - - [Prefer authoring callback prop types with arguments that can be extended](#prefer-authoring-callback-prop-types-with-arguments-that-can-be-extended) - - [Hooks](#hooks) - - [Prefer authoring hooks that accept a `ref` instead of returning a `ref` to apply](#prefer-authoring-hooks-that-accept-a-ref-instead-of-returning-a-ref-to-apply) - - [Prefer creating stable callbacks from hook arguments instead of adding them to dependency arrays](#prefer-creating-stable-callbacks-from-hook-arguments-instead-of-adding-them-to-dependency-arrays) -- [CSS](#css) - - [Prefer using `clsx` for conditional class names](#prefer-using-clsx-for-conditional-class-names) - - [Prefer using `data-*` attributes for states or modifiers instead of CSS class names](#prefer-using-data--attributes-for-states-or-modifiers-instead-of-css-class-names) - - [Prefer using CSS Custom Properties to bridge between CSS and JavaScript instead of inline styles](#prefer-using-css-custom-properties-to-bridge-between-css-and-javascript-instead-of-inline-styles) - - - - -## React - -### Server-side rendering - -All components should be able to be rendered on the server. This means that the -component must not access any JavaScript APIs that are only available in a -browser. - -In addition, authors should consider how components will be hydrated as a result -of being server-side rendered. In particular, components should not cause layout -shifts as a result of being hydrated. Instead components should progressively -enhance as JavaScript is loaded on the page. - -### Prefer managing focus through event handlers instead of effects - -When managing focus within a component, prefer focusing elements within event -handlers as opposed to effects. This helps to ensure that focus is only being -moved as a result of a user interaction. When focus is managed in an effect, -there is a risk that focus will be moved due to an unrelated dependency in the -effect dependency array being updated. - -### Prefer the `useControllableState` hook when authoring components that can be controlled or uncontrolled - -Components may be authored in a way that allows state values to be controlled or -uncontrolled. For example: - -```tsx -type ExampleProps = { - value?: string - defaultValue?: string - onChange({value}: {value: string}): void -} - -function Example(props: ExampleProps) { - // ... -} - -function UncontrolledExample() { - return ( - { - console.log(value) - }} - /> - ) -} - -function ControlledExample() { - const [value, setValue] = React.useState('Example controlled value') - return ( - { - setValue(value) - }} - /> - ) -} -``` - -When building out components that can controlled or uncontrolled, prefer using -the `useControllableState` hook. With the `Example` component above, this -would look like: - -```tsx -function Example({defaultValue, onChange, value: controlledValue}: ExampleProps) { - const [value, setValue] = useControllableState({ - value: controlledValue, - defaultValue, - onChange: ({value}) => { - onChange({value}) - }, - }) -} -``` - -### Prefer building components across a spectrum of abstraction - -When authoring Primer components, prefer starting with flexible presentational -components and companion behavior hooks. As opinions or defaults become -established, create config components that compose those presentational -components and hooks instead of duplicating their behavior. This keeps common -patterns easy to adopt while still giving consumers a clear path when they need -to customize appearance, content, semantics, or behavior. - -Use the following API types when designing component APIs: - -| API type | Use for | -| ------------------------- | ----------------------------------------------------------------------------------- | -| Config components | Ready-made, props-based components for stable product patterns and common use-cases | -| Presentational components | Styled pieces that consumers compose directly, often with companion behavior hooks | -| Base components | Unstyled primitives, often for accessibility structure or low-level behavior | -| Utilities | Hooks, state management, behaviors, and functions used to build components | - -#### Config components - -Config components are "all-in-one" APIs that let consumers describe intent -through props or data rather than composing markup directly. They should provide -opinionated defaults for structure, behavior, accessibility, styling, and -interactions so teams can implement established patterns quickly and correctly. - -Use config components when Primer understands the pattern well and expects many -teams to reuse it. Keep the supported customization surface explicit through -props, slots, render props, or configuration. Do not make config components -support every variation; if consumers need to change structure, semantics, or -behavior beyond the config API, they should be able to drop down to -presentational components and behavior hooks. - -#### Presentational components - -Presentational components are styled pieces that consumers compose directly. -They should let consumers control layout, ordering, conditional rendering, and -content while Primer still owns the styling, accessibility expectations, data -attributes, and component contracts for each piece. - -Prefer presentational components and behavior hooks for emerging patterns or -flexible APIs where a single high-level config API has not stabilized. Build -config components over time by composing those components and hooks after common -use-cases and defaults become clear. - -Do not add slots by default. Prefer normal React composition first, and only -introduce slots when a parent component must identify a specific child part or -the requested API explicitly needs child extraction. Do not reorder -consumer-authored children in flexible presentational components just to enforce -a preferred structure; document the recommended structure or add a dev warning -instead. - -#### Base components - -Base components are unstyled primitives used to build higher-level components. -Use them for accessibility primitives and low-level behaviors that need full -markup and style control. Accessibility primitives for established patterns, -such as ARIA Authoring Practices Guide patterns, should be consolidated and -reused rather than reimplemented across components. Before adding custom -behavior to a component, look for an existing base component, hook, utility, or -behavior that can provide the foundation. - -Prefer existing base primitives over recreating native elements and their reset -styles. For example, use shared button primitives such as `ButtonBase` when a -new component needs Primer-owned button semantics, interaction behavior, and -reset styling instead of hand-rolling a button with custom reset CSS. - -#### Utilities - -Utilities include hooks, functions, behaviors, and other reusable logic. Provide -utilities for established patterns so teams can build accessible experiences on -solid foundations. Examples include hooks such as `useMergedRefs`, -`useOnEscapePress`, and `useTimeout`, or lower-level packages such as -`@primer/behaviors`. - -### Props - -#### Prefer applying component rest parameters to the root element rendered by a component - -Components may accept forwarding props to an underlying element through rest parameters. This strategy allows the component to accept common prop types like `className`, `data-testid`, and more without having to explicitly annotate each property in the component prop type definition. For example: - -```tsx -type Props = React.HTMLAttributes & { - variant?: 'primary' | 'secondary' -} - -function Example({variant = 'primary', ...rest}: Props) { - return ( -
-
{children}
-
- ) -} -``` - -These rest parameters should be applied to the root element rendered by the -component. Tests must also make assertions these props are applied to the root -element. - - - - - -
UnpreferredPreferred
- -```tsx -type Props = React.ComponentPropsWithoutRef<'div'> - -function Example({children, ...rest}: Props) { - return ( -
-
{children}
-
- ) -} -``` - -		 - -```tsx -type Props = React.ComponentPropsWithoutRef<'div'> - -function Example({children, ...rest}: Props) { - return
{children}
-} -``` - -
- -#### Prefer authoring callback prop types with arguments that can be extended - -When authoring callback props, it is helpful to structure the types for these -functions in ways that allow for changes over time. For example, instead of -having a dedicated argument for a value consider using an object that contains -that value as a property. - -This structure allows additional properties to be added to the object without -having to update the function signature. In particular, this structure allows -us to more easily structure breaking changes if a property needs to be removed -from this type. - -This structure is also helpful for creating a consistent function signature -across callback props. Consumers only ever need to supply one argument and will -not need to worry about which position the argument they care about is in. - - - - - -
UnpreferredPreferred
- -```tsx -type Props = { - onChange: (a: boolean, b: string, c: number) => void -} -``` - - - -```tsx -type Props = { - onChange: ({a, b, c}: {a: boolean; b: string; c: number}) => void -} -``` - -
- -### Hooks - -#### Prefer authoring hooks that accept a `ref` instead of returning a `ref` to apply - -When designing hooks that require a reference to a DOM node (using a `ref`) you -should design the hook to take in a `ref` as an argument instead of creating a -`ref` on behalf of the caller. -This is important when a caller decides to use multiple hooks that rely on a -`ref`. For example, - -```tsx -function MyComponent() { - const [ref1, isHovering] = useHover() - const [ref2, isDragging] = useDrag() - - // How should the caller merge these two refs? -} -``` - -If, instead, these hooks took in a `ref` we could have the caller manage the -`ref` and pass it into the hooks. - -```tsx -function MyComponent() { - const ref = useRef(null) - const isHovering = useHover(ref) - const isDragging = useDrag(ref) - - // Caller has to add `ref` to a node below -} -``` - -#### Prefer creating stable callbacks from hook arguments instead of adding them to dependency arrays - -A common practice when authoring hooks is to accept a `callback` as an argument -that is called when something is updated or changed. For example: - -```tsx -// Example hook -function useEvent(ref, eventName, callback) { - // -} - -function Example() { - const ref = React.useRef(null) - - useEvent(ref, 'click', () => { - // - }) -} -``` - -Instead of adding the callback to a dependency array, create a stable callback -that can be referenced. - -```tsx -function useEvent(ref, eventName, callback) { - const savedCallback = React.useRef(callback) - - React.useEffect(() => { - savedCallback.current = callback - }, [callback]) - - React.useEffect(() => { - const node = ref.current - - const eventListener = event => { - // Call the saved callback - savedCallback.current(event) - } - - node.addEventListener(eventName, eventListener) - return () => { - node.removeEventListener(eventName, eventListener) - } - }, [ref, eventName]) -} -``` - -Structuring hooks in this way allow us to decouple an effect from the identity -of the callback being passed in. This is helpful since the effect should only -synchronize with ref and event being passed in, not when the callback itself -changes. - -Authoring hooks in this way allow us to avoid propagating dependency arrays and -ensures that callbacks are always called with the latest value. - -## CSS - -### Prefer using `clsx` for conditional class names - -When authoring components, there may be situations where you want a class name -to only be applied if a condition is met. In these situations, prefer using the -`clsx` library to conditional apply class names. - - - - - -
UnpreferredPreferred
- -```tsx -function Example({isActive}) { - return ( -
-
Example
-
- ) -} -``` - -		 - -```tsx -function Example({isActive}) { - return ( -
-
Example
-
- ) -} -``` - -
- -### Prefer using `data-*` attributes for states or modifiers instead of CSS class names - - - - - -
UnpreferredPreferred
- -```tsx -function Example({variant}: {variant: 'primary' | 'secondary'}) { - return ( -
- Example -
- ) -} -``` - -		 - -```tsx -function Example({variant}: {variant: 'primary' | 'secondary'}) { - return ( -
- Example -
- ) -} -``` - -
- -### Prefer using CSS Custom Properties to bridge between CSS and JavaScript instead of inline styles - - - - - -
UnpreferredPreferred
- -```tsx -function Example({variant}: {variant: 'primary' | 'secondary'}) { - return ( -
- Example -
- ) -} -``` - -		 - -```tsx -function Example({variant}: {variant: 'primary' | 'secondary'}) { - return ( -
- Example -
- ) -} -``` - -
From 872aa08ba50a44f38cf14b65bbb20aebec18219a Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 15 Jul 2026 10:47:51 -0500 Subject: [PATCH 2/2] Enhance storybook documentation for feature stories Added guidance for writing feature stories to facilitate easy integration into projects. --- .github/skills/style-guide/docs/storybook.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/skills/style-guide/docs/storybook.md b/.github/skills/style-guide/docs/storybook.md index 3ce3d775920..df5fda09704 100644 --- a/.github/skills/style-guide/docs/storybook.md +++ b/.github/skills/style-guide/docs/storybook.md @@ -25,4 +25,4 @@ The `*.stories.tsx` file should contain two stories: - A `Default` story rendering the default case for the component - A `Playground` story rendering a playground for the component, allowing users to interact with the component and change its props -When additional functionality needs to be documented, a `*.features.stories.tsx` file should be created. This file should contain stories for each feature of the component, demonstrating how to use the feature and any relevant props. +When additional functionality needs to be documented, a `*.features.stories.tsx` file should be created. This file should contain stories for each feature of the component, demonstrating how to use the feature and any relevant props. Feature stories must be written in a way that allows for individuals to easily copy and paste the snippets into their own project. As a result, avoid abstractions that inhibit this workflow.