Fix regressions introduced in Popover

[acelaya]

This PR reverts a few of the changes introduced in #2011, which have caused a few visual regressions (see hypothesis/client#7159 (comment) and #2013).

The root cause was that the DOM structure was changed, adding one extra element between the outermost Popover element and the children, and moving some of the classes to this new element, including the propagation of the classes prop.

#2013 was an attempt at working around one of those issues, but by checking how we use Popover.classes in different places, I realized it would not be possible to consistently split those into classes and containerClasses (or contentClasses), specially considering Select makes use of these with its own popoverClasses, which would require splitting in two as well.

Instead, this PR restructures the DOM elements again, so that it is exactly as before, when arrow is false, and when it is true, it only changes overflow-y-auto overflow-x-hidden with overflow-visible in the outermost element.

This overflow difference was the motivation of the original DOM structure, but it's now clear the implemented solution was overkill.

Having visible overflow might require tweaks in consuming code, but since that is only going to affect new implementations where arrow is true, we can address those on a case-by-case basis, but ensuring all other existing Popover usages are "safe" of regressions.

[acelaya]

The offset has apparently changed a bit, although I am not able to tell by visually comparing it.

The test is still valid though, as it demonstrates the offset is bigger with arrow than without it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (e9ef507) to head (4ffc27e).
⚠️ Report is 157 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #2014   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           70        70           
  Lines         1329      1330    +1     
  Branches       497       498    +1     
=========================================
+ Hits          1329      1330    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[robertknight]

Having visible overflow might require tweaks in consuming code, but since that is only going to affect new implementations where arrow is true, we can address those on a case-by-case basis, but ensuring all other existing Popover usages are "safe" of regressions.

Except for the concern about regressions in existing components which rely on this behavior, could we always make overflow visible?

[acelaya]

Having visible overflow might require tweaks in consuming code, but since that is only going to affect new implementations where arrow is true, we can address those on a case-by-case basis, but ensuring all other existing Popover usages are "safe" of regressions.

Except for the concern about regressions in existing components which rely on this behavior, could we always make overflow visible?

I think there was a reason for the overflow to be set like that, so I'm not completely sure.

It's perfectly possible it was due to one specific case, and that ended up here too soon, while it should have been handled in consuming code.

Once this is merged, I can probably try to look for all cases where we use Popover and see if overflow can be safely made visible.

[robertknight]

Looks good. We discussed in a Slack thread that it isn't ideal to have arrow: true have this side effect of changing overflow behavior. We discussed that potentially it can be eliminated by moving the overflow rules down into components that need it (Select was mentioned as the main reason), but this is best done as a separate PR.