feat: Add container positioning to Toast and Snackbar components#2203
Open
rkaraivanov wants to merge 4 commits into
Open
feat: Add container positioning to Toast and Snackbar components#2203rkaraivanov wants to merge 4 commits into
rkaraivanov wants to merge 4 commits into
Conversation
- Both toast and snackbar are now popovers, ensuring top-layer rendering and no need for z-index management. Closes #2154
Contributor
There was a problem hiding this comment.
Pull request overview
Switches the toast and snackbar components to render via the popover API (top-layer) and adds a new positioning property that lets consumers anchor them to their nearest visible ancestor instead of the viewport, addressing #2154.
Changes:
- Adds
positioning: 'viewport' | 'container'to the sharedIgcBaseAlertLikeComponentmixin and usesshowPopover()(with thesourceinvoker option and CSSanchor()for container mode) instead ofposition: fixedto render in the top layer. - Moves the
_playeranimation controller from the per-component subclasses up to the base mixin, removing the duplicated declarations fromIgcToastComponentandIgcSnackbarComponent(and the now-unused_contentRef). - Adds new
ContainerPositioningstories and apositioningarg to both toast and snackbar Storybook entries.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/components/common/mixins/alert.ts | Adds popover-based show/hide flow, positioning property, container-anchor logic and visible-ancestor lookup. |
| src/components/toast/toast.ts | Removes the now-redundant local _player declaration and unused import. |
| src/components/snackbar/snackbar.ts | Removes the local _player/_contentRef, drops the ref directive on the base part, and minor template formatting. |
| stories/toast.stories.ts | Adds positioning arg/control and a new ContainerPositioning story; reworks Basic actions layout. |
| stories/snackbar.stories.ts | Adds positioning arg/control and a new ContainerPositioning story mirroring the toast one. |
Comments suppressed due to low confidence (5)
src/components/common/mixins/alert.ts:129
- CSS anchor positioning (
anchor(top|bottom|center)) and theshowPopover({ source })invoker option are currently only supported in Chromium-based browsers. In Firefox and Safari (as of 2026),positioning="container"will fail silently: the popover will still be opened in the top layer, but the inlinetop/leftstyles set toanchor(...)will be invalid and the toast/snackbar will end up positioned at0,0(or wherever the default popover styles place it) instead of within the parent container. This should at least be documented as a Chromium-only feature, and ideally a feature-detection fallback should be provided.
private _showPopover(): boolean {
if (!this._isContained) {
this.showPopover();
return true;
}
const visibleAncestor = getVisibleAncestor(this);
if (!visibleAncestor) {
return false;
}
this.style.top = this._containerPosition;
this.style.left = 'anchor(center)';
this.showPopover({ source: visibleAncestor });
return true;
}
src/components/common/mixins/alert.ts:112
- Programmatically setting the
openproperty (e.g.snackbar.open = true) no longer makes the component visible:_showPopover()is only invoked from_setOpenState()(viashow()/toggle()) and fromupdate()whenposition/positioningchange. Since the popover is registered asmanual, the element will not appear in the top layer when onlyopenis toggled directly, even though the existingsnackbar.spec.tstest'openproperty'(line 123) and the'show()/hide() are no-op...'test (line 175) rely on this property assignment to put the component in the open state. Consider reacting toopenchanges inupdate()(or removing the public setter and routing throughshow()/hide()), and update the tests to assert the popover state.
protected override update(props: PropertyValues<this>): void {
if (props.has('displayTime')) {
this._setAutoHideTimer();
}
if (props.has('keepOpen')) {
this.keepOpen
? clearTimeout(this._autoHideTimeout)
: this._setAutoHideTimer();
}
if (this.open && (props.has('positioning') || props.has('position'))) {
this._hidePopover();
this._showPopover();
}
super.update(props);
}
src/components/common/mixins/alert.ts:138
- When
positionorpositioningchanges while the component is already open,_hidePopover()and_showPopover()are called back-to-back synchronously inupdate(). This will fire twotoggleevents on the popover and bypass the fade-in/fade-out animation, which is inconsistent with the animation behavior in_setOpenState. Additionally,_hidePopover()removes the inlinetop/leftstyles only when_isContainedis currently true — ifpositioningis being changed fromcontainertoviewport, the previous-state check is wrong (the new value is already reflected onthis.positioningby the timeupdate()runs), so stale inline styles will be left on the element.
if (this.open && (props.has('positioning') || props.has('position'))) {
this._hidePopover();
this._showPopover();
}
super.update(props);
}
private _showPopover(): boolean {
if (!this._isContained) {
this.showPopover();
return true;
}
const visibleAncestor = getVisibleAncestor(this);
if (!visibleAncestor) {
return false;
}
this.style.top = this._containerPosition;
this.style.left = 'anchor(center)';
this.showPopover({ source: visibleAncestor });
return true;
}
private _hidePopover(): void {
this.hidePopover();
if (this._isContained) {
this.style.removeProperty('top');
this.style.removeProperty('left');
}
}
src/components/common/mixins/alert.ts:123
- When
positioning="container"is set but no visible ancestor is found,show()silently resolves tofalseand theopenflag is reset, with no indication to the developer of why the toast/snackbar failed to open. Consider logging a warning (or throwing) so that this misconfiguration is discoverable, especially because the most common cause — the component being detached or inside a hidden ancestor — is easy to hit during conditional rendering.
const visibleAncestor = getVisibleAncestor(this);
if (!visibleAncestor) {
return false;
}
src/components/common/mixins/alert.ts:77
- No new tests are added for the new
positioningproperty, the container-positioned_showPopover/_hidePopoverflow, or thegetVisibleAncestorhelper. The rest ofsnackbar.spec.ts/toast.spec.tsprovides comprehensive coverage of the public API (open/show/hide/toggle, displayTime, keepOpen). Please add tests for: opening withpositioning="container", opening when no visible ancestor exists (expected to fail / log), and switchingposition/positioningwhile open.
@property({ reflect: true })
public positioning: 'viewport' | 'container' = 'viewport';
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.
Description
Both toast and snackbar are now popovers, ensuring top-layer rendering and no need for z-index management.
Type of Change
Related Issues
Closes #2154
Checklist