Skip to content

Commit

Permalink
Merge branch 'main' into siddharth/composable-actionmenu
Browse files Browse the repository at this point in the history
  • Loading branch information
siddharthkp committed Nov 30, 2021
2 parents acaac55 + 65b63e4 commit 6f0712d
Show file tree
Hide file tree
Showing 24 changed files with 199 additions and 161 deletions.
5 changes: 5 additions & 0 deletions .changeset/clean-clocks-warn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': major
---

Details no longer accepts styled-system props. Please use the `sx` prop to extend Primer component styling instead. See also https://primer.style/react/overriding-styles for information about `sx` and https://primer.style/react/system-props for context on the removal.
5 changes: 5 additions & 0 deletions .changeset/rude-squids-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': major
---

Avatar no longer accepts styled-system props. Please use the `sx` prop to extend Primer component styling instead. See also https://primer.style/react/overriding-styles for information about `sx` and https://primer.style/react/system-props for context on the removal.
5 changes: 5 additions & 0 deletions .changeset/ten-doors-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': major
---

BranchName no longer accepts styled-system props. Please use the `sx` prop to extend Primer component styling instead. See also https://primer.style/react/overriding-styles for information about `sx` and https://primer.style/react/system-props for context on the removal.
5 changes: 5 additions & 0 deletions .changeset/three-moose-impress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': major
---

