fix(getEnvironmentProps): remove obsolete check causing tap not to close

[sarahdayan]

This PR fixes an issue that made mobile users have to tap twice outside the autocomplete to close the panel.

The issue was caused by an obsolete check within the isTargetWithinAutocomplete flag. We were checking whether the current active element was contained within the form or the panel, as a way to ensure we weren't closing the panel when interacting with the autocomplete. However, this was problematic:

• We're already verifying this with the first check by assessing whether the event.target is in the form or the panel.
• On touchstart, the current active element is the previous target (when it's a focusable element), so at this stage it was still the search input. This is why the panel was closing on second tap.

touchstart.mov

Tests

I've updated the related test to better reflect user scenarios and catch the error. We were relying on the panel being initially opened (using initialState). In the test, document.activeElement was always body, and we never were in the scenario of having first opened the panel from a tap.

Now, we first focus the search input, verify that the panel is open, then tap outside, and verify that the panel is closed. Without the fix, the second assertion doesn't pass.

I've also updated the other tests asserting what the active element was, because this causes false negatives (see explanation on touchstart behavior explained above).

Next steps

I've moved the @TODO higher because the problem that occurs with multiple Autocomplete instances happens higher, at the state level (the first check is wrong). I'll open a separate issue for it.

We'll also need to update the test suite for onTouchMove (I added a @TODO).

fixes #710

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b7273f5:

Sandbox	Source
zealous-mayer-k4kv0	Issue #710

[francoischalifour]

The change seems to be working well on my mobile.

[francoischalifour]

I wonder if the check below is even required:

event.target === inputElement

It could be the source of the multi-instance bug.

[sarahdayan]

I'm not sure. From my tests it came from the state being wrong, but indeed this check could be useless. We'll check that.