feat(Dialog): remove support for sx

.changeset/sweet-results-smell.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': major
+---
+
+Remove support for `sx` from the Dialog component and sub-components

e2e/components/Dialog.test.ts

@@ -27,18 +27,6 @@ const stories = [
     title: 'Position sidesheet',
     id: 'components-dialog-features--side-sheet',
   },
-  {
-    title: 'Dev: With Css',
-    id: 'components-dialog-dev--with-css',
-  },
-  {
-    title: 'Dev: With Sx',
-    id: 'components-dialog-dev--with-sx',
-  },
-  {
-    title: 'Dev: With Sx And Css',
-    id: 'components-dialog-dev--with-sx-and-css',
-  },
 ] as const
 
 test.describe('Dialog', () => {

packages/react/src/Dialog/Dialog.dev.stories.tsx

@@ -87,113 +87,6 @@ export const WithCss = ({width, height, subtitle}: DialogStoryProps) => {
   )
 }
 
-function SxHeader({title, subtitle, dialogLabelId}: React.PropsWithChildren<DialogProps & {dialogLabelId: string}>) {
-  if (typeof title === 'string' && typeof subtitle === 'string') {
-    return (
-      <Dialog.Header sx={{color: 'accent.emphasis'}}>
-        <h1 id={dialogLabelId}>{title.toUpperCase()}</h1>
-      </Dialog.Header>
-    )
-  }
-  return null
-}
-function SxBody({children}: React.PropsWithChildren<DialogProps>) {
-  return <Dialog.Body sx={{color: 'danger.emphasis'}}>{children}</Dialog.Body>
-}
-function SxFooter({footerButtons}: React.PropsWithChildren<DialogProps>) {
-  return (
-    <Dialog.Footer sx={{fontFamily: 'Times New Roman'}}>
-      {footerButtons ? <Dialog.Buttons buttons={footerButtons} /> : null}
-    </Dialog.Footer>
-  )
-}
-export const WithSx = ({width, height, subtitle}: DialogStoryProps) => {
-  const [isOpen, setIsOpen] = useState(true)
-  const onDialogClose = useCallback(() => setIsOpen(false), [])
-  return (
-    <>
-      <Button onClick={() => setIsOpen(!isOpen)}>Show dialog</Button>
-      {isOpen && (
-        <Dialog
-          title="My Dialog"
-          subtitle={subtitle ? 'This is a subtitle!' : undefined}
-          width={width}
-          height={height}
-          renderHeader={SxHeader}
-          renderBody={SxBody}
-          renderFooter={SxFooter}
-          onClose={onDialogClose}
-          footerButtons={[
-            {buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
-            {buttonType: 'primary', content: 'Proceed'},
-          ]}
-          sx={{borderColor: 'accent.emphasis', borderWidth: '1px', borderStyle: 'solid', animation: 'none !important'}}
-        >
-          {lipsum}
-        </Dialog>
-      )}
-    </>
-  )
-}
-
-function SxAndCssHeader({
-  title,
-  subtitle,
-  dialogLabelId,
-}: React.PropsWithChildren<DialogProps & {dialogLabelId: string}>) {
-  if (typeof title === 'string' && typeof subtitle === 'string') {
-    return (
-      <Dialog.Header sx={{color: 'accent.emphasis'}} className="testCustomClassnameColor">
-        <h1 id={dialogLabelId}>{title.toUpperCase()}</h1>
-      </Dialog.Header>
-    )
-  }
-  return null
-}
-function SxAndCssBody({children}: React.PropsWithChildren<DialogProps>) {
-  return (
-    <Dialog.Body sx={{color: 'danger.emphasis'}} className="testCustomClassnameColor">
-      {children}
-    </Dialog.Body>
-  )
-}
-function SxAndCssFooter({footerButtons}: React.PropsWithChildren<DialogProps>) {
-  return (
-    <Dialog.Footer sx={{fontFamily: 'Times New Roman'}} className="testCustomClassnameMono">
-      {footerButtons ? <Dialog.Buttons buttons={footerButtons} /> : null}
-    </Dialog.Footer>
-  )
-}
-export const WithSxAndCss = ({width, height, subtitle}: DialogStoryProps) => {
-  const [isOpen, setIsOpen] = useState(true)
-  const onDialogClose = useCallback(() => setIsOpen(false), [])
-  return (
-    <>
-      <Button onClick={() => setIsOpen(!isOpen)}>Show dialog</Button>
-      {isOpen && (
-        <Dialog
-          title="My Dialog"
-          subtitle={subtitle ? 'This is a subtitle!' : undefined}
-          width={width}
-          height={height}
-          renderHeader={SxAndCssHeader}
-          renderBody={SxAndCssBody}
-          renderFooter={SxAndCssFooter}
-          onClose={onDialogClose}
-          footerButtons={[
-            {buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
-            {buttonType: 'primary', content: 'Proceed'},
-          ]}
-          sx={{borderColor: 'border.accent', borderWidth: '1px', borderStyle: 'solid', animation: 'none !important'}}
-          className="testCustomClassnameBorder"
-        >
-          {lipsum}
-        </Dialog>
-      )}
-    </>
-  )
-}
-
 export const WithScrollBody = () => {
   const [isOpen, setIsOpen] = useState(false)
   const buttonRef = useRef<HTMLButtonElement>(null)

packages/react/src/Dialog/Dialog.features.stories.tsx

@@ -79,11 +79,11 @@ function CustomHeader({
   return null
 }
 function CustomBody({children}: React.PropsWithChildren<DialogProps>) {
-  return <Dialog.Body sx={{bg: 'danger.subtle'}}>{children}</Dialog.Body>
+  return <Dialog.Body style={{backgroundColor: 'var(--bgColor-danger-muted)'}}>{children}</Dialog.Body>
 }
 function CustomFooter({footerButtons}: React.PropsWithChildren<DialogProps>) {
   return (
-    <Dialog.Footer sx={{bg: 'attention.subtle'}}>
+    <Dialog.Footer style={{backgroundColor: 'var(--bgColor-attention-muted)'}}>
       {footerButtons ? <Dialog.Buttons buttons={footerButtons} /> : null}
     </Dialog.Footer>
   )

packages/react/src/Dialog/Dialog.module.css

@@ -270,6 +270,18 @@ Add a border between the body and footer if:
   flex-shrink: 0;
 }
 
+.HeaderInner {
+  display: flex;
+}
+
+.HeaderContent {
+  display: flex;
+  padding-inline: var(--base-size-8);
+  padding-block: var(--base-size-6);
+  flex-direction: column;
+  flex-grow: 1;
+}
+
 .Title {
   margin: 0; /* override default margin */
   font-size: var(--text-body-size-medium);

packages/react/src/Dialog/Dialog.tsx

@@ -1,10 +1,8 @@
 import React, {useCallback, useEffect, useRef, useState, type SyntheticEvent} from 'react'
 import type {ButtonProps} from '../Button'
 import {Button, IconButton} from '../Button'
-import Box from '../Box'
 import {useOnEscapePress, useProvidedRefOrCreate} from '../hooks'
 import {useFocusTrap} from '../hooks/useFocusTrap'
-import type {SxProp} from '../sx'
 import {XIcon} from '@primer/octicons-react'
 import {useFocusZone} from '../hooks/useFocusZone'
 import {FocusKeys} from '@primer/behaviors'
@@ -17,7 +15,6 @@ import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../uti
 
 import classes from './Dialog.module.css'
 import {clsx} from 'clsx'
-import {BoxWithFallback} from '../internal/components/BoxWithFallback'
 
 /* Dialog Version 2 */
 
@@ -52,7 +49,7 @@ export type DialogButtonProps = Omit<ButtonProps, 'content'> & {
 /**
  * Props to customize the rendering of the Dialog.
  */
-export interface DialogProps extends SxProp {
+export interface DialogProps {
   /**
    * Title of the Dialog. Also serves as the aria-label for this Dialog.
    */
@@ -198,13 +195,13 @@ const DefaultHeader: React.FC<React.PropsWithChildren<DialogHeaderProps>> = ({
   }, [onClose])
   return (
     <Dialog.Header>
-      <Box display="flex">
-        <Box display="flex" px={2} py="6px" flexDirection="column" flexGrow={1}>
+      <div className={classes.HeaderInner}>
+        <div className={classes.HeaderContent}>
           <Dialog.Title id={dialogLabelId}>{title ?? 'Dialog'}</Dialog.Title>
           {subtitle && <Dialog.Subtitle id={dialogDescriptionId}>{subtitle}</Dialog.Subtitle>}
-        </Box>
+        </div>
         <Dialog.CloseButton onClose={onCloseClick} />
-      </Box>
+      </div>
     </Dialog.Header>
   )
 }
@@ -245,7 +242,6 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
     position = defaultPosition,
     returnFocusRef,
     initialFocusRef,
-    sx,
     className,
   } = props
   const dialogLabelId = useId()
@@ -311,8 +307,7 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
   return (
     <>
       <Portal>
-        <BoxWithFallback
-          as="div"
+        <div
           ref={backdropRef}
           className={classes.Backdrop}
           {...positionDataAttributes}
@@ -321,8 +316,7 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
             setLastMouseDownIsBackdrop(e.target === e.currentTarget)
           }}
         >
-          <BoxWithFallback
-            as="div"
+          <div
             ref={dialogRef}
             role={role}
             aria-labelledby={dialogLabelId}
@@ -331,58 +325,57 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
             {...positionDataAttributes}
             data-width={width}
             data-height={height}
-            sx={sx}
             className={clsx(className, classes.Dialog)}
           >
             {header}
             <ScrollableRegion aria-labelledby={dialogLabelId} className={classes.DialogOverflowWrapper}>
               {body}
             </ScrollableRegion>
             {footer}
-          </BoxWithFallback>
-        </BoxWithFallback>
+          </div>
+        </div>
       </Portal>
     </>
   )
 })
 _Dialog.displayName = 'Dialog'
 
-type StyledHeaderProps = React.ComponentProps<'div'> & SxProp
+type StyledHeaderProps = React.ComponentProps<'div'>
 
-const Header = React.forwardRef<HTMLElement, StyledHeaderProps>(function Header({className, ...rest}, forwardRef) {
-  return <BoxWithFallback as="div" ref={forwardRef} className={clsx(className, classes.Header)} {...rest} />
+const Header = React.forwardRef<HTMLDivElement, StyledHeaderProps>(function Header({className, ...rest}, forwardRef) {
+  return <div ref={forwardRef} className={clsx(className, classes.Header)} {...rest} />
 })
 Header.displayName = 'Dialog.Header'
 
-type StyledTitleProps = React.ComponentProps<'h1'> & SxProp
+type StyledTitleProps = React.ComponentProps<'h1'>
 
-const Title = React.forwardRef<HTMLElement, StyledTitleProps>(function Title({className, ...rest}, forwardRef) {
-  return <BoxWithFallback as="h1" ref={forwardRef} className={clsx(className, classes.Title)} {...rest} />
+const Title = React.forwardRef<HTMLHeadingElement, StyledTitleProps>(function Title({className, ...rest}, forwardRef) {
+  return <h1 ref={forwardRef} className={clsx(className, classes.Title)} {...rest} />
 })
 Title.displayName = 'Dialog.Title'
 
-type StyledSubtitleProps = React.ComponentProps<'h2'> & SxProp
+type StyledSubtitleProps = React.ComponentProps<'h2'>
 
-const Subtitle = React.forwardRef<HTMLElement, StyledSubtitleProps>(function Subtitle(
+const Subtitle = React.forwardRef<HTMLHeadingElement, StyledSubtitleProps>(function Subtitle(
   {className, ...rest},
   forwardRef,
 ) {
-  return <BoxWithFallback as="h2" ref={forwardRef} className={clsx(className, classes.Subtitle)} {...rest} />
+  return <h2 ref={forwardRef} className={clsx(className, classes.Subtitle)} {...rest} />
 })
 Subtitle.displayName = 'Dialog.Subtitle'
 
-type StyledBodyProps = React.ComponentProps<'div'> & SxProp
+type StyledBodyProps = React.ComponentProps<'div'>
 
-const Body = React.forwardRef<HTMLElement, StyledBodyProps>(function Body({className, ...rest}, forwardRef) {
-  return <BoxWithFallback as="div" ref={forwardRef} className={clsx(className, classes.Body)} {...rest} />
+const Body = React.forwardRef<HTMLDivElement, StyledBodyProps>(function Body({className, ...rest}, forwardRef) {
+  return <div ref={forwardRef} className={clsx(className, classes.Body)} {...rest} />
 }) as PolymorphicForwardRefComponent<'div', StyledBodyProps>
 
 Body.displayName = 'Dialog.Body'
 
-type StyledFooterProps = React.ComponentProps<'div'> & SxProp
+type StyledFooterProps = React.ComponentProps<'div'>
 
-const Footer = React.forwardRef<HTMLElement, StyledFooterProps>(function Footer({className, ...rest}, forwardRef) {
-  return <BoxWithFallback as="div" ref={forwardRef} className={clsx(className, classes.Footer)} {...rest} />
+const Footer = React.forwardRef<HTMLDivElement, StyledFooterProps>(function Footer({className, ...rest}, forwardRef) {
+  return <div ref={forwardRef} className={clsx(className, classes.Footer)} {...rest} />
 })
 Footer.displayName = 'Dialog.Footer'
 

packages/styled-react/src/__tests__/primer-react.browser.test.tsx

@@ -155,6 +155,33 @@ describe('@primer/react', () => {
     expect(window.getComputedStyle(screen.getByRole('dialog')).backgroundColor).toBe('rgb(255, 0, 0)')
   })
 
+  test('Dialog.Header supports `sx` prop', () => {
+    render(
+      <Dialog
+        onClose={() => {}}
+        renderHeader={() => <Dialog.Header data-testid="component" sx={{background: 'red'}} />}
+      />,
+    )
+    expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)')
+  })
+
+  test('Dialog.Body supports `sx` prop', () => {
+    render(
+      <Dialog onClose={() => {}} renderBody={() => <Dialog.Body data-testid="component" sx={{background: 'red'}} />} />,
+    )
+    expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)')
+  })
+
+  test('Dialog.Footer supports `sx` prop', () => {
+    render(
+      <Dialog
+        onClose={() => {}}
+        renderFooter={() => <Dialog.Footer data-testid="component" sx={{background: 'red'}} />}
+      />,
+    )
+    expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)')
+  })
+
   test('Flash supports `sx` prop', () => {
     render(<Flash data-testid="component" sx={{background: 'red'}} />)
     expect(window.getComputedStyle(screen.getByTestId('component')).backgroundColor).toBe('rgb(255, 0, 0)')

packages/styled-react/src/components/Dialog.tsx

@@ -0,0 +1,41 @@
+import {Dialog as PrimerDialog} from '@primer/react'
+import type {DialogProps as PrimerDialogProps} from '@primer/react'
+import {Box} from './Box'
+import type {SxProp} from '../sx'
+import {forwardRef, type ComponentPropsWithoutRef, type PropsWithChildren} from 'react'
+
+type DialogProps = PropsWithChildren<PrimerDialogProps> & SxProp
+
+const DialogImpl = forwardRef<HTMLDivElement, DialogProps>(function Dialog(props, ref) {
+  // @ts-expect-error - PrimerDialog is not recognized as a valid component type
+  return <Box as={PrimerDialog} ref={ref} {...props} />
+})
+
+type DialogHeaderProps = ComponentPropsWithoutRef<'div'> & SxProp
+
+const DialogHeader = forwardRef<HTMLDivElement, DialogHeaderProps>(function DialogHeader(props, ref) {
+  return <Box as={PrimerDialog.Header} ref={ref} {...props} />
+})
+
+type StyledBodyProps = React.ComponentProps<'div'> & SxProp
+
+const DialogBody = forwardRef<HTMLDivElement, StyledBodyProps>(function DialogBody(props, ref) {
+  // @ts-expect-error - PrimerDialog.Body is not recognized as a valid component type
+  return <Box as={PrimerDialog.Body} ref={ref} {...props} />
+})
+
+type StyledFooterProps = React.ComponentProps<'div'> & SxProp
+
+const DialogFooter = forwardRef<HTMLDivElement, StyledFooterProps>(function DialogFooter(props, ref) {
+  return <Box as={PrimerDialog.Footer} ref={ref} {...props} />
+})
+
+const Dialog = Object.assign(DialogImpl, {
+  Buttons: PrimerDialog.Buttons,
+  Header: DialogHeader,
+  Body: DialogBody,
+  Footer: DialogFooter,
+})
+
+export {Dialog}
+export type {DialogProps}