fix(Dropdown): Remove timeouts, Fix ref bug

[cehsu]

These changes remove the timeout in handleMenuBlur, which was causing issues when attempting to focus on inputs while the dropdown was open, and would cause the menu not to close on blur.

Also this fixes a bug, when attempting to open the dropdown via the enter key (when menuRef.current was null).

[cehsu]

Fixing some tests for escape/close and also adding one for enter, and then will open back up.

[cehsu]

This setTimeout was causing a bug with blur when inputs were clicked. Managing whether blur is being triggered via the state now allows inputs clicks to trigger blur. Blur is now handled via relatedTarget.

[codecov]

Codecov Report

Merging #42 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master      #42   +/-   ##
=======================================
  Coverage   68.36%   68.36%           
=======================================
  Files          22       22           
  Lines         433      433           
  Branches       91       92    +1     
=======================================
  Hits          296      296           
  Misses        107      107           
  Partials       30       30

Impacted Files	Coverage Δ
src/Dropdown/Dropdown.js	92.85% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e7b7d3...4ca9453. Read the comment docs.

[kylealwyn]

@cehsu Could you elaborate on why enzyme was added? It'd be preferable to not have two testing frameworks in play simultaneously

[cehsu]

@kylealwyn the initial test framework is based on direct DOM manipulation, but as noted here, I wasn't able to trigger the toggle that way in the console. Therefore enzyme which is more closely tied to the component lifecycle, was tested out, and it does the job.

[kylealwyn]

@cehsu We were using blur successfully in the tests before, so I'm not entirely convinced that's the case:

// Some issues with fireEvent.focus: https://github.com/kentcdodds/react-testing-library/issues/276#issuecomment-473392827
renderUtils.wrapper.focus();
renderUtils.getByTestId('dropdown-menu').blur();

[cehsu]

@kylealwyn thanks again, I think removing the setTimeout may have affected the ability to use blur directly. I can push up a version using the other library for reference.

[kylealwyn]

@cehsu I'd prefer we stick with react-testing-library so if you point me to the exact issue you were encountering, I'd like to take a look

[cehsu]

Thanks @kylealwyn. The original (now failing) test has been restored for reference.

[cehsu]

I'm restored the enzyme tests for now since they work, if that's alright. We can revisit the libraries used again.

[kylealwyn]

@cehsu I'd prefer that we skip the failing test for now with manual inspection rather than adding an entirely new test framework

[kylealwyn]

@cehsu Could we tackle the tests or are they still cryptically failing?

[cehsu]

We manually implement returnFocus here, so that inputs can be directly focused (without a second click), which was prevented by using FocusLock's returnFocus.

[kylealwyn]

Works and looks great. Thanks for seeing this through @cehsu. If tests are giving too much trouble, we can loop back.

[cehsu]

@kylealwyn, good idea, the blur test re-enabled, I wasn't able to get the keypress trigger event tests to work however.

I tried something along these lines:

+  const stopPropagation = jest.mock();
+    
+  const triggerEvents = {
+    'click': { type: 'click', stopPropagation },
+    'enter': { type: 'keyPress', which: 13, stopPropagation },
+    'space': { type: 'keyPress', which: 32, stopPropagation },
+  };
+
 ...
-  const openDropdown = async () => {
+  const openDropdown = async ( event = triggerEvents.click ) => {
     const trigger = renderUtils.getByText('Trigger');
-    fireEvent.click(trigger, { stopPropagation: () => null });
+    fireEvent[event.type](trigger, { ...event, type: event.type.toLowerCase() });
     return assertDropdownOpen();
   };