Extract Dialog header into separate component

docs/content/Dialog.md

@@ -8,26 +8,33 @@ The dialog component is used for all modals. It renders on top of the rest of th
 
 **Note:** You'll need to manage the `isOpen` state in a wrapper component of your own. For documentation purposes only we've created a mock `State` to handle this, but you should handle the state in the component you consume `Dialog` in or in a wrapper component.
 
-
+```jsx live
 <State default={false}>
   {([isOpen, setIsOpen]) => (
     <>
       <Button onClick={() => setIsOpen(true)}>Open</Button>
-      <Dialog title="Title" isOpen={isOpen} onDismiss={() => setIsOpen(false)}>
+      <Dialog isOpen={isOpen} onDismiss={() => setIsOpen(false)}>
+        <Dialog.Header>Title</Dialog.Header>
         <Box p={3}>
           <Text fontFamily="sans-serif">Some content</Text>
         </Box>
       </Dialog>
     </>
   )}
 </State>
+```
+
+You can also pass any non-text content into the header:
 
-```jsx
+```jsx live
 <State default={false}>
   {([isOpen, setIsOpen]) => (
     <>
       <Button onClick={() => setIsOpen(true)}>Open</Button>
-      <Dialog title="Title" isOpen={isOpen} onDismiss={() => setIsOpen(false)}>
+      <Dialog isOpen={isOpen} onDismiss={() => setIsOpen(false)}>
+        <Dialog.Header>
+          <Octicon icon={Zap} />
+        </Dialog.Header>
         <Box p={3}>
           <Text fontFamily="sans-serif">Some content</Text>
         </Box>
@@ -37,23 +44,15 @@ The dialog component is used for all modals. It renders on top of the rest of th
 </State>
 ```
 
-
-You can also pass any React node as the title to override the styling:
-
-```jsx
-<Dialog title={<Text fontSize={6}>Title</Text>}>
-```
-
 ## System props
 
-Dialog components get the `COMMON` and `LAYOUT` categories of system props. Read our [System Props](/system-props) doc page for a full list of available props.
+`Dialog` components get the `COMMON` and `LAYOUT` categories of system props. `Dialog.Header` components get `COMMON`, `LAYOUT`, and `FLEX` system props. Read our [System Props](/system-props) doc page for a full list of available props.
 
 ## Component props
 
-These props are **all required**.
-
 | Prop name | Type | Description |
 | :- | :- | :- |
-| title | String or Node | The title shown in the header |
-| isOpen | Boolean | Handles opening and closing the dialog |
-| onDismiss | Function | Should close the dialog |
+| isOpen | Boolean | Set whether or not the dialog is shown |
+| onDismiss | Function | A user-provided function that should close the dialog |
+
+`Dialog.Header` does not take any non-system props.

index.d.ts

@@ -389,12 +389,15 @@ declare module '@primer/components' {
   export const themeGet: (key: any) => any
 
   export interface DialogProps extends CommonProps {
-    title: string
     isOpen: boolean
     onDismiss: () => unknown
   }
 
-  export const Dialog: React.FunctionComponent<DialogProps>
+  export interface DialogHeaderProps extends FlexProps {}
+
+  export const Dialog: React.FunctionComponent<DialogProps> & {
+    Header: React.FunctionComponent<DialogHeaderProps>
+  }
 
   export interface LabelGroupProps extends CommonProps, Omit<React.HTMLAttributes<HTMLSpanElement>, 'color'> {}
 

src/Dialog.js

@@ -26,6 +26,7 @@ export const StyledDialog = styled(ReachDialog)`
   box-shadow: 0px 4px 32px rgba(0, 0, 0, 0.35);
   border-radius: 4px;
   padding: 0 !important;
+  position: relative;
 
   @media screen and (max-width: 750px) {
     width: 100vw !important;
@@ -45,14 +46,13 @@ const UnstyledButton = styled(Flex).attrs({
   background: none;
   border: none;
   padding: 0;
+
+  position: absolute;
+  top: 16px;
+  right: 16px;
 `
 
-const DialogHeader = styled(Flex).attrs({
-  p: 3,
-  bg: 'gray.1',
-  justifyContent: 'space-between',
-  alignItems: 'center'
-})`
+const DialogHeaderBase = styled(Flex)`
   border-radius: 4px 4px 0px 0px;
   border-bottom: 1px solid #dad5da;
 
@@ -61,22 +61,29 @@ const DialogHeader = styled(Flex).attrs({
   }
 `
 
-const Dialog = ({title, children, ...props}) => {
+function DialogHeader({theme, children, ...rest}) {
+  if (React.Children.toArray(children).every(ch => typeof ch === 'string')) {
+    children = (
+      <Text theme={theme} color="gray.9" fontSize={1} fontWeight="bold" fontFamily="sans-serif">
+        {children}
+      </Text>
+    )
+  }
+
+  return (
+    <DialogHeaderBase theme={theme} p={3} {...rest}>
+      {children}
+    </DialogHeaderBase>
+  )
+}
+
+const Dialog = ({children, ...props}) => {
   return (
     <>
       <StyledDialog {...props}>
-        <DialogHeader>
-          {typeof title === 'string' ? (
-            <Text color="gray.9" fontSize={1} fontWeight="bold" fontFamily="sans-serif">
-              {title}
-            </Text>
-          ) : (
-            title
-          )}
-          <UnstyledButton onClick={props.onDismiss}>
-            <StyledOcticon icon={X} />
-          </UnstyledButton>
-        </DialogHeader>
+        <UnstyledButton onClick={props.onDismiss}>
+          <StyledOcticon icon={X} />
+        </UnstyledButton>
         {children}
       </StyledDialog>
       <ReachGlobalStyle />
@@ -93,8 +100,17 @@ Dialog.propTypes = {
   children: PropTypes.node.isRequired,
   isOpen: PropTypes.bool.isRequired,
   onDismiss: PropTypes.func.isRequired,
-  theme: PropTypes.object,
-  title: PropTypes.oneOfType([PropTypes.string, PropTypes.node]).isRequired
+  theme: PropTypes.object
+}
+
+DialogHeader.defaultProps = {
+  backgroundColor: 'gray.1',
+  theme
+}
+
+DialogHeader.propTypes = {
+  ...Flex.propTypes
 }
 
+Dialog.Header = DialogHeader
 export default Dialog

src/__tests__/Dialog.js

@@ -8,7 +8,8 @@ import 'babel-polyfill'
 expect.extend(toHaveNoViolations)
 
 const comp = (
-  <Dialog title="Title" isOpen onDismiss={() => null}>
+  <Dialog isOpen onDismiss={() => null}>
+    <Dialog.Header>Title</Dialog.Header>
     <Box p={3}>
       <Text fontFamily="sans-serif">Some content</Text>
     </Box>
@@ -22,4 +23,8 @@ describe('Dialog', () => {
     expect(results).toHaveNoViolations()
     cleanup()
   })
+
+  it('renders consistently', () => {
+    expect(comp).toMatchSnapshot()
+  })
 })