Skip to content

JetBrains: Use DialogWrapper instead of JBPopup

Warren Gifford requested to merge ps/jetbrains-dialogwrapper into main

Created by: philipp-spiess

Closes #37322 (closed) Closes #34967 (closed)

This PR rewrites the way we handle the popover inside Java to use the DialogWrapper (the same primitive that is underneath the native Find in Files dialog) instead of a JBPopup.

There are many considerations that went into this with a lot of trial and error but I will try to recap this as good as possible:

  • The DialogWrapper makes the modal behave like it's own window which means that if another process is brought into the forgeround on macOS, it will not be rendered on top of it (Our main motivation for creating this work, c.f. #37322 (closed))
  • It does by default also look like a native window (with the app close icons etc.). We have to completely opt-out of this behavior via setUndercorated(true).
  • Undecorated modals do not have any behavior to them. E.g. they won't close if you do anything, they won't have resize listeners, nothing. Thus, we had to re-implement all these behaviors manually. Most of that is inspired by the native find in files implementation and was moved to addNativeFindInFilesBehaviors().
    • This method is applying all "tweaks" that the native find in files component is also doing. It also has special code to handle restoring of window dimensions that are saved under special considerations (e.g. different screen resolutions), so it is probably a good win for us to rely on that logic and not have to find the same mistakes over time.
  • There are two main changes that we are applying:
    • We don't dispose the window if we no longer need it because of the web view disappearance issue. Instead, we only hide it visually. This needed some special handling to persist the window size as the default behavior only triggers when the window is disposed (which it no longer is).
    • We want the window to close when another window gets focus. This needs to be scoped per-project as multiple JetBrains project can be open in the same Application. For that, we could reuse the previous registerOutsideClickListener logic.

I’m sorry for the mess in the commit history. Prior to coming back to this approach where we subclass the DialogWrapper I worked on an approach similar to the native find in files. I've taken the results and learnings from this work and applied it here in one go so you don't see the individual stages of API surgery that I did before, lol. Here's a quick history of the historical changes: https://github.com/sourcegraph/sourcegraph/commits/ps/jetbrains-find-in-files-fork

Test plan

A lot of manual testing but I need more help! Here are just some of the cases that I tried:

https://drive.google.com/file/d/1elpP1wF4JtEpDZw07qgRo20NItFYP5Wh/view?usp=sharing

App preview:

Check out the client app preview documentation to learn more.

Merge request reports