Skip to content

[WIP] Fix misc issues with Dialog v2 #3443

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

Closed
wants to merge 5 commits into from
Closed
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/silver-olives-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Add the experimental folder to the npm package for @primer/react
2 changes: 1 addition & 1 deletion docs/content/ActionMenu.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
componentId: action_menu
title: ActionMenu
status: Beta
a11yReviewed: true
a11yReviewed: false
source: https://github.com/primer/react/tree/main/src/ActionMenu.tsx
storybook: '/react/storybook?path=/story/components-actionmenu'
description: An ActionMenu is an ActionList-based component for creating a menu of actions that expands through a trigger button.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"index.d.ts",
"deprecated/package.json",
"drafts/package.json",
"experimental/package.json",
"!lib/__tests__",
"!lib/stories",
"!lib-esm/__tests__",
Expand Down
53 changes: 15 additions & 38 deletions src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
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 {Box, Button, ButtonProps, IconButton, Text} from '../'
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 Down Expand Up @@ -323,21 +321,15 @@ const Header = styled.div<SxProp>`
flex-shrink: 0;
`

const Title = styled.h1<SxProp>`
font-size: ${get('fontSizes.1')};
font-weight: ${get('fontWeights.bold')};
margin: 0; /* override default margin */
${sx};
`

const Subtitle = styled.h2<SxProp>`
font-size: ${get('fontSizes.0')};
color: ${get('colors.fg.muted')};
margin: 0; /* override default margin */
margin-top: ${get('space.1')};
// TODO: check if this is an appropriate use of `h1`
const Title: React.FC<React.PropsWithChildren<React.ComponentPropsWithoutRef<'h1'> & SxProp>> = ({sx, ...props}) => (
<Text as="h2" margin="0" fontSize={1} sx={sx} {...props} />
)

${sx};
`
// TODO: check if this is an appropriate use of `h2`
const Subtitle: React.FC<React.PropsWithChildren<React.ComponentPropsWithoutRef<'h2'> & SxProp>> = ({sx, ...props}) => (
<Text as="h2" color="fg.muted" margin="0" fontWeight="normal" fontSize={0} sx={sx} {...props} />
)

const Body = styled.div<SxProp>`
flex-grow: 1;
Expand Down Expand Up @@ -368,8 +360,8 @@ const Footer = styled.div<SxProp>`

const buttonTypes = {
normal: Button,
primary: ButtonPrimary,
danger: ButtonDanger,
primary: (props: ButtonProps) => <Button variant="primary" {...props} />,
danger: (props: ButtonProps) => <Button variant="danger" {...props} />,
}
const Buttons: React.FC<React.PropsWithChildren<{buttons: DialogButtonProps[]}>> = ({buttons}) => {
const autoFocusRef = useProvidedRefOrCreate<HTMLButtonElement>(buttons.find(button => button.autoFocus)?.ref)
Expand All @@ -393,7 +385,6 @@ const Buttons: React.FC<React.PropsWithChildren<{buttons: DialogButtonProps[]}>>
<ButtonElement
key={index}
{...buttonProps}
variant={buttonType}
ref={autoFocus && autoFocusCount === 0 ? (autoFocusCount++, autoFocusRef) : null}
>
{content}
Expand All @@ -403,24 +394,10 @@ const Buttons: React.FC<React.PropsWithChildren<{buttons: DialogButtonProps[]}>>
</>
)
}
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>
)
}

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

/**
* A dialog is a type of overlay that can be used for confirming actions, asking
Expand Down
3 changes: 2 additions & 1 deletion src/stories/Dialog.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import {BaseStyles, ThemeProvider, Box} from '..'
import {Button} from '../Button'
import {Dialog, DialogProps, DialogWidth, DialogHeight} from '../Dialog/Dialog'

// TODO: update this title and all other draft component titles to "Drafts/{component name}"
export default {
title: 'Components/Dialog',
title: 'Drafts/Components/Dialog',
component: Dialog,
decorators: [
Story => {
Expand Down