Merge pull request #555 from primer/details-polyfill

Details polyfill + useEffect linting

Author: Emily

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@primer/components",
-  "version": "14.0.0",
+  "version": "14.1.0",
   "description": "Primer react components",
   "main": "dist/index.umd.js",
   "module": "dist/index.esm.js",
@@ -41,6 +41,7 @@
     "@types/styled-system": "5.1.1",
     "babel-plugin-macros": "2.4.2",
     "classnames": "^2.2.5",
+    "details-element-polyfill": "2.4.0",
     "nanoid": "2.0.0",
     "primer-colors": "1.0.1",
     "primer-markdown": "3.7.9",

package.json

@@ -1,9 +1,16 @@
-import React, {useState, useEffect, useRef} from 'react'
+import React, {useState, useEffect, useCallback, useRef} from 'react'
 import PropTypes from 'prop-types'
 import styled from 'styled-components'
 import {COMMON} from './constants'
 import theme from './theme'
 
+// The <details> element is not yet supported in Edge so we have to use a polyfill.
+// We have to check if window is defined before importing the polyfill
+// so the code doesn’t run while pages build
+if (typeof window !== 'undefined') {
+  import('details-element-polyfill')
+}
+
 const DetailsReset = styled('details')`
   & > summary {
     list-style: none;
@@ -20,27 +27,30 @@ function DetailsBase({children, overlay, render = getRenderer(children), default
   const [open, setOpen] = useState(defaultOpen)
   const ref = useRef(null)
 
+  const closeMenu = useCallback(
+    event => {
+      // only close the menu if we're clicking outside
+      if (event && event.target.closest('details') !== ref.current) {
+        setOpen(false)
+        document.removeEventListener('click', closeMenu)
+      }
+    },
+    [ref]
+  )
+
   useEffect(() => {
     if (overlay && open) {
       document.addEventListener('click', closeMenu)
       return () => {
         document.removeEventListener('click', closeMenu)
       }
     }
-  }, [open, overlay])
+  }, [open, overlay, closeMenu])
 
   function toggle(event) {
     setOpen(event.target.open)
   }
 
-  function closeMenu(event) {
-    // only close the menu if we're clicking outside
-    if (event && event.target.closest('details') !== ref.current) {
-      setOpen(false)
-      document.removeEventListener('click', closeMenu)
-    }
-  }
-
   return (
     <DetailsReset {...rest} ref={ref} open={open} onToggle={toggle} overlay={overlay}>
       {render({open})}