Skip to content

Wildcard: Add tether position calculation sub package for future popover component

Administrator requested to merge vk/add-popover-tether into main

Created by: vovakulikov

PRs map

  1. This PR (Add tether sub package PR)
  2. Adding Popover component
  3. PRs from the third phase could be merged in parallel and reviewed independently 3.1 Use Popover component in Menu button instead of built-in reach UI popover 3.2 Migrate Code Insights to new wildcard Popover component

Context

This PR is the first PR of the series of changes of implementation Popover component. Previous these changes were in one big PR here

  1. This PR - tether package
  2. Popover components implementation

This PR adds a custom solution for tooltip, popover, or floating panel positioning. This PR is only about position logic and doesn't add anything else (like popover components, adoption of this solution, or even demo) All of these will be added in separate PRs.

Sidenote about @floating-ui

It's a fair question why do we need to use something custom for this problem, does some OSS lib solve this problem better? Well, the most popular solution in the OSS market now is floating-ui (previously popperJS)

I tried to use them in some code insights specific use cases where we use the Popover component. Here are the problems that I've got in the code insights drill-down popover with floating-ui implementation.

  • Determine available space and set this as a max-size max-height to popup element. Here we need to point out that @floating-ui has a special setting/plugin for it but this doesn't work really well if we need a floating panel with auto-placement and sizing at the same time.
  • Flipping isn't perfect as well. TLDR; with size, shift, flip plugins at least in code insights popovers this logic picks non-optimal sizes/sides of popover elements.

If you're curious here is a draft PR where you can check the implementation in action https://github.com/sourcegraph/sourcegraph/pull/29488

Merge request reports