Repository: zeit-ui/react
Pull request #283 (2020-06-22 09:40:47)

fix(modal): fix height constraint of modal
## PR Checklist

- [x] Fix linting errors
- [x] Label has been added


## Change information


fixed zeit-ui/vue#194


Patch:
diff --git a/components/modal/modal-actions.tsx b/components/modal/modal-actions.tsx
index 875e778..0c2d088 100644
--- a/components/modal/modal-actions.tsx
+++ b/components/modal/modal-actions.tsx
@@ -28,6 +28,7 @@ const ModalActions: React.FC<React.PropsWithChildren<{}>> = ({ children, ...prop
 
         div {
           height: 3.625rem;
+          flex-shrink: 0;
         }
       `}</style>
     </>
diff --git a/components/modal/modal-content.tsx b/components/modal/modal-content.tsx
index af443b5..ac7c45b 100644
--- a/components/modal/modal-content.tsx
+++ b/components/modal/modal-content.tsx
@@ -23,8 +23,9 @@ const ModalContent: React.FC<ModalContentProps> = ({ className, children, ...pro
       </div>
       <style jsx>{`
         .content {
-          margin: 0;
-          padding: ${theme.layout.gap} 0 ${theme.layout.gapHalf} 0;
+          margin: 0 -${theme.layout.gap};
+          padding: ${theme.layout.gap} ${theme.layout.gap} ${theme.layout.gapHalf};
+          overflow-y: auto;
         }
 
         .content > :global(*:first-child) {
diff --git a/components/modal/modal-title.tsx b/components/modal/modal-title.tsx
index 87831ac..a3e11e5 100644
--- a/components/modal/modal-title.tsx
+++ b/components/modal/modal-title.tsx
@@ -29,6 +29,7 @@ const ModalTitle: React.FC<ModalTitleProps> = ({ className, children, ...props }
           text-align: center;
           margin: 0;
           display: inline-flex;
+          flex-shrink: 0;
           justify-content: center;
           align-items: center;
           word-break: break-word;
diff --git a/components/modal/modal-wrapper.tsx b/components/modal/modal-wrapper.tsx
index fa425f8..568f81e 100644
--- a/components/modal/modal-wrapper.tsx
+++ b/components/modal/modal-wrapper.tsx
@@ -32,6 +32,7 @@ const ModalWrapper: React.FC<React.PropsWithChildren<ModalWrapperProps>> = ({
         <style jsx>{`
           .wrapper {
             max-width: 85vw;
+            max-height: 75vh;
             width: ${width};
             overflow: hidden;
             display: flex;
diff --git a/components/modal/modal.tsx b/components/modal/modal.tsx
index d182ed2..f0af962 100644
--- a/components/modal/modal.tsx
+++ b/components/modal/modal.tsx
@@ -80,7 +80,7 @@ const Modal: React.FC<React.PropsWithChildren<ModalProps>> = ({
   if (!portal) return null
   return createPortal(
     <ModalContext.Provider value={modalConfig}>
-      <Backdrop onClick={closeFromBackdrop} visible={visible} offsetY={25}>
+      <Backdrop onClick={closeFromBackdrop} visible={visible}>
         <ModalWrapper visible={visible} className={wrapClassName} width={wrapperWidth}>
           {withoutActionsChildren}
           {hasActions && <ModalActions>{ActionsChildren}</ModalActions>}
diff --git a/components/shared/backdrop.tsx b/components/shared/backdrop.tsx
index a5ce5e8..eb2fc80 100644
--- a/components/shared/backdrop.tsx
+++ b/components/shared/backdrop.tsx
@@ -7,7 +7,6 @@ import useCurrentState from '../utils/use-current-state'
 interface Props {
   onClick?: (event: MouseEvent<HTMLElement>) => void
   visible?: boolean
-  offsetY?: number
 }
 
 const defaultProps = {
@@ -19,7 +18,7 @@ const defaultProps = {
 export type BackdropProps = Props & typeof defaultProps
 
 const Backdrop: React.FC<React.PropsWithChildren<BackdropProps>> = React.memo(
-  ({ children, onClick, visible, offsetY }) => {
+  ({ children, onClick, visible }) => {
     const theme = useTheme()
     const [, setIsContentMouseDown, IsContentMouseDownRef] = useCurrentState(false)
     const clickHandler = (event: MouseEvent<HTMLElement>) => {
@@ -57,7 +56,7 @@ const Backdrop: React.FC<React.PropsWithChildren<BackdropProps>> = React.memo(
               align-content: center;
               align-items: center;
               flex-direction: column;
-              justify-content: center;
+              justify-content: space-around;
               height: 100vh;
               width: 100vw;
               overflow: auto;
@@ -74,8 +73,9 @@ const Backdrop: React.FC<React.PropsWithChildren<BackdropProps>> = React.memo(
             }
 
             .offset {
-              height: ${offsetY}vh;
+              height: 0;
               opacity: 0;
+              display: flex;
               background-color: transparent;
             }
 


Test patch:
diff --git a/components/modal/__tests__/__snapshots__/index.test.tsx.snap b/components/modal/__tests__/__snapshots__/index.test.tsx.snap
index 5c12241..df1c29c 100644
--- a/components/modal/__tests__/__snapshots__/index.test.tsx.snap
+++ b/components/modal/__tests__/__snapshots__/index.test.tsx.snap
@@ -9,6 +9,7 @@ exports[`Modal customization should be supported 1`] = `
           text-align: center;
           margin: 0;
           display: inline-flex;
+          flex-shrink: 0;
           justify-content: center;
           align-items: center;
           word-break: break-word;
@@ -18,6 +19,7 @@ exports[`Modal customization should be supported 1`] = `
       </style><style>
           .wrapper {
             max-width: 85vw;
+            max-height: 75vh;
             width: 100px;
             overflow: hidden;
             display: flex;
@@ -66,6 +68,7 @@ exports[`Modal should render correctly 1`] = `
           text-align: center;
           margin: 0;
           display: inline-flex;
+          flex-shrink: 0;
           justify-content: center;
           align-items: center;
           word-break: break-word;
@@ -88,8 +91,9 @@ exports[`Modal should render correctly 1`] = `
         }
       </style><div class=\\"content \\"><p>Some content contained within the modal.</p></div><style>
         .content {
-          margin: 0;
-          padding: 16pt 0 8pt 0;
+          margin: 0 -16pt;
+          padding: 16pt 16pt 8pt;
+          overflow-y: auto;
         }
 
         .content > :global(*:first-child) {
@@ -166,10 +170,12 @@ exports[`Modal should render correctly 1`] = `
 
         div {
           height: 3.625rem;
+          flex-shrink: 0;
         }
       </style><style>
           .wrapper {
             max-width: 85vw;
+            max-height: 75vh;
             width: 26rem;
             overflow: hidden;
             display: flex;
@@ -215,7 +221,7 @@ exports[`Modal should render correctly 1`] = `
               align-content: center;
               align-items: center;
               flex-direction: column;
-              justify-content: center;
+              justify-content: space-around;
               height: 100vh;
               width: 100vw;
               overflow: auto;
@@ -232,8 +238,9 @@ exports[`Modal should render correctly 1`] = `
             }
 
             .offset {
-              height: 25vh;
+              height: 0;
               opacity: 0;
+              display: flex;
               background-color: transparent;
             }
 
diff --git a/components/shared/__tests__/__snapshots__/backdrop.test.tsx.snap b/components/shared/__tests__/__snapshots__/backdrop.test.tsx.snap
index 8082819..e2c6c04 100644
--- a/components/shared/__tests__/__snapshots__/backdrop.test.tsx.snap
+++ b/components/shared/__tests__/__snapshots__/backdrop.test.tsx.snap
@@ -1,103 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Backdrop content should be offset 1`] = `
-"<div class=\\"backdrop  transition-enter\\"><div class=\\"layer\\"></div><div class=\\"content\\"><span>test-value</span></div><div class=\\"offset\\"></div><style>
-            .backdrop {
-              position: fixed;
-              top: 0;
-              left: 0;
-              display: flex;
-              align-content: center;
-              align-items: center;
-              flex-direction: column;
-              justify-content: center;
-              height: 100vh;
-              width: 100vw;
-              overflow: auto;
-              z-index: 1000;
-            }
-
-            .content {
-              display: flex;
-              align-items: center;
-              justify-content: center;
-              position: relative;
-              z-index: 1001;
-              outline: none;
-            }
-
-            .offset {
-              height: 0vh;
-              opacity: 0;
-              background-color: transparent;
-            }
-
-            .layer {
-              position: fixed;
-              top: 0;
-              left: 0;
-              right: 0;
-              bottom: 0;
-              width: 100%;
-              height: 100%;
-              opacity: 0.25;
-              background-color: black;
-              transition: opacity 0.35s cubic-bezier(0.4, 0, 0.2, 1);
-              pointer-events: none;
-              z-index: 1000;
-            }
-          </style></div>"
-`;
-
-exports[`Backdrop content should be offset 2`] = `
-"<div class=\\"backdrop  transition-enter\\"><div class=\\"layer\\"></div><div class=\\"content\\"><span>test-value</span></div><div class=\\"offset\\"></div><style>
-            .backdrop {
-              position: fixed;
-              top: 0;
-              left: 0;
-              display: flex;
-              align-content: center;
-              align-items: center;
-              flex-direction: column;
-              justify-content: center;
-              height: 100vh;
-              width: 100vw;
-              overflow: auto;
-              z-index: 1000;
-            }
-
-            .content {
-              display: flex;
-              align-items: center;
-              justify-content: center;
-              position: relative;
-              z-index: 1001;
-              outline: none;
-            }
-
-            .offset {
-              height: 100vh;
-              opacity: 0;
-              background-color: transparent;
-            }
-
-            .layer {
-              position: fixed;
-              top: 0;
-              left: 0;
-              right: 0;
-              bottom: 0;
-              width: 100%;
-              height: 100%;
-              opacity: 0.25;
-              background-color: black;
-              transition: opacity 0.35s cubic-bezier(0.4, 0, 0.2, 1);
-              pointer-events: none;
-              z-index: 1000;
-            }
-          </style></div>"
-`;
-
 exports[`Backdrop should render correctly 1`] = `
 "<div class=\\"backdrop  transition-enter\\"><div class=\\"layer\\"></div><div class=\\"content\\"><span>test-value</span></div><div class=\\"offset\\"></div><style>
             .backdrop {
@@ -108,7 +10,7 @@ exports[`Backdrop should render correctly 1`] = `
               align-content: center;
               align-items: center;
               flex-direction: column;
-              justify-content: center;
+              justify-content: space-around;
               height: 100vh;
               width: 100vw;
               overflow: auto;
@@ -125,8 +27,9 @@ exports[`Backdrop should render correctly 1`] = `
             }
 
             .offset {
-              height: 0vh;
+              height: 0;
               opacity: 0;
+              display: flex;
               background-color: transparent;
             }
 
diff --git a/components/shared/__tests__/backdrop.test.tsx b/components/shared/__tests__/backdrop.test.tsx
index d3de821..13462a6 100644
--- a/components/shared/__tests__/backdrop.test.tsx
+++ b/components/shared/__tests__/backdrop.test.tsx
@@ -61,20 +61,6 @@ describe('Backdrop', () => {
     handler.mockRestore()
   })
 
-  it('content should be offset', () => {
-    const wrapper = mount(
-      <Backdrop visible>
-        <span>test-value</span>
-      </Backdrop>,
-    )
-    const notOffset = wrapper.html()
-    expect(wrapper.html()).toMatchSnapshot()
-
-    wrapper.setProps({ offsetY: '100' })
-    expect(wrapper.html()).toMatchSnapshot()
-    expect(notOffset).not.toEqual(wrapper.html())
-  })
-
   it('backdrop handler should ignore click events from content', async () => {
     const handler = jest.fn()
     const wrapper = mount(