Skip to content

fix: change popover position to fixed to resolve position issues in absolute layouts #1114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ignatvilesov
Copy link

@ignatvilesov ignatvilesov commented Jul 11, 2025

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests
  • Other

Why is it needed?

Popover is positioned incorrectly in absolute scrollable containers like virtual tables.

Before

Screenshot 2025-07-11 at 10 42 25

After

Screenshot 2025-07-11 at 10 46 42

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have ran pnpm change and documented my changes
  • I have add necessary docs (if needed)
  • Added new tests to cover the fix / functionality

Copy link

changeset-bot bot commented Jul 11, 2025

🦋 Changeset detected

Latest commit: c7f2213

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@qwik-ui/headless Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jul 11, 2025

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@ignatvilesov ignatvilesov marked this pull request as ready for review July 11, 2025 07:51
@ignatvilesov ignatvilesov force-pushed the popover-fixed-position branch 2 times, most recently from d8bef2a to 0424d41 Compare July 11, 2025 07:55
@ignatvilesov
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@ignatvilesov
Copy link
Author

recheck

@ignatvilesov ignatvilesov force-pushed the popover-fixed-position branch from 0424d41 to c7f2213 Compare July 11, 2025 07:59
@ignatvilesov
Copy link
Author

@shairez Hey Shai 👋 I hope you're well.

It would nice if you could review this tiny PR whenever you have some free time.

Copy link

pkg-pr-new bot commented Jul 16, 2025

Open in StackBlitz

npm i https://pkg.pr.new/qwikifiers/qwik-ui@1114
npm i https://pkg.pr.new/qwikifiers/qwik-ui/@qwik-ui/headless@1114
npm i https://pkg.pr.new/qwikifiers/qwik-ui/@qwik-ui/styled@1114
npm i https://pkg.pr.new/qwikifiers/qwik-ui/@qwik-ui/utils@1114

commit: c7f2213

@shairez
Copy link
Contributor

shairez commented Jul 16, 2025

thanks @ignatvilesov !

@thejackshelton what is your opinion about this change?
as there are a couple of implications of changing this to fixed no?

@thejackshelton
Copy link
Collaborator

thejackshelton commented Jul 16, 2025

thanks @ignatvilesov !

@thejackshelton what is your opinion about this change? as there are a couple of implications of changing this to fixed no?

Positioned elements around an anchor should not be fixed. They are purposely absolute, making them fixed would break a lot of existing apps. To see this in practice, you can edit the css locally in the browser on the qwik ui popover docs from absolute to fixed.

That said, position fixed and strategy: "fixed", are likely two different things, where floating ui makes a different computation than normal.

Popover is positioned incorrectly in absolute scrollable containers like virtual tables.

Hey @ignatvilesov! Appreciate the initiative on a PR.

Have you tried making the outside containing block position relative? It looks like your tooltip does not have a relative container, absolute as a result is going to the bottom of the closest containing block (with the viewport being the last containing block, this goes into the layout algorithms of positioning).

Floating UI recommends the absolute strategy as that is the recommended and performant default. I'd be open to exposing an option to having the ability to change strategies and documenting it if there was a clear and common use case for fixed, but if it is just an issue with containing blocks, then that is a behavior of CSS itself.

Hope this helps in figuring out the containing block issue 🙏 . In either case as a last resort you can override the styles in your project, as that is the purpose of the styles in an @layer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants