search

getsentry / sentry

Developer-first error tracking and performance monitoring
https://sentry.io
Other
38.98k stars 4.18k forks source link

The <CompactSelect> component doesn't open a flyout when rendering in the Chrome Mobile Emulator #34320

Open ryan953 opened 2 years ago

ryan953 commented 2 years ago

Repro:

  1. Visit: https://storybook.sentry.dev/?path=/story/components-forms-fields--compact-select-field
    • Notice you can open the select field and see some options
  2. In devtools enable mobile emulation
    • Screen Shot 2022-05-06 at 10 17 14 PM
    • Notice the flyout no longer appears when you click the field.
Danondso commented 2 years ago

This is the warning emitting when it happens. Added non-passive event listener to a scroll-blocking 'wheel' event.

image
Danondso commented 2 years ago

For posterity, that warning had nothing to do with the problem, for whatever reason the CompactSelect overlay would get blurred immediately after opening.

Danondso commented 2 years ago

Details are listed in this PR. tl;dr the dirty fix is to set shouldCloseOnBlur prop to false which allows mobile screens to open dropdown correctly. Obv that doesn't work since folks can set the prop. The prop is passed to a useOverlay hook that comes from react-aria.

Below are my findings from looking more into that library.

I did some looking around on react and found https://github.com/adobe/react-spectrum/issues/816 that's essentially the same behavior, a blur event fires right after opening. There's claims of a fix but there's no direct version number pointing to it but there's a fix here: https://github.com/adobe/react-spectrum/pull/2175. I'm curious if a dependency upgrade will fix it and I can go down that path if we're okay with bumping the react-aria version up to 3.15.

For the sake of time I'm going to get a codesandbox link up that duplicates the problem and create an issue. In the meantime I'll adjust the PR to ignore the prop on mobile. My plan is to check isMobile() since leveraging a media query check gave me really mixed results, the only problem with that is we'll get inconsistent behavior between the mobile view in the browser vs a native view since the userAgent doesn't change.

Here's the codesandbox: https://codesandbox.io/s/react-codesandboxer-example-forked-w1cfv2?file=/example.js

Danondso commented 2 years ago

Here's a screen recording of the DOM, if you play it slow you can see the menu elements open for a second then get removed almost immediately. https://user-images.githubusercontent.com/7014871/168678911-622dae8e-0c73-42fb-8903-f9bab5e4602d.mov

vuluongj20 commented 2 years ago

Hello! I just posted a fix for this problem in https://github.com/getsentry/sentry/pull/34820, you can confirm if the fix works here.