fix: panel position in scrolled container#1338
Conversation
Make sure panel positioning is correct if the panel is placed in a container that has been scrolled horizontally or vertically.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 54775cb:
|
|
I'm a little confused, are you suggesting to revert #1336 ? |
Well, it's a bit more complicated than that :D 1.19.6 has actually two problems with positioning, both coming from
Let me illustrate on This is how on the
And the on current
And on
The options I see are:
Testing this is quite a tedious manual job, and given the lack of clarity on to-be-supported browsers, it is impossible for a human to tell if this does not break something in a browser where it should not. The |
|
Yes, that makes sense, but I'm somewhat confused what you mean that 1.19.6 has to do with this? your previous PR hasn't been released yet, is the scrollIntoView interfering somehow? |
|
browser support is evergreen browsers in general, there's never been an explicit user request for a browser to supported by autocomplete. Do you have specific browsers in mind that aren't tested? older safari? ie? |
I guess "evergreen browsers" in practice translates to latest Chrome, Safari, Firefox and Edge on desktop? Mobile gets wild, and you might want to be specific with operating systems too. In any case, I can manually test on those on macOS but I might not go the long way and booting up Windows somewhere:) Btw there is also But I personally don't care about anything but most recent versions of mainstream desktop browsers. No request for anything exotic/ancient :)
1.19.6 is the currently released version, and both "positioning inside a positioned container" and "positioning inside a scrollable container" are broken on that version. My two PRs together fix both. The first PR (#1336) fixes "positioning inside a positioned container" and makes "positioning inside a scrollable container" broken in a different way. The second PR (this one, #1338) fixes Sorry for the confusing mess, I was initially confused by these being two separate problems but implemented at the same place, and I also did not fully understand the implementation at first. If there are available resources to throw at this, or someone wants to volunteer, I'd recommend adding end-to-end tests using Playwright, Cypress or something similar to verify correct rendering of such details, but I'm afraid won't be the person doing that :) |
The PRs #1338 and #1336 were very promising, but caused some regressions in positioned containers. Therefore for now i'll revert them. @salomvary, if you could reimplement these changes, but taking the relative positioning in account, that would be ideal. fixes #1342
The PRs #1338 and #1336 were very promising, but caused some regressions in positioned containers. Therefore for now i'll revert them. @salomvary, if you could reimplement these changes, but taking the relative positioning in account, that would be ideal. fixes #1342



Make sure panel positioning is correct if the panel is placed in a container that has been scrolled
horizontally or vertically.
Summary
When the panel is positioned in a scrollable container (the
bodyor another scrollable element) the panel position is wrong if the container has actually been scrolled as explained in this issue comment: #763 (comment)Result
yarn workspace @algolia/autocomplete-example-panel-placement-positioned startI've tested this in latest Firefox, Chrome and Safari on macOS. I am not entirely sure what browsers should be supported (there seems to be no documentation on that, and
.browserslistrcis not very helpful for humans), but this way of positioning seems to be cross-browser, cross-platform.