From be8cdeb84fcf5e378b68fde3e67455c53c12b203 Mon Sep 17 00:00:00 2001 From: Victor Sanni Date: Fri, 20 Jun 2025 15:20:11 -0700 Subject: [PATCH] Close CupertinoContextMenu overlay if the widget is disposed or a new route is pushed (#170186) Fixes [CupertinoContextMenu potential unremoved overlay entry](https://github.com/flutter/flutter/issues/131471) Fixes [CupertinoContextMenu onTap gesture interferes with child widget with onTap GestureRecognizer](https://github.com/flutter/flutter/issues/169911)
Sample code ```dart import 'dart:async'; import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; bool ctxMenuRemoved = false; class ContextMenuApp extends StatelessWidget { const ContextMenuApp({super.key}); @override Widget build(BuildContext context) { final colorScheme = ColorScheme.fromSeed(seedColor: Colors.orange); return MaterialApp( theme: ThemeData( colorScheme: colorScheme, appBarTheme: AppBarTheme(backgroundColor: colorScheme.secondaryContainer), ), home: const HomePage(), ); } } class HomePage extends StatelessWidget { const HomePage({super.key}); @override Widget build(BuildContext context) => Scaffold( appBar: AppBar( title: Text('Home'), ), body: Center( child: CupertinoContextMenu( actions: [ CupertinoContextMenuAction( child: Text('Test'), ), ], child: GestureDetector( onTap: () { Navigator.of(context).push( MaterialPageRoute(builder: (context) => _OtherPage()), ); }, child: Container( color: Colors.orange, height: 100, width: 100, ), ), ), ), ); } class _OtherPage extends StatelessWidget { const _OtherPage({super.key}); @override Widget build(BuildContext context) { return Scaffold( body: Align( child: Builder(builder: (context) { return Listener( onPointerDown: (_) { Timer(const Duration(milliseconds: 480), () { ctxMenuRemoved = true; (context as Element).markNeedsBuild(); }); }, child: ctxMenuRemoved ? const SizedBox() : CupertinoContextMenu( actions: [ CupertinoContextMenuAction( child: const Text('Action one'), onPressed: () {}, ), ], child: Container( height: 100, width: 100, color: Colors.black45, ), ), ); }), ), ); } } ```
--- .../lib/src/cupertino/context_menu.dart | 43 +++++---- .../test/cupertino/context_menu_test.dart | 96 +++++++++++++++++++ 2 files changed, 121 insertions(+), 18 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/context_menu.dart b/packages/flutter/lib/src/cupertino/context_menu.dart index 05fd9cc0875..d93fed0ea32 100644 --- a/packages/flutter/lib/src/cupertino/context_menu.dart +++ b/packages/flutter/lib/src/cupertino/context_menu.dart @@ -488,6 +488,25 @@ class _CupertinoContextMenuState extends State with Ticker _route!.animation!.addStatusListener(_routeAnimationStatusListener); } + void _removeContextMenuDecoy() { + // Keep the decoy on the screen for one extra frame. We have to do this + // because _ContextMenuRoute renders its first frame offscreen. + // Otherwise there would be a visible flash when nothing is rendered for + // one frame. + SchedulerBinding.instance.addPostFrameCallback((Duration _) { + if (mounted) { + _closeContextMenu(); + _openController.reset(); + } + }, debugLabel: 'removeContextMenuDecoy'); + } + + void _closeContextMenu() { + _lastOverlayEntry?.remove(); + _lastOverlayEntry?.dispose(); + _lastOverlayEntry = null; + } + void _onDecoyAnimationStatusChange(AnimationStatus animationStatus) { switch (animationStatus) { case AnimationStatus.dismissed: @@ -496,28 +515,15 @@ class _CupertinoContextMenuState extends State with Ticker _childHidden = false; }); } - _lastOverlayEntry?.remove(); - _lastOverlayEntry?.dispose(); - _lastOverlayEntry = null; - + _closeContextMenu(); case AnimationStatus.completed: - setState(() { - _childHidden = true; - }); _openContextMenu(); - // Keep the decoy on the screen for one extra frame. We have to do this - // because _ContextMenuRoute renders its first frame offscreen. - // Otherwise there would be a visible flash when nothing is rendered for - // one frame. - SchedulerBinding.instance.addPostFrameCallback((Duration _) { - _lastOverlayEntry?.remove(); - _lastOverlayEntry?.dispose(); - _lastOverlayEntry = null; - _openController.reset(); - }, debugLabel: 'removeContextMenuDecoy'); - + _removeContextMenuDecoy(); case AnimationStatus.forward: case AnimationStatus.reverse: + if (!ModalRoute.of(context)!.isCurrent) { + _removeContextMenuDecoy(); + } return; } } @@ -617,6 +623,7 @@ class _CupertinoContextMenuState extends State with Ticker @override void dispose() { + _closeContextMenu(); _tapGestureRecognizer.dispose(); _openController.dispose(); super.dispose(); diff --git a/packages/flutter/test/cupertino/context_menu_test.dart b/packages/flutter/test/cupertino/context_menu_test.dart index 2fd425935f7..da4f678713a 100644 --- a/packages/flutter/test/cupertino/context_menu_test.dart +++ b/packages/flutter/test/cupertino/context_menu_test.dart @@ -1145,4 +1145,100 @@ void main() { expect(find.text('Item 0'), findsOneWidget); expect(find.text('Item 1'), findsOneWidget); }); + + testWidgets('Pushing a new route removes overlay', (WidgetTester tester) async { + final Widget child = getChild(); + const String page = 'Page 2'; + await tester.pumpWidget( + CupertinoApp( + home: Builder( + builder: (BuildContext context) { + return Center( + child: CupertinoContextMenu( + actions: const [CupertinoContextMenuAction(child: Text('Test'))], + child: GestureDetector( + onTap: () { + Navigator.of(context).push( + CupertinoPageRoute( + builder: + (BuildContext context) => + const CupertinoPageScaffold(child: Text(page)), + ), + ); + }, + child: child, + ), + ), + ); + }, + ), + ), + ); + + expect(find.byWidget(child), findsOneWidget); + final Rect childRect = tester.getRect(find.byWidget(child)); + expect(find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_DecoyChild'), findsNothing); + + // Start a press on the child. + final TestGesture gesture = await tester.startGesture(childRect.center); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + expect(find.text(page), findsNothing); + + await tester.pump(const Duration(milliseconds: 300)); + await gesture.up(); + + // Kickstart the route transition. + await tester.pump(); + await tester.pump(const Duration(milliseconds: 300)); + + // As the transition starts, the overlay has been removed. + // Only the child transitioning out is shown. + expect(find.text(page), findsOneWidget); + expect(find.byWidget(child), findsOneWidget); + }); + + testWidgets('Removing context menu from widget tree removes overlay', ( + WidgetTester tester, + ) async { + final Widget child = getChild(); + bool ctxMenuRemoved = false; + late StateSetter setState; + await tester.pumpWidget( + CupertinoApp( + home: StatefulBuilder( + builder: (BuildContext context, StateSetter stateSetter) { + setState = stateSetter; + return Center( + child: + ctxMenuRemoved + ? const SizedBox() + : CupertinoContextMenu( + actions: [ + CupertinoContextMenuAction(child: const Text('Test'), onPressed: () {}), + ], + child: child, + ), + ); + }, + ), + ), + ); + + expect(find.byWidget(child), findsOneWidget); + final Rect childRect = tester.getRect(find.byWidget(child)); + + // Start a press on the child. + final TestGesture gesture = await tester.startGesture(childRect.center); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 500)); + + setState(() { + ctxMenuRemoved = true; + }); + await gesture.up(); + await tester.pumpAndSettle(); + + expect(find.byWidget(child), findsNothing); + }); }