Repository navigation
fix: let a change of open end a running transition in the alerts and the tooltip - #2571
Merged
Merged
Conversation
…the tooltip - Toast, snackbar: a change of open against a running fade now ends it, and the component keeps the new state, as in the dialog. open = false and then open = true during the fade-out keeps the component open and starts the display time again. open = false during the fade-in stops it. The alert base now uses the toggle controller, and the methods resolve false when a newer call or an open change ends their fade. - Tooltip: a change of open now ends a show or a hide that waits for its delay or animates. The method resolves false, and no igcOpened or igcClosed follows. Internal commits no longer go through the open setter. Before, the delayed show or hide overwrote open or played again over the new state. - A set of the current value is not a change, because the React wrappers set every property on each render. - Toggle controller: emitEvent is optional for a host that asks for no events. - Specs, spec.md files and CHANGELOG.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The missing _isShown guard lets hide() start a fade-out and resolve true on an already-closed alert while a failed show() is still pending, violating its documented no-op behavior.
1 open issue
What changed in this PR
This PR makes changes to open interrupt pending transitions in toast, snackbar, and tooltip while preserving same-value assignments used by React wrappers.
Changes:
- Migrates alerts to the shared toggle controller and makes event emission optional.
- Cancels tooltip delays and animations when
openchanges. - Adds regression tests and updates component specifications and release notes.
| File | Description |
|---|---|
src/internals/testing/alert.spec.ts |
Tests interrupted alert fades and same-value assignments. |
src/internals/controllers/toggle.ts |
Allows hosts without an event emitter. |
src/internals/bases/alert.ts |
Uses the toggle controller to coordinate alert transitions. |
src/components/tooltip/tooltip.ts |
Separates internal state commits from external transition cancellation. |
src/components/tooltip/tooltip.spec.ts |
Tests cancellation during delays and animations. |
src/components/tooltip/spec.md |
Documents interrupted tooltip transitions. |
src/components/toast/spec.md |
Documents alert fade cancellation and test scenarios. |
src/components/snackbar/spec.md |
Updates shared alert behavior documentation. |
CHANGELOG.md |
Records alert and tooltip transition fixes. |
Review effort: Balanced
A show() that finds no visible ancestor in container positioning stays pending in the toggle controller for one microtask. A hide() in that time superseded it, faded out the closed component and resolved true. hide() checks again that the component is shown before it closes it.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A critical animation-cancellation bug can leave a reopened tooltip inert after a queued hide is canceled by toggling open, requiring cleanup of outstanding animations and inert state plus a regression test.
1 open issue
1 resolved since last review
Review effort: Balanced
- Tooltip: a superseded transition now stops its animation at once. A hide trigger during the show animation ends the animation, and the tooltip is not inert during the hide delay, so a pointer on it keeps it open. Before, a change of open, a new show trigger or show() in that delay could leave the open tooltip inert. - Tooltip: open = undefined closes the tooltip again, a change of open in an igcOpening or igcClosing handler ends the transition, and a show trigger keeps an inert that the application set. - Toast, snackbar: show() checks for a visible ancestor before the toggle controller, so a refused show() no longer blocks the next one in the same task. The open property says that setting it skips the fade. - Toggle controller: a closing getter replaces the alert's own flag. - Specs, spec.md files and CHANGELOG.
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
Type of Change
Checklist
CHANGELOG.mdentry under[Unreleased]