fix(Dropdown): Remove timeouts, Fix ref bug (#42)

heydoctor · May 15, 2019 · 85590cf · 85590cf
1 parent 489e50b
commit 85590cf
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 16 deletions.
diff --git a/src/Dropdown/Dropdown.js b/src/Dropdown/Dropdown.js
@@ -52,6 +52,8 @@ export default function Dropdown({
 }) {
   const popperRef = useRef();
   const menuRef = useRef();
+  const triggerRef = useRef();
+
   const [isOpen, setOpen] = useState(false);
 
   const open = () => setOpen(true);
@@ -62,6 +64,7 @@ export default function Dropdown({
     // Allow all clicks and, for non-button elements, Enter and Space to toggle Dropdown
     // https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role#Required_JavaScript_Features
     if (e.type === 'click' || (e.type === 'keypress' && (e.which === 13 || e.which === 32))) {
+      e.preventDefault();
       e.stopPropagation();
       toggle();
     }
@@ -71,13 +74,14 @@ export default function Dropdown({
   useEffect(() => {
     if (isOpen) {
       setTimeout(() => {
-        menuRef.current.focus();
+        if (menuRef.current) menuRef.current.focus();
       });
     }
   }, [isOpen]);
 
   useKeyPress('Escape', () => {
     if (isOpen) {
+      if (triggerRef.current) triggerRef.current.focus();
       close();
     }
   });
@@ -95,16 +99,9 @@ export default function Dropdown({
     }
   });
 
-  const handleMenuBlur = () => {
-    if (autoclose) {
-      // Use timeout to delay examination of activeElement until after blur/focus
-      // events have been processed.
-      setTimeout(() => {
-        const nextActiveElement = document.activeElement;
-        if (menuRef.current && !menuRef.current.contains(nextActiveElement)) {
-          close();
-        }
-      });
+  const handleMenuBlur = e => {
+    if (autoclose && menuRef.current && !menuRef.current.contains(e.relatedTarget)) {
+      close();
     }
   };
 
@@ -114,15 +111,16 @@ export default function Dropdown({
     <DropdownContext.Provider value={{ close }}>
       <Manager>
         <Reference>
-          {({ ref: triggerRef }) => (
-            <Trigger style={styles.Trigger} ref={triggerRef} tabIndex={-1}>
+          {({ ref: passedTriggerRef }) => (
+            <Trigger style={styles.Trigger} ref={passedTriggerRef} tabIndex={-1}>
               {React.cloneElement(trigger, {
                 role: trigger.role || 'button',
                 tabIndex: trigger.tabIndex || 0,
                 'aria-haspopup': true,
                 'aria-expanded': isOpen,
                 onClick: handleTrigger,
                 onKeyPress: handleTrigger,
+                ref: triggerRef,
                 style: {
                   cursor: 'pointer',
                   ...(trigger.style || {}),
@@ -155,7 +153,7 @@ export default function Dropdown({
               {({ ref, style }) => (
                 <Transition in={isOpen} timeout={0} appear>
                   {state => (
-                    <FocusTrap returnFocus autoFocus={false}>
+                    <FocusTrap autoFocus={false}>
                       <DropdownMenu
                         ref={menuInner => {
                           menuRef.current = menuInner;

diff --git a/src/Dropdown/Dropdown.spec.js b/src/Dropdown/Dropdown.spec.js
@@ -67,6 +67,7 @@ describe('<Dropdown />', () => {
     expect(renderUtils.asFragment()).toMatchSnapshot();
   });
 
+  // TODO: add cases for space and enter keypress events
   test('opens menu with focus when trigger is clicked', async () => {
     await openDropdown();
     expect(renderUtils.asFragment()).toMatchSnapshot();
@@ -88,9 +89,7 @@ describe('<Dropdown />', () => {
 
     // Some issues with fireEvent.focus: https://github.com/kentcdodds/react-testing-library/issues/276#issuecomment-473392827
     renderUtils.wrapper.focus();
-    renderUtils.getByTestId('dropdown-menu').blur();
 
-    await waitForDomChange();
     await assertDropdownClosed();
 
     console.error = ogError;