diff --git a/docs/content/Dialog.md b/docs/content/Dialog.md index 23d083efb04..7884eb65d36 100644 --- a/docs/content/Dialog.md +++ b/docs/content/Dialog.md @@ -8,12 +8,13 @@ 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 {([isOpen, setIsOpen]) => ( <> - setIsOpen(false)}> + setIsOpen(false)}> + Title Some content @@ -21,13 +22,19 @@ The dialog component is used for all modals. It renders on top of the rest of th )} +``` + +You can also pass any non-text content into the header: -```jsx +```jsx live {([isOpen, setIsOpen]) => ( <> - setIsOpen(false)}> + setIsOpen(false)}> + + + Some content @@ -37,23 +44,15 @@ The dialog component is used for all modals. It renders on top of the rest of th ``` - -You can also pass any React node as the title to override the styling: - -```jsx -Title}> -``` - ## 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. diff --git a/docs/content/Heading.md b/docs/content/Heading.md index 824a33be9f4..3e71f224abf 100644 --- a/docs/content/Heading.md +++ b/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` (`H1 Element`). ## Default example ```jsx live -With fontSize={1} +H2 heading with fontSize={1} ``` ## 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 | diff --git a/index.d.ts b/index.d.ts index f4538f169d4..61fbd30d03d 100644 --- a/index.d.ts +++ b/index.d.ts @@ -72,7 +72,9 @@ declare module '@primer/components' { extends BaseProps, CommonProps, TypographyProps, - Omit, 'color'> {} + Omit, 'color'> { + as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' + } export const Heading: React.FunctionComponent @@ -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 + export interface DialogHeaderProps extends FlexProps {} + + export const Dialog: React.FunctionComponent & { + Header: React.FunctionComponent + } export interface LabelGroupProps extends CommonProps, Omit, 'color'> {} diff --git a/src/Dialog.js b/src/Dialog.js index ca67571f1d6..b00d4c46ad1 100644 --- a/src/Dialog.js +++ b/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 = ( + + {children} + + ) + } + + return ( + + {children} + + ) +} + +const Dialog = ({children, ...props}) => { return ( <> - - {typeof title === 'string' ? ( - - {title} - - ) : ( - title - )} - - - - + + + {children} @@ -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 diff --git a/src/Heading.js b/src/Heading.js index f75ff14c3bc..424d3327572 100644 --- a/src/Heading.js +++ b/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 diff --git a/src/__tests__/BorderBox.js b/src/__tests__/BorderBox.js index 9304f6ab166..67a2c3becc8 100644 --- a/src/__tests__/BorderBox.js +++ b/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()).toHaveStyleRule('box-shadow', '0 1px 1px rgba(27,31,35,0.1)') + expect(render()).toHaveStyleRule('box-shadow', '0 1px 0 rgba(149,157,165,0.1)') }) }) diff --git a/src/__tests__/Dialog.js b/src/__tests__/Dialog.js index 99a32e4d065..38173a350d7 100644 --- a/src/__tests__/Dialog.js +++ b/src/__tests__/Dialog.js @@ -8,7 +8,8 @@ import 'babel-polyfill' expect.extend(toHaveNoViolations) const comp = ( - null}> + null}> + Title Some content @@ -22,4 +23,8 @@ describe('Dialog', () => { expect(results).toHaveNoViolations() cleanup() }) + + it('renders consistently', () => { + expect(comp).toMatchSnapshot() + }) }) diff --git a/src/__tests__/Heading.js b/src/__tests__/Heading.js index d447cc2c587..2540da192c0 100644 --- a/src/__tests__/Heading.js +++ b/src/__tests__/Heading.js @@ -31,8 +31,8 @@ const theme = { } describe('Heading', () => { - it('renders

by default', () => { - expect(render().type).toEqual('h1') + it('renders

by default', () => { + expect(render().type).toEqual('h2') }) it('should have no axe violations', async () => { @@ -94,7 +94,6 @@ describe('Heading', () => { it('respects the "fontStyle" prop', () => { expect(render()).toHaveStyleRule('font-style', 'italic') - expect(render()).toHaveStyleRule('font-style', 'normal') }) it.skip('renders fontSize with f* classes using inverse scale', () => { diff --git a/src/__tests__/__snapshots__/CircleBadge.js.snap b/src/__tests__/__snapshots__/CircleBadge.js.snap index e35e2e949b2..ccce0cf64b7 100644 --- a/src/__tests__/__snapshots__/CircleBadge.js.snap +++ b/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; } diff --git a/src/__tests__/__snapshots__/Dialog.js.snap b/src/__tests__/__snapshots__/Dialog.js.snap new file mode 100644 index 00000000000..8566ce03799 --- /dev/null +++ b/src/__tests__/__snapshots__/Dialog.js.snap @@ -0,0 +1,1255 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Dialog renders consistently 1`] = ` + + + Title + + + + Some content + + + +`; diff --git a/src/theme.js b/src/theme.js index cccbade424e..e0542dfa958 100644 --- a/src/theme.js +++ b/src/theme.js @@ -82,10 +82,10 @@ const borders = [0, '1px solid'] const radii = ['0', '3px', '6px', '100px'] const shadows = { - small: '0 1px 1px rgba(27, 31, 35, 0.1)', - medium: '0 1px 5px rgba(27, 31, 35, 0.15)', - large: '0 1px 15px rgba(27, 31, 35, 0.15)', - 'extra-large': '0 10px 50px rgba(27, 31, 35, 0.07)', + small: '0 1px 0 rgba(149, 157, 165, 0.1)', + medium: '0 3px 6px rgba(149, 157, 165, 0.15)', + large: '0 8px 24px rgba(149, 157, 165, 0.2)', + 'extra-large': '0 12px 48px rgba(149, 157, 165, 0.3)', formControl: 'inset 0px 2px 0px rgba(225, 228, 232, 0.2)', formControlDisabled: 'inset 0px 2px 0px rgba(220, 227, 237, 0.3)', formControlFocus: 'rgba(3, 102, 214, 0.3) 0px 0px 0px 0.2em',