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
5 changes: 5 additions & 0 deletions .changeset/old-taxis-argue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': major
---

Dialog2 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.
1 change: 1 addition & 0 deletions docs/content/Dialog2.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ By default, the Dialog component implements the design and interactions defined
| role | `"dialog" │ "alertdialog"` | `"dialog"` | The ARIA role given to the dialog element. More info: [dialog](https://www.w3.org/TR/wai-aria-practices-1.1/#dialog_modal), [alertdialog](https://www.w3.org/TR/wai-aria-practices-1.1/#alertdialog) |
| width | `"small" │ "medium" │ "large" │ "xlarge"` | `"xlarge"` | The fixed width of the dialog. |
| height | `"small" │ "large" │ "auto"` | `"auto"` | The fixed height of the dialog, or, auto to adjust the height based on its contents. |
| sx | `SystemStyleObject` | {} | Style to be applied to the component |

### DialogHeaderProps

Expand Down
33 changes: 22 additions & 11 deletions src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, {useCallback, useEffect, useRef, useState} from 'react'
import styled from 'styled-components'
import Button, {ButtonPrimary, ButtonDanger, ButtonProps} from '../Button'
import Box from '../Box'
import {get, SystemCommonProps, SystemPositionProps, COMMON, POSITION} from '../constants'
import {get} from '../constants'
import {useOnEscapePress, useProvidedRefOrCreate} from '../hooks'
import {useFocusTrap} from '../hooks/useFocusTrap'
import sx, {SxProp} from '../sx'
Expand Down Expand Up @@ -180,12 +180,12 @@ const widthMap = {
export type DialogWidth = keyof typeof widthMap
export type DialogHeight = keyof typeof heightMap

interface StyledDialogProps {
type StyledDialogProps = {
width?: DialogWidth
height?: DialogHeight
}
} & SxProp

const StyledDialog = styled.div<StyledDialogProps & SystemCommonProps & SystemPositionProps & SxProp>`
const StyledDialog = styled.div<StyledDialogProps>`
display: flex;
flex-direction: column;
background-color: ${get('colors.canvas.overlay')};
Expand All @@ -210,8 +210,6 @@ const StyledDialog = styled.div<StyledDialogProps & SystemCommonProps & SystemPo
}
}

${COMMON};
${POSITION};
${sx};
`

Expand Down Expand Up @@ -309,27 +307,37 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
})
_Dialog.displayName = 'Dialog'

const Header = styled(Box).attrs({as: 'header'})`
const Header = styled.div.attrs<SxProp>({as: 'header'})`
box-shadow: 0 1px 0 ${get('colors.border.default')};
padding: ${get('space.2')};
z-index: 1;
flex-shrink: 0;
`
const Title = styled(Box)`

const Title = styled.div<SxProp>`
font-size: ${get('fontSizes.1')};
font-weight: ${get('fontWeights.bold')};

${sx};
`
const Subtitle = styled(Box)`

const Subtitle = styled.div<SxProp>`
font-size: ${get('fontSizes.0')};
margin-top: ${get('space.1')};
color: ${get('colors.fg.muted')};

${sx};
`
const Body = styled(Box)`

const Body = styled.div<SxProp>`
flex-grow: 1;
overflow: auto;
padding: ${get('space.3')};

${sx};
`
const Footer = styled(Box).attrs({as: 'footer'})`

const Footer = styled.div.attrs<SxProp>({as: 'footer'})`
box-shadow: 0 -1px 0 ${get('colors.border.default')};
padding: ${get('space.3')};
display: flex;
Expand All @@ -344,7 +352,10 @@ const Footer = styled(Box).attrs({as: 'footer'})`
margin-left: 0;
}
}

${sx};
`

const buttonTypes = {
normal: Button,
primary: ButtonPrimary,
Expand Down
11 changes: 11 additions & 0 deletions src/__tests__/Dialog2.types.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import React from 'react'
import {Dialog} from '../Dialog/Dialog'

export function shouldAcceptCallWithNoProps() {
return <Dialog onClose={() => null} />
}

export function shouldNotAcceptSystemProps() {
// @ts-expect-error system props should not be accepted
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, I didn't know @ts-expect-error was a thing. Cool!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I love writing type tests now! Given how many regressions I've run into around types, it's nice to have some more confidence!

return <Dialog onClose={() => null} backgroundColor="tomato" />
}
4 changes: 2 additions & 2 deletions src/stories/Dialog.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,11 @@ function CustomHeader({
return null
}
function CustomBody({children}: React.PropsWithChildren<DialogProps>) {
return <Dialog.Body bg="danger.subtle">{children}</Dialog.Body>
return <Dialog.Body sx={{bg: 'danger.subtle'}}>{children}</Dialog.Body>
}
function CustomFooter({footerButtons}: React.PropsWithChildren<DialogProps>) {
return (
<Dialog.Footer bg="attention.subtle">
<Dialog.Footer sx={{bg: 'attention.subtle'}}>
{footerButtons ? <Dialog.Buttons buttons={footerButtons} /> : null}
</Dialog.Footer>
)
Expand Down