Add missing changes in 17.0.0

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.

docs/content/Heading.md

@@ -2,11 +2,13 @@
 title: Heading
 ---
 
-The Heading component will render an html `h1-6` tag without any default styling. Please reference the [w3 recommendations for headings](https://www.w3.org/WAI/tutorials/page-structure/headings/) to ensure your headings provide an accessible experience for screen reader users.
+The Heading component will render an html `h2` tag without any default styling. Other heading level elements `h1-h6` are available through use of the `as` prop (see [System Props](/system-props) for additional explanation). Please reference the [w3 recommendations for headings](https://www.w3.org/WAI/tutorials/page-structure/headings/) to ensure your headings provide an accessible experience for screen reader users.
+
+**Attention:** Make sure to include a valid heading element to render a Heading component other than `h2` (`<Heading as="h1">H1 Element</Heading>`).
 
 ## Default example
 ```jsx live
-<Heading fontSize={1} mb={2}>With fontSize={1}</Heading>
+<Heading fontSize={1} mb={2}>H2 heading with fontSize={1}</Heading>
 ```
 
 ## System props
@@ -15,4 +17,6 @@ Heading components get `TYPOGRAPHY` and `COMMON` system props. Read our [System
 
 ## Component props
 
-Heading does not get any additional props other than the system props mentioned above.
+| Prop name | Type    | Description                                      |
+| :-------- | :------ | :----------------------------------------------- |
+| as        | String  | sets the HTML tag for the component              |

index.d.ts

@@ -72,7 +72,9 @@ declare module '@primer/components' {
     extends BaseProps,
       CommonProps,
       TypographyProps,
-      Omit<React.HTMLAttributes<HTMLHeadingElement>, 'color'> {}
+      Omit<React.HTMLAttributes<HTMLHeadingElement>, 'color'> {
+    as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
+  }
 
   export const Heading: React.FunctionComponent<HeadingProps>
 
@@ -480,12 +482,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/Heading.js

@@ -3,7 +3,7 @@ import PropTypes from 'prop-types'
 import {TYPOGRAPHY, COMMON, get} from './constants'
 import theme from './theme'
 
-const Heading = styled.h1`
+const Heading = styled.h2`
   font-weight: ${get('fontWeights.bold')};
   font-size: ${get('fontSizes.5')};
   margin: 0;
@@ -15,9 +15,10 @@ Heading.defaultProps = {
 }
 
 Heading.propTypes = {
+  as: PropTypes.oneOf(['h1', 'h2', 'h3', 'h4', 'h5', 'h6']),
+  theme: PropTypes.object,
   ...COMMON.propTypes,
-  ...TYPOGRAPHY.propTypes,
-  theme: PropTypes.object
+  ...TYPOGRAPHY.propTypes
 }
 
 export default Heading

src/__tests__/BorderBox.js

@@ -40,6 +40,6 @@ describe('BorderBox', () => {
 
   // the test returns the box shadow value without spaces, so had to manually provide the expected string here
   it('renders box shadow', () => {
-    expect(render(<BorderBox boxShadow="small" />)).toHaveStyleRule('box-shadow', '0 1px 1px rgba(27,31,35,0.1)')
+    expect(render(<BorderBox boxShadow="small" />)).toHaveStyleRule('box-shadow', '0 1px 0 rgba(149,157,165,0.1)')
   })
 })

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()
+  })
 })

src/__tests__/Heading.js

@@ -31,8 +31,8 @@ const theme = {
 }
 
 describe('Heading', () => {
-  it('renders <h1> by default', () => {
-    expect(render(<Heading />).type).toEqual('h1')
+  it('renders <h2> by default', () => {
+    expect(render(<Heading />).type).toEqual('h2')
   })
 
   it('should have no axe violations', async () => {
@@ -94,7 +94,6 @@ describe('Heading', () => {
 
   it('respects the "fontStyle" prop', () => {
     expect(render(<Heading fontStyle="italic" />)).toHaveStyleRule('font-style', 'italic')
-    expect(render(<Heading as="i" fontStyle="normal" />)).toHaveStyleRule('font-style', 'normal')
   })
 
   it.skip('renders fontSize with f* classes using inverse scale', () => {

src/__tests__/__snapshots__/CircleBadge.js.snap

@@ -16,7 +16,7 @@ exports[`CircleBadge respects "as" prop 1`] = `
   justify-content: center;
   background-color: #fff;
   border-radius: 50%;
-  box-shadow: 0 1px 5px rgba(27,31,35,0.15);
+  box-shadow: 0 3px 6px rgba(149,157,165,0.15);
   width: 96px;
   height: 96px;
 }
@@ -42,7 +42,7 @@ exports[`CircleBadge respects the inline prop 1`] = `
   justify-content: center;
   background-color: #fff;
   border-radius: 50%;
-  box-shadow: 0 1px 5px rgba(27,31,35,0.15);
+  box-shadow: 0 3px 6px rgba(149,157,165,0.15);
   width: 96px;
   height: 96px;
 }
@@ -68,7 +68,7 @@ exports[`CircleBadge respects the variant prop 1`] = `
   justify-content: center;
   background-color: #fff;
   border-radius: 50%;
-  box-shadow: 0 1px 5px rgba(27,31,35,0.15);
+  box-shadow: 0 3px 6px rgba(149,157,165,0.15);
   width: 128px;
   height: 128px;
 }
@@ -94,7 +94,7 @@ exports[`CircleBadge uses the size prop to override the variant prop 1`] = `
   justify-content: center;
   background-color: #fff;
   border-radius: 50%;
-  box-shadow: 0 1px 5px rgba(27,31,35,0.15);
+  box-shadow: 0 3px 6px rgba(149,157,165,0.15);
   width: 20px;
   height: 20px;
 }