Repository navigation
Fix: Delay showing tooltip during page transition - #167614
Conversation
| final ModalRoute<dynamic>? route = ModalRoute.of(context); | ||
| if (route?.secondaryAnimation != null && route!.secondaryAnimation!.isAnimating) { | ||
| _timer?.cancel(); | ||
| _timer = Timer(route.transitionDuration, show); |
There was a problem hiding this comment.
Is it possible to listen to when the animation ends and start the timer then? The current way isn't compatible with cases where this happens halfway during the transition or simulation-based transition where there isn't a predefined duration.
There was a problem hiding this comment.
Yaa, was thinking the same. but for some reason below written code doesn't fix the issue.
if (route?.secondaryAnimation != null && route!.secondaryAnimation!.isAnimating) {
_timer?.cancel();
void onAnimationDismiss(AnimationStatus status) {
if (status == AnimationStatus.dismissed) {
route.secondaryAnimation!.removeStatusListener(onAnimationDismiss);
show();
}
}
route.secondaryAnimation!.addStatusListener(onAnimationDismiss);
return;
}There was a problem hiding this comment.
In some cases the secondaryAnimation is swapped for a different one, so that may be why. The secondaryAnimation with the status listener is replaced.
There was a problem hiding this comment.
Rather than putting the logic here, should we have the check happen in tandem with the _visible check for _ExclusiveMouseRegion? So it ignores all mouse events while the route transitions are in motion?
There was a problem hiding this comment.
Oh yes, that will be better place and we don't even need listener.
There was a problem hiding this comment.
Pushed the change, everything looks good. Thanks.
4ef9c09 to
0617cf1
Compare
MitchellGoodwin
left a comment
There was a problem hiding this comment.
LGTM! Thank you for making this fix. Just left one tiny nit.
| expect(tester.element(textAncestors.first).size, equals(tooltipConstraints.biggest)); | ||
| }); | ||
|
|
||
| testWidgets('Tooltip does not show while transitioning from another page', ( |
There was a problem hiding this comment.
Extreme nit, for this case I think it needs a link to the issue it is checking against.
| testWidgets('Tooltip does not show while transitioning from another page', ( | |
| // This is a regression test for https://github.com/flutter/flutter/issues/167359. | |
| testWidgets('Tooltip does not show while transitioning from another page', ( |
There was a problem hiding this comment.
Thanks for suggestion. Pushed the change.
0617cf1 to
910429c
Compare
Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](#168546) and [Delay showing tooltip during page transition](#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](#169741)
Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](flutter#168546) and [Delay showing tooltip during page transition](flutter#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](flutter#169741)
Fix: Delay showing tooltip during page transition fixes: flutter#167359 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](flutter#168546) and [Delay showing tooltip during page transition](flutter#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](flutter#169741)
Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](flutter#168546) and [Delay showing tooltip during page transition](flutter#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](flutter#169741)
Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](flutter#168546) and [Delay showing tooltip during page transition](flutter#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](flutter#169741)
Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](flutter#168546) and [Delay showing tooltip during page transition](flutter#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](flutter#169741)
Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](flutter/flutter#168546) and [Delay showing tooltip during page transition](flutter/flutter#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](flutter/flutter#169741)
Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](flutter/flutter#168546) and [Delay showing tooltip during page transition](flutter/flutter#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](flutter/flutter#169741)
Fix: Delay showing tooltip during page transition
fixes: #167359
Pre-launch Checklist
///).