From 7f0e50d4e4503bfb99b0befe9d147a365f21e287 Mon Sep 17 00:00:00 2001 From: 21pages Date: Mon, 2 Nov 2026 08:44:37 +0800 Subject: [PATCH] fix: distinguish request approval submenus on mobile --- .../common/widgets/request_approval_menu.dart | 43 +++++++++++++++++-- .../widgets/material_mod_popup_menu.dart | 8 +++- flutter/lib/desktop/widgets/popup_menu.dart | 6 +++ flutter/test/request_approval_menu_test.dart | 32 ++++++++++++++ 4 files changed, 84 insertions(+), 5 deletions(-) diff --git a/flutter/lib/common/widgets/request_approval_menu.dart b/flutter/lib/common/widgets/request_approval_menu.dart index 2b6ee8bad..ac6b5597b 100644 --- a/flutter/lib/common/widgets/request_approval_menu.dart +++ b/flutter/lib/common/widgets/request_approval_menu.dart @@ -1,5 +1,6 @@ import 'package:flutter/material.dart'; +import '../../common.dart'; import '../../desktop/widgets/material_mod_popup_menu.dart' as mod_menu; import '../../desktop/widgets/popup_menu.dart'; @@ -33,10 +34,13 @@ class RequestApprovalMenu extends MenuEntryBase { text: title, dismissParentOnCanceled: true, arrowScale: arrowScale, + barrierColor: isMobile ? Colors.black26 : null, padding: (padding ?? EdgeInsets.zero) .copyWith(right: (padding?.right ?? 0) + arrowInset), - entries: entries - .map((entry) => MenuEntryButton( + entries: [ + if (isMobile) + _RequestApprovalMenuTitle(title, padding), + ...entries.map((entry) => MenuEntryButton( childBuilder: entry.childBuilder, proc: entry.proc, padding: entry.padding, @@ -45,8 +49,39 @@ class RequestApprovalMenu extends MenuEntryBase { dismissCallback: () { if (Navigator.canPop(context)) Navigator.pop(context); }, - )) - .toList(), + )), + ], ).build(context, conf); } } + +class _RequestApprovalMenuTitle extends MenuEntryBase { + final String title; + final EdgeInsets? padding; + + _RequestApprovalMenuTitle(this.title, this.padding); + + @override + List> build( + BuildContext context, MenuConfig conf) => [ + mod_menu.PopupMenuItem( + enabled: false, + height: conf.height, + padding: EdgeInsets.zero, + child: TextButton( + onPressed: null, + child: Container( + alignment: AlignmentDirectional.centerStart, + height: conf.height, + padding: padding, + child: Semantics( + header: true, + child: Text(title, + style: enabledStyle(context) + .copyWith(fontWeight: FontWeight.w600)), + ), + ), + ), + ), + ]; +} diff --git a/flutter/lib/desktop/widgets/material_mod_popup_menu.dart b/flutter/lib/desktop/widgets/material_mod_popup_menu.dart index 0e085a14e..45b179912 100644 --- a/flutter/lib/desktop/widgets/material_mod_popup_menu.dart +++ b/flutter/lib/desktop/widgets/material_mod_popup_menu.dart @@ -797,6 +797,7 @@ class _PopupMenuRoute extends PopupRoute { this.initialValue, this.elevation, required this.barrierLabel, + this.barrierColor, this.semanticLabel, this.shape, this.color, @@ -832,7 +833,7 @@ class _PopupMenuRoute extends PopupRoute { bool get barrierDismissible => true; @override - Color? get barrierColor => null; + final Color? barrierColor; @override final String barrierLabel; @@ -1049,6 +1050,7 @@ Future showMenu({ String? semanticLabel, ShapeBorder? shape, Color? color, + Color? barrierColor, bool useRootNavigator = false, BoxConstraints? constraints, }) { @@ -1076,6 +1078,7 @@ Future showMenu({ elevation: elevation, semanticLabel: semanticLabel, barrierLabel: MaterialLocalizations.of(context).modalBarrierDismissLabel, + barrierColor: barrierColor, shape: shape, color: color, capturedThemes: @@ -1152,6 +1155,7 @@ class PopupMenuButton extends StatefulWidget { this.enabled = true, this.shape, this.color, + this.barrierColor, this.enableFeedback, this.constraints, this.position = PopupMenuPosition.over, @@ -1248,6 +1252,7 @@ class PopupMenuButton extends StatefulWidget { /// If [PopupMenuThemeData.color] is also null, then /// Theme.of(context).cardColor is used. final Color? color; + final Color? barrierColor; /// Whether detected gestures should provide acoustic and/or haptic feedback. /// @@ -1354,6 +1359,7 @@ class PopupMenuButtonState extends State> { position: position, shape: widget.shape ?? popupMenuTheme.shape, color: widget.color ?? popupMenuTheme.color, + barrierColor: widget.barrierColor, constraints: widget.constraints, ).then((T? newValue) { if (!mounted) return null; diff --git a/flutter/lib/desktop/widgets/popup_menu.dart b/flutter/lib/desktop/widgets/popup_menu.dart index c66f2b499..a82860848 100644 --- a/flutter/lib/desktop/widgets/popup_menu.dart +++ b/flutter/lib/desktop/widgets/popup_menu.dart @@ -17,6 +17,7 @@ class PopupMenuChildrenItem extends mod_menu.PopupMenuEntry { this.onTap, this.dismissParentOnCanceled = false, this.useButtonLayout = false, + this.barrierColor, this.position = mod_menu.PopupMenuPosition.overSide, this.offset = Offset.zero, required this.itemBuilder, @@ -31,6 +32,7 @@ class PopupMenuChildrenItem extends mod_menu.PopupMenuEntry { final void Function()? onTap; final bool dismissParentOnCanceled; final bool useButtonLayout; + final Color? barrierColor; final List> Function(BuildContext) itemBuilder; final Widget child; @@ -82,6 +84,7 @@ class MyPopupMenuItemState> : null, itemBuilder: widget.itemBuilder, padding: EdgeInsets.zero, + barrierColor: widget.barrierColor, child: Builder(builder: (context) { final child = AnimatedDefaultTextStyle( style: style, @@ -673,6 +676,7 @@ class MenuEntrySubMenu extends MenuEntryBase { final EdgeInsets? padding; final bool dismissParentOnCanceled; final double arrowScale; + final Color? barrierColor; MenuEntrySubMenu({ required this.text, @@ -680,6 +684,7 @@ class MenuEntrySubMenu extends MenuEntryBase { this.padding, this.dismissParentOnCanceled = false, this.arrowScale = MenuConfig.iconScale, + this.barrierColor, RxBool? enabled, }) : super(enabled: enabled); @@ -694,6 +699,7 @@ class MenuEntrySubMenu extends MenuEntryBase { padding: padding, dismissParentOnCanceled: dismissParentOnCanceled, useButtonLayout: true, + barrierColor: barrierColor, position: mod_menu.PopupMenuPosition.overSide, itemBuilder: (BuildContext context) => entries .map((entry) => entry.build(context, conf)) diff --git a/flutter/test/request_approval_menu_test.dart b/flutter/test/request_approval_menu_test.dart index f82d5dfcd..d8290bab9 100644 --- a/flutter/test/request_approval_menu_test.dart +++ b/flutter/test/request_approval_menu_test.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter_hbb/common.dart' as common; import 'package:flutter_hbb/common/widgets/request_approval_menu.dart'; import 'package:flutter_hbb/desktop/widgets/material_mod_popup_menu.dart' as mod_menu; @@ -89,6 +90,34 @@ Future openMenu(WidgetTester tester, List actions, } void main() { + testWidgets('only mobile approval submenus have a heading and dim background', + (tester) async { + final wasMobile = common.isMobile; + addTearDown(() => common.isMobile = wasMobile); + for (final mobile in [true, false]) { + common.isMobile = mobile; + final actions = []; + await openMenu(tester, actions); + await tester.tap(find.text('Request approval')); + await tester.pumpAndSettle(); + expect(find.text('Request approval'), findsNWidgets(mobile ? 2 : 1)); + expect( + find.byWidgetPredicate((widget) => + widget is ModalBarrier && widget.color == Colors.black26), + mobile ? findsOneWidget : findsNothing); + if (mobile) { + await tester.tap(find.text('Request approval').last); + await tester.pumpAndSettle(); + expect(actions, isEmpty); + expect(find.text('Request approval'), findsNWidgets(2)); + } + await tester.tapAt(const Offset(2, 2)); + await tester.pumpAndSettle(); + expect(find.text('Request approval'), findsNothing); + expect(find.text('Rename'), findsNothing); + } + }); + testWidgets('approval trigger has one Tab stop and opens with Enter', (tester) async { await openMenu(tester, []); @@ -221,6 +250,9 @@ void main() { testWidgets('the whole approval row opens the submenu on a narrow screen', (tester) async { + final wasMobile = common.isMobile; + common.isMobile = true; + addTearDown(() => common.isMobile = wasMobile); tester.view.physicalSize = const Size(360, 640); tester.view.devicePixelRatio = 1; addTearDown(tester.view.resetPhysicalSize);