Heading no longer accepts styled-system props. Please use the `sx` prop to extend Primer component styling instead. See also https://primer.style/react/overriding-styles for information about `sx` and https://primer.style/react/system-props for context on the removal.
72 changes: 14 additions & 58 deletions contributor-docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,65 +77,35 @@ Additionally, every component should support [the `sx` prop](https://primer.styl

Here's an example of a basic component written in the style of Primer Components:

```jsx
import {TYPOGRAPHY, COMMON} from './constants'
import sx from './sx'
```tsx
import sx, {SxProp} from './sx'

const Component = styled.div`
const Component = styled.div<SxProp>`
// additional styles here
${COMMON};
${TYPOGRAPHY};
${sx};
`

Component.defaultProps = {
m: 0,
fontSize: 5,
fontSize: 5
}

export default Component
```

### Adding system props

Each component should have access to the appropriate system props. Every component has access to `COMMON`. For **most** components added, you'll only need to give the component to `COMMON`. If you are unsure, ping a DS team member on your PR.

Categories of system props are exported from `src/constants.js`:

- `COMMON` includes color and spacing (margin and padding) props
- `TYPOGRAPHY` includes font family, font weight, and line-height props
- `POSITION` includes positioning props
- `FLEX` includes flexbox props
- `BORDER` includes border and box-shadow props
- `GRID` includes grid props

To give the component access to a group of system props, import the system prop function from `./constants` and include the system prop function in your styled-component like so:

```jsx
import {COMMON} from './constants'

const Component = styled.div`
// additional styles here
${COMMON};
`
```

Remember that the system prop function inside your style declaration needs to go _after_ any built-in styles you want to be overridable.

### Adding the `sx` prop

Each component should provide access to a prop called `sx` that allows for setting theme-aware ad-hoc styles. See the [overriding styles](https://primer.style/components/overriding-styles) doc for more information on using the prop.
Each component should accept a prop called `sx` that allows for setting theme-aware ad-hoc styles. See the [overriding styles](https://primer.style/components/overriding-styles) doc for more information on using the prop.

Adding the `sx` prop is similar to adding system props; import the default export from the `sx` module, add it to your style definition, and add the appropriate prop types. **The `sx` prop should go at the _very end_ of your style definition.**
To add the `sx` prop to your component: import the default export from the `sx` module, add it to your style definition, and add the appropriate prop types. **The `sx` prop should go at the _very end_ of your style definition.**

```jsx
import {COMMON} from './constants'
import sx from './sx'
```tsx
import sx, {SxProp} from './sx'

const Component = styled.div`
const Component = styled.div<SxProp>`
// additional styles here
${COMMON};
${sx};
`
```
Expand Down Expand Up @@ -175,15 +145,16 @@ See [`src/__tests__/example.js`](src/__tests__/example.js) for examples of ways

### TypeScript support

Several of the projects that consume Primer Components are written in TypeScript. Though Primer Components is not currently written in TS, we do export type definitions in order to make Primer Components compatible with other TS projects.
Primer React is written in TypeScript. We include type definitions in our built artifacts. To check types, run the `typecheck` script:

Whenever adding new components or modifying the props of an existing component, **please make sure to update the type definition** in `index.d.ts`! This is super important to make sure we don't break compatibility :)
```
npm run typecheck
```

### Additional resources

- [Primer Components Philosophy](https://primer.style/components/philosophy)
- [Primer Components Core Concepts](https://primer.style/components/core-concepts)
- [Primer Components System Props](https://primer.style/components/system-props)
- [Styled Components docs](https://styled-components.com/)
- [Styled System docs](https://styled-system.com/)

Expand All @@ -206,7 +177,6 @@ After opening a pull request, a member of the design systems team will add the a
- If it's a new component, does the component make sense to add to Primer Components? (Ideally this is discussed before the pull request stage, please reach out to a DS member if you aren't sure if a component should be added to Primer Components!)
- Does the component follow our [Primer Components code style](#component-patterns)?
- Does the component use theme values for most CSS values?
- Does the component have the [correct system props implemented](#adding-system-props)?
- Is the component API intuitive?
- Does the component have the appropriate [type definitions in `index.d.ts`](#typescript-support)?
- Is the component documented accurately?
Expand Down Expand Up @@ -251,20 +221,6 @@ Make sure to run `npm install` from inside the `docs/` subfolder _as well as_ th

Ensure you are using the latest minor of Node.js for the major version specified in the `.nvmrc` file. For example, if `.nvmrc` contains `8`, make sure you're using the latest version of Node.js with the major version of 8.

## Glossary

### System props

System props are style functions that provide one or more props, and can be passed directly the return value of [styled-components]'s `styled()` function:

```jsx
import styled from 'styled-components'
import {space} from 'styled-system'
const SpaceDiv = styled.div`
${space}
`
```

[classnames]: https://www.npmjs.com/package/classnames
[spread syntax]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax
[styled-system]: https://styled-system.com
Expand Down
11 changes: 6 additions & 5 deletions docs/content/BranchName.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@ BranchName is a label-type component rendered as an `<a>` tag by default with mo

## Props

| Name | Type | Default | Description |
| :--- | :----- | :-----: | :---------------------------------- |
| as | String | `<a>` | sets the HTML tag for the component |
| href | String | | a URL to link the component to |
| Name | Type | Default | Description |
| :--- | :---------------- | :-----: | :----------------------------------- |
| as | String | `<a>` | sets the HTML tag for the component |
| href | String | | a URL to link the component to |
| sx | SystemStyleObject | {} | Style to be applied to the component |

## Component status

<ComponentChecklist
items={{
items={{
propsDocumented: true,
noUnnecessaryDeps: true,
adaptsToThemes: true,
Expand Down
12 changes: 4 additions & 8 deletions docs/content/Details.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,11 @@ In previous versions of Primer React Components, we allowed users to pass in a c
</State>
```
## `Details` System props
## Details props
<Note variant="warning">
System props are deprecated in all components except [Box](/Box). Please use the [`sx` prop](/overriding-styles) instead.
</Note>
Details components get `COMMON` system props. Read our [System Props](/system-props) doc page for a full list of available props.
| Name | Type | Default | Description |
| :--- | :---------------- | :-----: | :----------------------------------- |
| sx | SystemStyleObject | {} | Style to be applied to the component |
## `useDetails` hook configuration options
Expand Down
15 changes: 5 additions & 10 deletions docs/content/Heading.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,12 @@ The Heading component will render an html `h2` tag without any default styling.
## Default example

```jsx live
<Heading fontSize={1} mb={2}>
H2 heading with fontSize={1}
</Heading>
<Heading sx={{fontSize: 1, mb: 2}}>H2 heading with fontSize={1}</Heading>
```

## System props

Heading components get `TYPOGRAPHY` and `COMMON` system props. Read our [System Props](/system-props) doc page for a full list of available props.

## Component props

| Prop name | Type | Description |
| :-------- | :---------------------- | :---------------------------------- |
| as | String or React element | sets the HTML tag for the component |
| Name | Type | Default | Description |
| :--- | :---------------------- | :-----: | :----------------------------------- |
| as | String or React element | | sets the HTML tag for the component |
| sx | SystemStyleObject | {} | Style to be applied to the component |
13 changes: 6 additions & 7 deletions docs/content/Label.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@ The Label component is used to add contextual metadata to a design. Visually it

## Component props

| Name | Type | Default | Description |
| :--------- | :---------------- | :--------------------: | :----------------------------------------------------------------------------- |
| outline | Boolean | | Creates an outline style label |
| variant | String | 'medium' | Can be one of `small`, `medium` (default), `large` or `xl` . |
| dropshadow | Boolean | | Adds a dropshadow to the label |
| bg | String | 'label.primary.border' | Part of the `COMMON` system props, used to change the background of the label. |
| sx | SystemStyleObject | {} | Style to be applied to the component |
| Name | Type | Default | Description |
| :--------- | :---------------- | :------: | :----------------------------------------------------------- |
| outline | Boolean | | Creates an outline style label |
| variant | String | 'medium' | Can be one of `small`, `medium` (default), `large` or `xl` . |
| dropshadow | Boolean | | Adds a dropshadow to the label |
| sx | SystemStyleObject | {} | Style to be applied to the component |
13 changes: 7 additions & 6 deletions docs/content/ProgressBar.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ If you'd like to use ProgressBar inline, pass the `inline` boolean prop & **be s

## Component props

| Name | Type | Default | Description |
| :------- | :------ | :-----------------: | :------------------------------------------------------------------------------------------------------------------------------- |
| progress | Number | | Used to set the size of the green bar |
| barSize | String | 'default' | Controls the height of the progress bar. Can be 'small', 'large', or 'default'. If omitted, height is set to the default height. |
| inline | Boolean | false | Styles the progress bar inline |
| bg | String | 'bg.successInverse' | Set the progress bar color, defaults to bg-green |
| Name | Type | Default | Description |
| :------- | :---------------- | :-----------------: | :------------------------------------------------------------------------------------------------------------------------------- |
| progress | Number | | Used to set the size of the green bar |
| barSize | String | 'default' | Controls the height of the progress bar. Can be 'small', 'large', or 'default'. If omitted, height is set to the default height. |
| inline | Boolean | false | Styles the progress bar inline |
| bg | String | 'bg.successInverse' | Set the progress bar color, defaults to bg-green |
| sx | SystemStyleObject | {} | Style to be applied to the component |
6 changes: 0 additions & 6 deletions docs/content/Text.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ The Text component is a wrapper component that will apply typography styles to t

## System props

<Note variant="warning">

System props are deprecated in all components except [Box](/Box). Please use the [`sx` prop](/overriding-styles) instead.

</Note>

Text components get `TYPOGRAPHY` and `COMMON` system props. Read our [System Props](/system-props) doc page for a full list of available props.

## Component props
Expand Down
2 changes: 1 addition & 1 deletion docs/content/system-props.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {PropsList, COMMON, LAYOUT, BORDER, TYPOGRAPHY, FLEX, POSITION, GRID} fro

<Note variant="warning">

System props are deprecated in all components except [Box](/Box). Please use the [`sx` prop](/overriding-styles) instead.
System props are deprecated in all components except [Box](/Box) and [Text](/Text). Please use the [`sx` prop](/overriding-styles) instead.

</Note>

Expand Down
6 changes: 2 additions & 4 deletions src/Avatar.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import styled from 'styled-components'
import {COMMON, get, SystemCommonProps} from './constants'
import {get} from './constants'
import sx, {SxProp} from './sx'
import {ComponentProps} from './utils/types'

Expand All @@ -12,8 +12,7 @@ type StyledAvatarProps = {
src: string
/** Provide alt text when the Avatar is used without the user's name next to it. */
alt?: string
} & SystemCommonProps &
SxProp
} & SxProp

function getBorderRadius({size, square}: StyledAvatarProps) {
if (square) {
Expand All @@ -32,7 +31,6 @@ const Avatar = styled.img.attrs<StyledAvatarProps>(props => ({
line-height: ${get('lineHeights.condensedUltra')};
vertical-align: middle;
border-radius: ${props => getBorderRadius(props)};
${COMMON};
${sx}
`

Expand Down
6 changes: 3 additions & 3 deletions src/BranchName.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import styled from 'styled-components'
import {COMMON, get, SystemCommonProps} from './constants'
import {get} from './constants'
import sx, {SxProp} from './sx'
import {ComponentProps} from './utils/types'

const BranchName = styled.a<SystemCommonProps & SxProp>`
const BranchName = styled.a<SxProp>`
display: inline-block;
padding: 2px 6px;
font-size: ${get('fontSizes.0')};
font-family: ${get('fonts.mono')};
color: ${get('colors.fg.muted')};
background-color: ${get('colors.accent.subtle')};
border-radius: ${get('radii.2')};
${COMMON};
${sx};
`

Expand Down
6 changes: 1 addition & 5 deletions src/Details.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import styled from 'styled-components'
import {COMMON, SystemCommonProps} from './constants'
import sx, {SxProp} from './sx'
import {ComponentProps} from './utils/types'

type StyledDetailsProps = SystemCommonProps & SxProp

const Details = styled.details<StyledDetailsProps>`
const Details = styled.details<SxProp>`
& > summary {
list-style: none;
}
& > summary::-webkit-details-marker {
display: none;
}
${COMMON}
${sx};
`

Expand Down
11 changes: 2 additions & 9 deletions src/Heading.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
import styled from 'styled-components'
import {COMMON, get, SystemCommonProps, SystemTypographyProps, TYPOGRAPHY} from './constants'
import {get} from './constants'
import sx, {SxProp} from './sx'
import theme from './theme'
import {ComponentProps} from './utils/types'

const Heading = styled.h2<SystemTypographyProps & SystemCommonProps & SxProp>`
const Heading = styled.h2<SxProp>`
font-weight: ${get('fontWeights.bold')};
font-size: ${get('fontSizes.5')};
margin: 0;
${TYPOGRAPHY};
${COMMON};
${sx};
`

Heading.defaultProps = {
theme
}

export type HeadingProps = ComponentProps<typeof Heading>
export default Heading
Loading

0 comments on commit 6f0712d

Please sign in to comment.