Something went wrong while fetching comments. Please try again.

JetBrains: Apply popover hiding strategy for every OS

Merged Administrator requested to merge ps/hide-popover-on-every-os into main 3 years ago

Created by: philipp-spiess

This PR removes the conditional popover logic that we added to fix a macOS-only issue (#34773 (closed)) and applies it to every OS instead.

There are multiple motivation for this:

We primarily develop on macOS, so any diversion of the logic would need extensive additional testing on Windows and Linux, an environment that is not easily available for us. We should thus keep the difference to the bare minimum.
The workaround already adds better event handling than the initial solution, causing #34893 (closed) to not be an issue with this workaround turned on while it is an issue for the native version. (Note this is pending further tests that are not scope of this issue).
There's only very minimal theoretical overhead, since we already reuse anything that is rendered inside the Popover component and keep that in memory. This change only adds the Popover class instance as an additional item to be kept in memory. On the other side, this has made the overall experience on macOS much better with greatly faster startup times after the initial load. We do not have data for Windows and Linux to support that, though.

Test plan

Since I am annoyed from copy-pasting on Windows, you can take a look at the test plan video from https://github.com/sourcegraph/sourcegraph/pull/36235 which I have based on those changes.

I did some manual verification and the hiding behavior works as expected on Windows.