-
Notifications
You must be signed in to change notification settings - Fork 673
docs: update style guide skill with prop naming conventions for boolean props #8274
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
Open
+2,587
−297
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
1ace387
docs: extend style-guide skill
joshblack 15b2123
docs: update prop naming doc
joshblack 2ebaacb
docs: update description
joshblack bb11ea9
docs: update prop naming with durable defaults idea
joshblack 5f24e6f
Potential fix for pull request finding
joshblack 9b6633b
Merge branch 'main' into docs/update-style-guide
joshblack cdbc344
Merge branch 'main' into docs/update-style-guide
joshblack File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
143 changes: 143 additions & 0 deletions
143
.github/skills/style-guide/docs/component-prop-naming.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| # Component Prop Naming | ||
|
|
||
| Use these conventions when creating, editing, or evaluating props for Primer React components. | ||
|
|
||
| <!-- prettier-ignore-start --> | ||
| <!-- START doctoc generated TOC please keep comment here to allow auto update --> | ||
| <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> | ||
| ## Table of Contents | ||
|
|
||
| - [Boolean props](#boolean-props) | ||
| - [Name state props as a bare adjective](#name-state-props-as-a-bare-adjective) | ||
| - [Prefix default values with `default*`](#prefix-default-values-with-default) | ||
| - [Name configuration props based on durable defaults](#name-configuration-props-based-on-durable-defaults) | ||
| - [Use `hide` or `show` for visibility-related props](#use-hide-or-show-for-visibility-related-props) | ||
| - [Prefer named modes for behavior with multiple options](#prefer-named-modes-for-behavior-with-multiple-options) | ||
| - [Use a single mode prop instead of mutually exclusive boolean props](#use-a-single-mode-prop-instead-of-mutually-exclusive-boolean-props) | ||
|
|
||
| <!-- END doctoc generated TOC please keep comment here to allow auto update --> | ||
| <!-- prettier-ignore-end --> | ||
|
|
||
| ## Boolean props | ||
|
|
||
| ### Name state props as a bare adjective | ||
|
|
||
| Component props that represent state must be named without a prefix. For | ||
| example: | ||
|
|
||
| - Prefer `open` over `isOpen` | ||
| - Prefer `expanded` over `isExpanded` | ||
| - Prefer `selected` over `isSelected` | ||
|
|
||
| ### Prefix default values with `default*` | ||
|
|
||
| When exposing a default value for a state prop, prefix the bare adjective with | ||
| `default*`. For example: | ||
|
|
||
| - Prefer `defaultOpen` over `defaultIsOpen` | ||
| - Prefer `defaultExpanded` over `defaultIsExpanded` | ||
| - Prefer `defaultSelected` over `defaultIsSelected` | ||
|
|
||
| ### Name configuration props based on durable defaults | ||
|
|
||
| When a prop is used to configure what a component does, for example hiding a | ||
| title, name the prop after the non-default action a consumer takes. For example: | ||
|
|
||
| - Use `hideTitle` when the title is visible by default | ||
| - Use `showDivider` when the divider is hidden by default | ||
| - Use `hideCloseButton` when the close button is visible by default | ||
|
|
||
| This helps avoid inverted naming. For example, a prop named `showTitle` may end up | ||
| being passed as `false` in most usage, whereas a prop named `hideTitle` would | ||
| typically be passed as `true`. | ||
|
|
||
| A default is durable when: | ||
|
|
||
| - It is part of the component's structural contract, such as whether an optional | ||
| part is rendered | ||
| - It is consistent across variants, viewport sizes, and usage contexts | ||
| - Changing it would be an intentional design change rather than an implementation | ||
| detail or contextual adjustment | ||
| - The non-default action has a concise, unambiguous name | ||
|
|
||
| If a default varies by context or both values represent meaningful behaviors, | ||
| prefer a stable state name or named modes. The prop name should continue to | ||
| describe the requested behavior if the default changes. For example: | ||
|
|
||
| ```tsx | ||
| // Avoid | ||
| type ExampleProps = { | ||
| disableTruncation?: boolean | ||
| disableStickyPositioning?: boolean | ||
| } | ||
|
|
||
| // Prefer | ||
| type ExampleProps = { | ||
| textOverflow?: 'truncate' | 'wrap' | ||
| position?: 'sticky' | 'static' | ||
| } | ||
| ``` | ||
|
|
||
| ### Use `hide` or `show` for visibility-related props | ||
|
|
||
| When a configuration prop adds or removes a named optional part of a component | ||
| and has one stable default, use `hide` or `show` in the prop name. For example: | ||
|
|
||
| - Use `hideTitle` when the title is visible by default | ||
| - Use `showDivider` when the divider is hidden by default | ||
|
|
||
| For visibility state, native HTML semantics, or responsive values, names such as | ||
| `hidden` may be appropriate. | ||
|
|
||
| ### Prefer named modes for behavior with multiple options | ||
|
|
||
| When behavior has multiple meaningful modes, prefer a named string-literal union | ||
| that describes those modes. For example: | ||
|
|
||
| ```tsx | ||
| // Avoid | ||
| type ExampleProps = { | ||
| preventOverflow?: boolean | ||
| } | ||
|
|
||
| // Prefer | ||
| type ExampleProps = { | ||
| overflow?: 'auto' | 'prevent' | ||
| } | ||
| ``` | ||
|
|
||
| Keep a boolean prop for a stable binary capability or state, such as `disabled`, | ||
| `required`, or `loading`, or for configuration based on a durable default. Use a | ||
| discriminated union when a mode determines which other props are valid. For | ||
| example: | ||
|
|
||
| ```tsx | ||
| type ExampleProps = | ||
| | { | ||
| variant: 'icon' | ||
| 'aria-label': string | ||
| } | ||
| | { | ||
| variant: 'text' | ||
| children: React.ReactNode | ||
| } | ||
| ``` | ||
|
|
||
| ### Use a single mode prop instead of mutually exclusive boolean props | ||
|
|
||
| When a component has multiple boolean props that are mutually exclusive, replace | ||
| them with one string-literal union prop. This makes it clear that only one option | ||
| can be selected at a time and improves type safety. For example: | ||
|
|
||
| ```tsx | ||
| // Avoid | ||
| type ExampleProps = { | ||
| singleSelect?: boolean | ||
| multiSelect?: boolean | ||
| } | ||
|
|
||
| // Prefer | ||
| type ExampleProps = { | ||
| selectionVariant?: 'single' | 'multiple' | ||
| } | ||
| ``` | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not fully convinced by this one, but approving because good is better than perfect