Skip to content
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

[bug]: post-popovercontainer promotes to Top Layer, causing UI overlap issues #4398

Closed
2 tasks
alionazherdetska opened this issue Jan 8, 2025 · 5 comments · Fixed by #4589
Closed
2 tasks
Assignees
Labels
📦 components Related to the @swisspost/design-system-components package
Milestone

Comments

@alionazherdetska
Copy link
Contributor

alionazherdetska commented Jan 8, 2025

The post-popovercontainer component is automatically promoted to the top layer when used (due to its implementation of the popover attribute). This behavior causes conflicts in UI layering:

  1. Overlapping Issues:

The post-popovercontainer appears above critical elements like the header (e.g., tooltips or popovers overlapping the header).
https://github.com/user-attachments/assets/ce080ac9-4922-40e3-9bb9-11ffc7ebb735

2. Layering Conflicts:
If the header is also implemented as a top-layer element, post-popovercontainer used in megadropdown added later take precedence in the top layer, rendering firing the animation on top of the header.
https://github.com/user-attachments/assets/45fc6112-de58-4f1b-8623-18e0d88d5fe5

Image
Image

Desktop:

  • OS: [e.g., Windows 11]
  • Browser: [e.g., Chrome, Edge]
  • Version 131.0.6778.140

Tasks

Preview Give feedback
  1. 📦 components
@alionazherdetska alionazherdetska added 🐞 bug Something isn't working 📦 components Related to the @swisspost/design-system-components package labels Jan 8, 2025
@alionazherdetska alionazherdetska added the needs: 🏓 dev roundtable To be discussed at the roundtable of esteemed developers label Jan 8, 2025
@gfellerph
Copy link
Member

Resolution: Remove popovercontainer from the megadropdown and add a margin top to floating ui corresponding to the header height and accept that open popovers or tooltips overlap the header because they are in the top layer.

@gfellerph gfellerph removed the needs: 🏓 dev roundtable To be discussed at the roundtable of esteemed developers label Jan 9, 2025
@alizedebray
Copy link
Contributor

The hide feature of the floating UI middleware (https://floating-ui.com/docs/hide) may enable us to easily hide the popover when it is over the header.

@myrta2302 myrta2302 self-assigned this Jan 13, 2025
@myrta2302 myrta2302 moved this from 👀 Triage to 🕹️ Coding in progress in Design System Production Board Jan 13, 2025
@myrta2302 myrta2302 removed their assignment Jan 14, 2025
@myrta2302 myrta2302 moved this from 🕹️ Coding in progress to 👀 Triage in Design System Production Board Jan 14, 2025
@gfellerph gfellerph added this to the PPNL Support milestone Jan 15, 2025
@gfellerph gfellerph self-assigned this Jan 15, 2025
@gfellerph gfellerph removed 🐞 bug Something isn't working needs: refinement labels Jan 22, 2025
@gfellerph
Copy link
Member

The hide feature of the floating UI middleware (https://floating-ui.com/docs/hide) may enable us to easily hide the popover when it is over the header.

Not sure if we want to auto-hide something the user opened on scroll, we don't do that with anything else. My thought was just to flip positions a little bit earlier when the position is 'top'.

@gfellerph gfellerph removed their assignment Jan 22, 2025
@gfellerph gfellerph moved this from 👀 Triage to 💻 Ready for development in Design System Production Board Jan 29, 2025
@alionazherdetska alionazherdetska self-assigned this Jan 29, 2025
@oliverschuerch oliverschuerch added Header Everything related to our composible header components and removed Header Everything related to our composible header components labels Jan 30, 2025
@alionazherdetska alionazherdetska moved this from 💻 Ready for development to 🤬 Dev in Code Review in Design System Production Board Feb 3, 2025
@alionazherdetska alionazherdetska moved this from 🤬 Dev in Code Review to 💻 Ready for development in Design System Production Board Feb 3, 2025
@alizedebray
Copy link
Contributor

The hide feature of the floating UI middleware (https://floating-ui.com/docs/hide) may enable us to easily hide the popover when it is over the header.

Not sure if we want to auto-hide something the user opened on scroll, we don't do that with anything else. My thought was just to flip positions a little bit earlier when the position is 'top'.

@gfellerph do we have anything else overflowing the header?
The hide feature would not close the popovercontainer just hide it so that it seams to slide behind the header, after its position as been flipped. And if the user scrolls back in the other direction, the popovercontainer becomes visible again, and then flips back to its original position.

I also checked the native select and the default behavior is either to close the popup menu (Chrome) or to prevent scroll (Firefox).

Since we already have a resolution for this ticket so I am fine with simply implementing it, I added this comment as additional possibility.

@gfellerph
Copy link
Member

@alizedebray I did not consider just hiding it, that might indeed be the best way to handle it. @alionazherdetska can you check in connection with the new header height measurement?

@alionazherdetska alionazherdetska moved this from 💻 Ready for development to 🤬 Dev in Code Review in Design System Production Board Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 components Related to the @swisspost/design-system-components package
Projects
Status: 🚀 Done
5 participants