Skip to content

feat(overlay) use popover in overlay service#16835

Merged
ChronosSF merged 42 commits intomasterfrom
mvenkov/use-popover-in-overlay
Feb 19, 2026
Merged

feat(overlay) use popover in overlay service#16835
ChronosSF merged 42 commits intomasterfrom
mvenkov/use-popover-in-overlay

Conversation

@wnvko
Copy link
Contributor

@wnvko wnvko commented Jan 30, 2026

Use popover global attribute to show the overlay. We are adding the attribute to overlay wrapper element and show the overlay by calling showPopover.

Closes #16824

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@wnvko wnvko added the ❌ status: awaiting-test PRs awaiting manual verification label Jan 30, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes the overlay service by adopting the native Popover API to manage element layering, replacing the traditional z-index approach. The implementation adds the popover="manual" attribute to overlay wrapper elements and uses showPopover()/hidePopover() methods to control visibility in the browser's top layer.

Changes:

  • Added Popover API integration with showPopover() call before positioning and hidePopover() during cleanup
  • Removed z-index styling in favor of browser-managed top layer positioning
  • Added comprehensive style overrides to reset browser default popover styles while maintaining custom overlay positioning

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
projects/igniteui-angular/core/src/services/overlay/overlay.ts Implements Popover API calls in show/hide logic with proper error handling and browser compatibility checks
projects/igniteui-angular/core/src/core/styles/components/overlay/_overlay-theme.scss Removes z-index properties and adds popover attribute styles to override browser defaults

@wnvko wnvko marked this pull request as draft January 30, 2026 12:04
@wnvko wnvko marked this pull request as ready for review February 3, 2026 09:12
@dafo dafo added 💥 status: in-test PRs currently being tested and removed ❌ status: awaiting-test PRs awaiting manual verification labels Feb 3, 2026
@Hristo313 Hristo313 added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Feb 4, 2026
@wnvko wnvko changed the title Use popover in overlay service feat(overlay) use popover in overlay service Feb 6, 2026
simeonoff
simeonoff previously approved these changes Feb 11, 2026
@coveralls
Copy link
Collaborator

coveralls commented Feb 17, 2026

Coverage Status

coverage: 91.585%. first build
when pulling 5d24d46 on mvenkov/use-popover-in-overlay
into 5b681a1 on master.

@gedinakova
Copy link
Contributor

@wnvko When a dialog is open, when resizing smaller the browser, the first time the dialog briefly goes above the nav-drawer before going under. In addition, when the nav-drawer is open, initially the dialog remains in its previous position, it moves only after the resizing starts.

screen-capture.9.webm

Copy link
Member

@damyanpetev damyanpetev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could probably fight to inch some smoother frames when doing animated moves, tough that's likely not too critical of a scenario. With the limited observer feedback likely not worth it, so I'd like to focus on CSS anchors that should be natively smooth and this LGTM.

@damyanpetev
Copy link
Member

damyanpetev commented Feb 19, 2026

@wnvko When a dialog is open, when resizing smaller the browser, the first time the dialog briefly goes above the nav-drawer before going under. In addition, when the nav-drawer is open, initially the dialog remains in its previous position, it moves only after the resizing starts.

screen-capture.9.webm

Um, the drawer is initially pinned (not in overlay mode), so pretty sure the dialog going on top is before the drawer unpins, after which the drawer goes on top as expected.

@wnvko
Copy link
Contributor Author

wnvko commented Feb 19, 2026

@wnvko When a dialog is open, when resizing smaller the browser, the first time the dialog briefly goes above the nav-drawer before going under. In addition, when the nav-drawer is open, initially the dialog remains in its previous position, it moves only after the resizing starts.

screen-capture.9.webm

This how it is works on 21.0.12 branch
image
I am not even sure where this dialog should be positioned. It is using connected position strategy, but actually is trying to position in the middle of the grid. And then it is draggable so can be moved around. IMO this was not working correctly and before the changes introduced in this PR and in nav-drawer related to popover attribute.

@gedinakova gedinakova added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Feb 19, 2026
@ChronosSF ChronosSF merged commit a4bdfa4 into master Feb 19, 2026
5 checks passed
@ChronosSF ChronosSF deleted the mvenkov/use-popover-in-overlay branch February 19, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

squash-merge Merge PR with "Squash and Merge" option triage: blocking version: 21.1.x ✅ status: verified Applies to PRs that have passed manual verification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use popover attribute in Overlay service

9 participants