Improvements to Modal layout helpers (#1025)

Luke Bowerman · web-flow · commit bd3cc8ad904a · 2020-06-05T17:08:57.000-07:00
* Improvements to Modal layout helpers
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- `ModalContent`
+  - Tweaked the presentation of overflow state (simple border on top, more subtle shadow + border at bottom)
+  - now supports `borderBottom` that will always display a border at the bottom of the content area regardless of whether the content overflows the container.
+- `ModalFooter` now leverages `Space` components internally
+- `ModalHeader` refactored to invert Styled Components usage
 - `SpaceVertical` now has `align=stretch` by default
 
 ### Fixed
diff --git a/packages/components/src/Modal/Layout/ModalContent.tsx b/packages/components/src/Modal/Layout/ModalContent.tsx
@@ -46,6 +46,8 @@ export interface ModalContentProps
    * touch the container edges.
    */
   innerProps?: SpaceProps
+
+  borderBottom?: boolean
 }
 
 interface ContentState {
@@ -77,14 +79,21 @@ class Internal extends Component<InternalContentProps, ContentState> {
   }
 
   public render() {
-    const { children, className, innerProps, ...props } = this.props
+    const {
+      borderBottom,
+      children,
+      className,
+      innerProps,
+      ...props
+    } = this.props
 
     if (innerProps && innerProps.p && !innerProps.px) {
       innerProps.px = innerProps.p
     }
 
     return (
       <Outer
+        borderBottom={borderBottom}
         className={`${className} ${this.state.overflow && 'overflow'}`}
         ref={this.ref}
         {...omit(props, ['renderedHeight'])}
@@ -103,16 +112,19 @@ export const ModalContent = (props: ModalContentProps) => {
   )
 }
 
-const Outer = styled.div`
+const Outer = styled.div<{ borderBottom?: boolean }>`
   ${reset}
   ${layout}
 
   overflow: auto;
   flex: 8;
+  ${({ borderBottom, theme: { colors } }) =>
+    borderBottom && `border-bottom: 1px solid ${colors.ui2}`};
 
   &.overflow {
-    box-shadow: inset 0 -16px 16px -16px ${({ theme }) => theme.colors.ui2},
-      inset 0 16px 16px -16px ${({ theme }) => theme.colors.ui2};
+    border-bottom: 1px solid ${({ theme }) => theme.colors.ui2};
+    border-top: 1px solid ${({ theme }) => theme.colors.ui2};
+    box-shadow: inset 0 -4px 4px -4px ${({ theme }) => theme.colors.ui2};
   }
 `
 
diff --git a/packages/components/src/Modal/Layout/ModalFooter.tsx b/packages/components/src/Modal/Layout/ModalFooter.tsx
@@ -27,42 +27,33 @@
 import React, { FC, ReactNode } from 'react'
 import styled from 'styled-components'
 import { CompatibleHTMLProps } from '@looker/design-tokens'
-import { SpaceProps, space } from 'styled-system'
+import { Space, SpaceHelperProps } from '../../Layout/Space'
 
-export interface ModalFooterProps extends CompatibleHTMLProps<HTMLDivElement> {
+export interface ModalFooterProps
+  extends CompatibleHTMLProps<HTMLDivElement>,
+    SpaceHelperProps {
   /**
    * Secondary content to place in the footer
    */
   secondary?: ReactNode
 }
 
-export const ModalFooter: FC<ModalFooterProps> = ({
+export const ModalFooterLayout: FC<ModalFooterProps> = ({
   children,
   secondary,
   ...props
 }) => {
   return (
-    <Layout {...props}>
-      <SpaceChildren>{children}</SpaceChildren>
-      {secondary && <SpaceChildren mr="auto">{secondary}</SpaceChildren>}
-    </Layout>
+    <Space as="footer" reverse between {...props}>
+      <Space reverse>{children}</Space>
+      {secondary && <Space>{secondary}</Space>}
+    </Space>
   )
 }
 
-const Layout = styled.footer`
-  align-items: center;
-  display: flex;
-  flex-direction: row-reverse;
-  padding: ${({ theme }) => `${theme.space.large} ${theme.space.xlarge}`};
-`
+export const ModalFooter = styled(ModalFooterLayout)``
 
-const SpaceChildren = styled.div<SpaceProps>`
-  ${space};
-  display: grid;
-  grid-gap: ${({ theme }) => theme.space.medium};
-  grid-auto-flow: column;
-  direction: rtl; /* Grid equivalent to flex-direction: row-reverse */
-  * {
-    direction: ltr;
-  }
-`
+ModalFooter.defaultProps = {
+  px: 'xlarge',
+  py: 'large',
+}
diff --git a/packages/components/src/Modal/Layout/ModalHeader.tsx b/packages/components/src/Modal/Layout/ModalHeader.tsx
@@ -56,20 +56,22 @@ export interface ModalHeaderProps
    * Render an icon next to the title text
    */
   headerIcon?: ReactElement<IconProps>
+
+  className?: string
 }
 
-export const ModalHeader: FC<ModalHeaderProps> = ({
+const ModalHeaderLayout: FC<ModalHeaderProps> = ({
+  className,
   children,
   closeIcon = 'Close',
   hideClose,
   headerIcon,
   id,
-  ...props
 }) => {
   const { closeModal } = useContext(ModalContext)
 
   return (
-    <Header {...props}>
+    <header className={className}>
       {headerIcon && <HeaderIconWrapper>{headerIcon}</HeaderIconWrapper>}
       <Heading
         as="h3"
@@ -92,19 +94,19 @@ export const ModalHeader: FC<ModalHeaderProps> = ({
           style={{ gridArea: 'close' }}
         />
       )}
-    </Header>
+    </header>
   )
 }
 
-const Header = styled.header<SpaceProps>`
+export const ModalHeader = styled(ModalHeaderLayout)`
   ${reset}
   ${space}
   display: grid;
   grid-template-columns: [icon] auto [text] 1fr [close] auto;
   align-items: center;
 `
 
-Header.defaultProps = {
+ModalHeader.defaultProps = {
   p: 'large',
   pr: 'medium',
   px: 'xlarge',
diff --git a/packages/components/src/Modal/Layout/__snapshots__/ModalContent.test.tsx.snap b/packages/components/src/Modal/Layout/__snapshots__/ModalContent.test.tsx.snap
@@ -9,7 +9,9 @@ exports[`ModalContent - no overflow 1`] = `
 }
 
 .c0.overflow {
-  box-shadow: inset 0 -16px 16px -16px #DEE1E5, inset 0 16px 16px -16px #DEE1E5;
+  border-bottom: 1px solid #DEE1E5;
+  border-top: 1px solid #DEE1E5;
+  box-shadow: inset 0 -4px 4px -4px #DEE1E5;
 }
 
 .c1 {
@@ -39,7 +41,9 @@ exports[`ModalContent - no overflow 2`] = `
 }
 
 .c0.overflow {
-  box-shadow: inset 0 -16px 16px -16px #DEE1E5, inset 0 16px 16px -16px #DEE1E5;
+  border-bottom: 1px solid #DEE1E5;
+  border-top: 1px solid #DEE1E5;
+  box-shadow: inset 0 -4px 4px -4px #DEE1E5;
 }
 
 .c1 {
diff --git a/packages/components/src/Modal/Layout/__snapshots__/ModalFooter.test.tsx.snap b/packages/components/src/Modal/Layout/__snapshots__/ModalFooter.test.tsx.snap
@@ -2,6 +2,11 @@
 
 exports[`ModalFooter with Button 1`] = `
 .c0 {
+  width: 100%;
+  padding-left: 2rem;
+  padding-right: 2rem;
+  padding-top: 1.25rem;
+  padding-bottom: 1.25rem;
   -webkit-align-items: center;
   -webkit-box-align: center;
   -ms-flex-align: center;
@@ -13,25 +18,42 @@ exports[`ModalFooter with Button 1`] = `
   -webkit-flex-direction: row-reverse;
   -ms-flex-direction: row-reverse;
   flex-direction: row-reverse;
-  padding: 1.25rem 2rem;
+  -webkit-box-pack: justify;
+  -webkit-justify-content: space-between;
+  -ms-flex-pack: justify;
+  justify-content: space-between;
 }
 
 .c1 {
-  display: grid;
-  grid-gap: 1rem;
-  grid-auto-flow: column;
-  direction: rtl;
+  width: 100%;
+  -webkit-align-items: center;
+  -webkit-box-align: center;
+  -ms-flex-align: center;
+  align-items: center;
+  display: -webkit-box;
+  display: -webkit-flex;
+  display: -ms-flexbox;
+  display: flex;
+  -webkit-flex-direction: row-reverse;
+  -ms-flex-direction: row-reverse;
+  flex-direction: row-reverse;
+}
+
+.c1.c1 > * {
+  margin-left: 1rem;
 }
 
-.c1 * {
-  direction: ltr;
+.c1.c1 > *:last-child {
+  margin-left: 0rem;
 }
 
 <footer
-  className="c0"
+  className="c0 "
+  width="100%"
 >
   <div
     className="c1"
+    width="100%"
   >
     <button>
       Cancel
@@ -42,6 +64,11 @@ exports[`ModalFooter with Button 1`] = `
 
 exports[`ModalFooter with ModalContext 1`] = `
 .c0 {
+  width: 100%;
+  padding-left: 2rem;
+  padding-right: 2rem;
+  padding-top: 1.25rem;
+  padding-bottom: 1.25rem;
   -webkit-align-items: center;
   -webkit-box-align: center;
   -ms-flex-align: center;
@@ -53,25 +80,42 @@ exports[`ModalFooter with ModalContext 1`] = `
   -webkit-flex-direction: row-reverse;
   -ms-flex-direction: row-reverse;
   flex-direction: row-reverse;
-  padding: 1.25rem 2rem;
+  -webkit-box-pack: justify;
+  -webkit-justify-content: space-between;
+  -ms-flex-pack: justify;
+  justify-content: space-between;
 }
 
 .c1 {
-  display: grid;
-  grid-gap: 1rem;
-  grid-auto-flow: column;
-  direction: rtl;
+  width: 100%;
+  -webkit-align-items: center;
+  -webkit-box-align: center;
+  -ms-flex-align: center;
+  align-items: center;
+  display: -webkit-box;
+  display: -webkit-flex;
+  display: -ms-flexbox;
+  display: flex;
+  -webkit-flex-direction: row-reverse;
+  -ms-flex-direction: row-reverse;
+  flex-direction: row-reverse;
+}
+
+.c1.c1 > * {
+  margin-left: 1rem;
 }
 
-.c1 * {
-  direction: ltr;
+.c1.c1 > *:last-child {
+  margin-left: 0rem;
 }
 
 <footer
-  className="c0"
+  className="c0 "
+  width="100%"
 >
   <div
     className="c1"
+    width="100%"
   >
     <button>
       Cancel
diff --git a/packages/www/src/documentation/components/modals/dialog.mdx b/packages/www/src/documentation/components/modals/dialog.mdx
@@ -24,10 +24,25 @@ Dialogs break out of the standard application flow and UI to present new informa
   return (
     <div>
       <Dialog isOpen={value} onClose={() => setOff()}>
+        <ModalHeader>A Dialog Example</ModalHeader>
         <ModalContent>
-          <Paragraph>Some content inside the Dialog</Paragraph>
-          <Button>A button too!</Button>
+          <SpaceVertical>
+            <Paragraph>
+              {`Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin pellentesque dui sodales efficitur vehicula. Praesent at aliquet nisi. Fusce risus quam, scelerisque non lacus mollis, feugiat molestie sem. Praesent augue erat, molestie id sem quis, fermentum venenatis lacus. Morbi vehicula lacus id fringilla scelerisque. Nulla dignissim nisi nisl, ac porttitor orci accumsan nec. Vestibulum pulvinar risus felis, vel bibendum odio luctus a. Morbi pharetra, ipsum sit amet vehicula malesuada, lacus quam viverra sapien, eget malesuada sem risus quis ligula. Vestibulum non laoreet est. Nullam blandit sollicitudin cursus. Nam cursus nisl id ipsum ullamcorper consequat.`}
+            </Paragraph>
+            <Paragraph>
+              {`Aenean consectetur volutpat dolor id varius. Interdum et malesuada fames ac ante ipsum primis in faucibus. Nam mollis, sapien sit amet suscipit consectetur, odio metus malesuada arcu, et eleifend nisi lorem eu diam. Praesent malesuada finibus finibus. Suspendisse eu lobortis ante, sed gravida odio. Nam auctor eros nulla. Praesent maximus ligula quam, et pretium lacus vulputate non. Donec condimentum turpis vitae mi volutpat malesuada. Nam sagittis non metus in efficitur. Vestibulum commodo nibh velit, sed luctus felis condimentum non.`}
+              `
+            </Paragraph>
+            <Paragraph>
+              {`Aenean consectetur volutpat dolor id varius. Interdum et malesuada fames ac ante ipsum primis in faucibus. Nam mollis, sapien sit amet suscipit consectetur, odio metus malesuada arcu, et eleifend nisi lorem eu diam. Praesent malesuada finibus finibus. Suspendisse eu lobortis ante, sed gravida odio. Nam auctor eros nulla. Praesent maximus ligula quam, et pretium lacus vulputate non. Donec condimentum turpis vitae mi volutpat malesuada. Nam sagittis non metus in efficitur. Vestibulum commodo nibh velit, sed luctus felis condimentum non.`}
+            </Paragraph>
+          </SpaceVertical>
         </ModalContent>
+        <ModalFooter>
+          <Button>Close Example Dialog</Button>
+          <ButtonTransparent color="neutral">Cancel</ButtonTransparent>
+        </ModalFooter>
       </Dialog>
       <Button onClick={() => setOn()}>Open Dialog</Button>
     </div>

Original file line number	Diff line number	Diff line change
@@ -9,7 +9,9 @@ exports[`ModalContent - no overflow 1`] = `
`9`	`9`	`}`
`10`	`10`
`11`	`11`	`.c0.overflow {`
`12`		`- box-shadow: inset 0 -16px 16px -16px #DEE1E5, inset 0 16px 16px -16px #DEE1E5;`
	`12`	`+ border-bottom: 1px solid #DEE1E5;`
	`13`	`+ border-top: 1px solid #DEE1E5;`
	`14`	`+ box-shadow: inset 0 -4px 4px -4px #DEE1E5;`
`13`	`15`	`}`
`14`	`16`
`15`	`17`	`.c1 {`
@@ -39,7 +41,9 @@ exports[`ModalContent - no overflow 2`] = `
`39`	`41`	`}`
`40`	`42`
`41`	`43`	`.c0.overflow {`
`42`		`- box-shadow: inset 0 -16px 16px -16px #DEE1E5, inset 0 16px 16px -16px #DEE1E5;`
	`44`	`+ border-bottom: 1px solid #DEE1E5;`
	`45`	`+ border-top: 1px solid #DEE1E5;`
	`46`	`+ box-shadow: inset 0 -4px 4px -4px #DEE1E5;`
`43`	`47`	`}`
`44`	`48`
`45`	`49`	`.c1 {`