refactor(overlay): stop moving to body container & deprecate outlet#16989
Open
refactor(overlay): stop moving to body container & deprecate outlet#16989
Conversation
wnvko
added
🛠️ status: in-development
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces an opt-in keepInPlace flag for OverlaySettings to prevent overlay content from being moved to a centralized overlay container, and begins deprecating OverlaySettings.outlet in favor of the new behavior.
Changes:
- Added
keepInPlace?: booleantoOverlaySettingsand implemented in-place wrapping/unwrapping logic inIgxOverlayService. - Deprecated
OverlaySettings.outletand updated multiple feature READMEs + changelog to document the new option. - Updated grid/pivot-grid filtering overlay settings and adjusted dialog/overlay tests to account for in-place wrapping behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/igniteui-angular/core/src/services/overlay/utilities.ts | Adds keepInPlace to OverlaySettings; marks outlet as deprecated. |
| projects/igniteui-angular/core/src/services/overlay/overlay.ts | Implements in-place wrapping path for overlays + cleanup changes. |
| projects/igniteui-angular/core/src/services/overlay/overlay.spec.ts | Adds unit tests validating keep-in-place wrapping/unwrapping and mixed modes. |
| projects/igniteui-angular/grids/grid/src/grid-base.directive.ts | Sets keepInPlace on grid filter and advanced filtering overlay settings. |
| projects/igniteui-angular/grids/pivot-grid/src/pivot-grid.component.ts | Sets keepInPlace on pivot grid ESF filter overlay settings. |
| projects/igniteui-angular/directives/src/directives/toggle/toggle.directive.ts | Defaults toggle attach to { keepInPlace: true, ...overlaySettings }. |
| projects/igniteui-angular/dialog/src/dialog/dialog.component.spec.ts | Updates assertions to locate wrapper within the component host when in-place wrapping is used. |
| projects/igniteui-angular/core/src/services/overlay/README.md | Documents keepInPlace and deprecates outlet in docs. |
| projects/igniteui-angular/core/src/services/overlay/position/README.md | Notes outlet deprecation in positioning docs. |
| projects/igniteui-angular/drop-down/src/drop-down/autocomplete/README.md | Updates autocomplete overlay settings docs; notes outlet deprecation. |
| projects/igniteui-angular/directives/src/directives/toggle/README.md | Deprecates igxToggleOutlet docs and adds keepInPlace guidance. |
| projects/igniteui-angular/date-picker/src/date-picker/README.md | Marks date-picker outlet as deprecated in docs. |
| projects/igniteui-angular/date-picker/src/date-range-picker/README.md | Marks date-range-picker outlet as deprecated in docs. |
| CHANGELOG.md | Adds an entry for the new keepInPlace feature and outlet deprecation. |
projects/igniteui-angular/grids/grid/src/grid-base.directive.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/grids/pivot-grid/src/pivot-grid.component.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/core/src/services/overlay/utilities.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/core/src/services/overlay/position/README.md
Outdated
Show resolved
Hide resolved
mtsvyatkova
added
💥 status: in-test
wnvko
force-pushed
the
mvenkov/do-not-move-overlay-element-from-its-position
branch
from
1b73b1b to
73ce1fa
Compare
simeonoff
previously approved these changes
Mar 10, 2026
wnvko
force-pushed
the
mvenkov/do-not-move-overlay-element-from-its-position
branch
from
51e13b3 to
62135bb
Compare
projects/igniteui-angular/grids/hierarchical-grid/src/hierarchical-grid-base.directive.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/grids/core/src/common/grid.interface.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/grids/grid/src/grid-base.directive.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/drop-down/src/drop-down/autocomplete/README.md
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/directives/src/directives/toggle/README.md
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/directives/src/directives/notification/notifications.directive.ts
Outdated
Show resolved
Hide resolved
projects/igniteui-angular/date-picker/src/date-range-picker/README.md
Outdated
Show resolved
Hide resolved
wnvko
force-pushed
the
mvenkov/do-not-move-overlay-element-from-its-position
branch
from
32e9669 to
7b59366
Compare
wnvko
force-pushed
the
mvenkov/do-not-move-overlay-element-from-its-position
branch
from
7b59366 to
66c36b1
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
projects/igniteui-angular/core/src/services/overlay/position/container-position-strategy.ts
Dismissed
Show dismissed
Hide dismissed
wnvko
force-pushed
the
mvenkov/do-not-move-overlay-element-from-its-position
branch
from
2ded560 to
91d3085
Compare
damyanpetev
changed the title
damyanpetev
added
squash-merge
damyanpetev
requested changes
Mar 13, 2026
Member
damyanpetev
left a comment
damyanpetev
left a comment
There was a problem hiding this comment.
Might need to discuss the additional container properties, as I missed how that strategy worked before and those might be redundant, which will shift a few more things around.
| } | ||
| this.io = Util.setupIntersectionObserver( | ||
| outletElement, | ||
| target as HTMLElement || outletElement, |
Member
There was a problem hiding this comment.
Checks above already narrow the type to just that option
Suggested change
| target as HTMLElement || outletElement, | |
| target || outletElement, |
| target as HTMLElement || outletElement, | ||
| contentElement.ownerDocument, | ||
| () => this.updatePosition(contentElement) | ||
| () => this.updatePosition(contentElement, target as HTMLElement) |
Member
There was a problem hiding this comment.
Suggested change
| () => this.updatePosition(contentElement, target as HTMLElement) | |
| () => this.updatePosition(contentElement, target) |
| () => this.updatePosition(contentElement, target as HTMLElement) | ||
| ); | ||
| this.internalPosition(contentElement); | ||
| this.internalPosition(contentElement, target as HTMLElement); |
Member
There was a problem hiding this comment.
Suggested change
| this.internalPosition(contentElement, target as HTMLElement); | |
| this.internalPosition(contentElement, target); |
Comment on lines
+55
to
+68
| if (outletElement) { | ||
| const outletRect = outletElement.getBoundingClientRect(); | ||
| contentElement.parentElement.style.width = `${outletRect.width}px`; | ||
| contentElement.parentElement.style.height = `${outletRect.height}px`; | ||
| contentElement.parentElement.style.top = `${outletRect.top}px`; | ||
| contentElement.parentElement.style.left = `${outletRect.left}px`; | ||
| } | ||
| if (targetElement) { | ||
| const targetRect = targetElement.getBoundingClientRect(); | ||
| contentElement.parentElement.style.width = `${targetRect.width}px`; | ||
| contentElement.parentElement.style.height = `${targetRect.height}px`; | ||
| contentElement.parentElement.style.top = `${targetRect.top}px`; | ||
| contentElement.parentElement.style.left = `${targetRect.left}px`; | ||
| } |
Member
There was a problem hiding this comment.
Oh absolutely not - this was a bit much to begin with, duplicating it not helping.
Call it with either/or like this.updatePosition(contentElement ?? targetElement) and redo the flow to fit into a single logic path
Member
There was a problem hiding this comment.
Also, from what I see the strategy didn't actually use the outlet property? Merely sized itself on the current DOM placement? We might need to discuss, since that's pretty reasonable and we might not need to do extra work to begin with.
| | HorizontalAlignment.Center | VerticalAlignment.Middle | | ||
|
|
||
| 2) **Container** - Positions the element inside the containing outlet based on the directions passed in trough PositionSettings. These are Top/Middle/Bottom for verticalDirection and Left/Center/Right for horizontalDirection. Defaults to: | ||
| 2) **Container** - Positions the element inside a target container element. The `target` property in `OverlaySettings` must be set to an `HTMLElement` that serves as the container. The overlay wrapper is sized and positioned to match the container's bounds and automatically updates on resize via `ResizeObserver`. Directions are passed in through PositionSettings (Top/Middle/Bottom for verticalDirection and Left/Center/Right for horizontalDirection). Defaults to: |
Member
There was a problem hiding this comment.
Eh keep the slop down :D That info is entirely redundant for external use.
Suggested change
| 2) **Container** - Positions the element inside a target container element. The `target` property in `OverlaySettings` must be set to an `HTMLElement` that serves as the container. The overlay wrapper is sized and positioned to match the container's bounds and automatically updates on resize via `ResizeObserver`. Directions are passed in through PositionSettings (Top/Middle/Bottom for verticalDirection and Left/Center/Right for horizontalDirection). Defaults to: | |
| 2) **Container** - Positions the element inside a target container element. The `target` property in `OverlaySettings` must be set to an `HTMLElement` that serves as the container. Directions are passed in through PositionSettings (Top/Middle/Bottom for verticalDirection and Left/Center/Right for horizontalDirection). Defaults to: |
Also, if we keep the target, I fail to see why we can't position against a point if that's what's provided.
| @ViewChild('loadingOverlay', { read: IgxToggleDirective, static: true }) | ||
| public loadingOverlay: IgxToggleDirective; | ||
|
|
||
| // outlet - not public - do not deprecate |
Member
There was a problem hiding this comment.
Yes, right, also don't leave rando comments in the source for no reason
Suggested change
| // outlet - not public - do not deprecate |
| @ViewChild('tfoot', { static: true }) | ||
| public tfoot: ElementRef<HTMLElement>; | ||
|
|
||
| // outlet - not public - do not deprecate |
Member
There was a problem hiding this comment.
Suggested change
| // outlet - not public - do not deprecate |
| @ContentChildren(IgxPaginatorToken) | ||
| protected paginationComponents: QueryList<IgxPaginatorComponent>; | ||
|
|
||
| // outlet - not public - do not deprecate |
Member
There was a problem hiding this comment.
Suggested change
| // outlet - not public - do not deprecate |
| </igx-grid-header-row> | ||
|
|
||
| <div igxGridBody (keydown.control.c)="copyHandler($event)" (copy)="copyHandler($event)" class="igx-grid__tbody" role="rowgroup"> | ||
| <div #gridBody igxGridBody (keydown.control.c)="copyHandler($event)" (copy)="copyHandler($event)" class="igx-grid__tbody" role="rowgroup"> |
Member
There was a problem hiding this comment.
Suggested change
| <div #gridBody igxGridBody (keydown.control.c)="copyHandler($event)" (copy)="copyHandler($event)" class="igx-grid__tbody" role="rowgroup"> | |
| <div igxGridBody (keydown.control.c)="copyHandler($event)" (copy)="copyHandler($event)" class="igx-grid__tbody" role="rowgroup"> |
There are already a dozen places where grid body is used in logic, so there are plenty of selectors for it, prob a viewchild - use what's there already?
Comment on lines
+9
to
+20
| - `IgxOverlayService` | ||
| - The overlay service now integrates the HTML Popover API and prefers rendering content in place / in the top layer, significantly reducing the need for outlet containers. When configured, `OverlaySettings.outlet` is still honored, and overlays may also fall back to being appended to `document.body` when required. | ||
|
|
||
| - **Deprecation** - The `outlet` property in `OverlaySettings`, `IgxOverlayOutletDirective`, and `igxToggleOutlet` input on `IgxToggleActionDirective` are deprecated and will be removed in a future version. They remain functional in this release for backward compatibility, but new code should rely on the default in-place / top-layer rendering behavior instead of custom outlet containers. | ||
|
|
||
| - `ContainerPositionStrategy` - The `ContainerPositionStrategy` now uses the `target` property from `OverlaySettings` (when set to an `HTMLElement`) as the container in which the overlay is rendered. This replaces the previous reliance on the deprecated `outlet` property and internal DOM traversal. The overlay wrapper is sized and positioned to match the target container's bounds and automatically updates on resize via `ResizeObserver`. | ||
|
|
||
| - `IgxOverlayService.createAbsoluteOverlaySettings` - Added a new overload accepting `container?: HTMLElement` as the second parameter. When a container element is provided, a `ContainerPositionStrategy` is used and the `target` in the returned overlay settings is set to the container element. The previous overload accepting `outlet?: IgxOverlayOutletDirective | ElementRef` is still supported but deprecated. | ||
|
|
||
| - `IgxNotificationsDirective`, `IgxSnackbarComponent`, `IgxToastComponent` | ||
| - Added a new `container` input property of type `HTMLElement`. When set, overlay content is rendered inside the given container using `ContainerPositionStrategy`. The deprecated `outlet` property now points users to `container` as its replacement. | ||
|
|
Member
There was a problem hiding this comment.
Only the new IgxOverlayService.createAbsoluteOverlaySettings can really be considered a new feature and the container prop, though that's repalacing.
The behavior change and deprecation fall under a different section (prob General).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #16825
Closes #17041
With this PR we are introducing
keepInPlaceproperty inOverlaySettings. When set totrueoverlay will not move the element to be shown out of its position. Instead the element will be wrapped in a content element and in a wrapper element. The new property default value isfalseto keep the current behavior.With this PR we also deprecate the
outletproperty ofOverlaySettings. This is not needed anymore as overlay element could be kept in its original position via the newkeepInPlaceproperty.Next steps:
outlet.keepInPlacedefault value true and deprecate it.keepInPlaceand always keep the overlay element at its original position.Additional information (check all that apply):
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)