Skip to content
Closed
5 changes: 5 additions & 0 deletions .changeset/orange-mugs-pay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Dialog: Enhance the design by implementing our new Button in the footer and the IconButton in the header, and setting subtitles to regular.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion src/Dialog.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {Dialog, DialogWidth, DialogHeight} from './Dialog/Dialog'
/* Dialog Version 1? */

export default {
title: 'Components/DialogV1',
title: 'Components/Dialog (deprecated)',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you think it'd be better to move this to the deprecated folder?

@maximedegreve maximedegreve Nov 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea that's even more clear!

component: Dialog,
decorators: [
Story => {
Expand Down
27 changes: 27 additions & 0 deletions src/Dialog/Dialog.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,33 @@ export const StressTest = ({width, height, subtitle}: DialogStoryProps) => {
)
}

export const SmallButtons = ({width, height}: DialogStoryProps) => {
const [isOpen, setIsOpen] = useState(false)
const buttonRef = useRef<HTMLButtonElement>(null)
const onDialogClose = useCallback(() => setIsOpen(false), [])
return (
<>
<Button ref={buttonRef} onClick={() => setIsOpen(!isOpen)}>
Show dialog
</Button>
{isOpen && (
<Dialog
title="Your title"
onClose={onDialogClose}
width={width}
height={height}
footerButtons={[
{buttonType: 'normal', content: 'Cancel', onClick: onDialogClose, size: 'small'},
{buttonType: 'primary', content: 'Submit', autoFocus: true, size: 'small'},
]}
>
{lipsum}
</Dialog>
)}
</>
)
}

// repro for https://github.com/github/primer/issues/2480
export const ReproMultistepDialogWithConditionalFooter = ({width, height}: DialogStoryProps) => {
const [isOpen, setIsOpen] = useState(false)
Expand Down
45 changes: 13 additions & 32 deletions src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import React, {useCallback, useEffect, useRef, useState} from 'react'
import styled from 'styled-components'
import Button, {ButtonPrimary, ButtonDanger, ButtonProps} from '../deprecated/Button'
import Box from '../Box'
import {IconButton, Button, ButtonProps} from '../index'

import {get} from '../constants'
import {useOnEscapePress, useProvidedRefOrCreate} from '../hooks'
import {useFocusTrap} from '../hooks/useFocusTrap'
import sx, {SxProp} from '../sx'
import Octicon from '../Octicon'
import {XIcon} from '@primer/octicons-react'
import {useFocusZone} from '../hooks/useFocusZone'
import {FocusKeys} from '@primer/behaviors'
Expand All @@ -26,7 +26,7 @@ export type DialogButtonProps = Omit<ButtonProps, 'content'> & {
/**
* The type of Button element to use
*/
buttonType?: 'normal' | 'primary' | 'danger'
buttonType?: 'normal' | 'default' | 'primary' | 'danger' // 'normal' to avoid a breaking change.

/**
* The Button's inner text
Expand Down Expand Up @@ -215,6 +215,10 @@ const StyledDialog = styled.div<StyledDialogProps>`
${sx};
`

const CloseButton = ({onClose}: {onClose: () => void}) => {
return <IconButton aria-label="Close" onClick={onClose} variant="invisible" icon={XIcon} />
}

const DefaultHeader: React.FC<React.PropsWithChildren<DialogHeaderProps>> = ({
dialogLabelId,
title,
Expand All @@ -232,7 +236,7 @@ const DefaultHeader: React.FC<React.PropsWithChildren<DialogHeaderProps>> = ({
<Dialog.Title id={dialogLabelId}>{title ?? 'Dialog'}</Dialog.Title>
{subtitle && <Dialog.Subtitle id={dialogDescriptionId}>{subtitle}</Dialog.Subtitle>}
</Box>
<Dialog.CloseButton onClose={onCloseClick} />
<CloseButton onClose={onCloseClick} />
</Box>
</Dialog.Header>
)
Expand Down Expand Up @@ -351,6 +355,7 @@ const Title = styled.h1<SxProp>`
const Subtitle = styled.h2<SxProp>`
font-size: ${get('fontSizes.0')};
color: ${get('colors.fg.muted')};
font-weight: ${get('fontWeights.normal')};
margin: 0; /* override default margin */
margin-top: ${get('space.1')};

Expand Down Expand Up @@ -378,11 +383,6 @@ const Footer = styled.div<SxProp>`
${sx};
`

const buttonTypes = {
normal: Button,
primary: ButtonPrimary,
danger: ButtonDanger,
}
const Buttons: React.FC<React.PropsWithChildren<{buttons: DialogButtonProps[]}>> = ({buttons}) => {
const autoFocusRef = useProvidedRefOrCreate<HTMLButtonElement>(buttons.find(button => button.autoFocus)?.ref)
let autoFocusCount = 0
Expand All @@ -400,39 +400,20 @@ const Buttons: React.FC<React.PropsWithChildren<{buttons: DialogButtonProps[]}>>
<>
{buttons.map((dialogButtonProps, index) => {
const {content, buttonType = 'normal', autoFocus = false, ...buttonProps} = dialogButtonProps
const ButtonElement = buttonTypes[buttonType]
return (
<ButtonElement
<Button
key={index}
variant={buttonType === 'normal' ? 'default' : buttonType}
{...buttonProps}
variant={buttonType}
ref={autoFocus && autoFocusCount === 0 ? (autoFocusCount++, autoFocusRef) : null}
>
{content}
</ButtonElement>
</Button>
)
})}
</>
)
}
const DialogCloseButton = styled(Button)`
border-radius: 4px;
background: transparent;
border: 0;
vertical-align: middle;
color: ${get('colors.fg.muted')};
padding: ${get('space.2')};
align-self: flex-start;
line-height: normal;
box-shadow: none;
`
const CloseButton: React.FC<React.PropsWithChildren<{onClose: () => void}>> = ({onClose}) => {
return (
<DialogCloseButton aria-label="Close" onClick={onClose}>
<Octicon icon={XIcon} />
</DialogCloseButton>
)
}

/**
* A dialog is a type of overlay that can be used for confirming actions, asking
Expand Down Expand Up @@ -461,6 +442,6 @@ export const Dialog = Object.assign(_Dialog, {
Subtitle,
Body,
Footer,
Buttons,
CloseButton,
Buttons,
})