From 4ee9f8b35a612b5c83459af2fcee9f77000a29f6 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Sat, 19 Sep 2026 10:25:18 -0700 Subject: [PATCH] fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) Long-pressing a message while the keyboard dismissed could leave the iOS action menu visible but unresponsive. Rebuilds installed a new selection handler before the previous hook's cleanup removed it. Keep that handler for the native view's lifetime and dispatch to the latest callback. Also let taps on decorative action icons reach the row control. Added regression coverage for callback rebuilds, view replacement/dismissal, and native icon/label/edge hit targets. The callback regression fails before the fix and passes after it. Validation: all 2,257 mobile tests (4 skipped), three unconfigured-push tests, mobile analysis, 42 focused action/handler tests, and three native XCTest checks passed. Physical iOS 26.5 taps verified keyboard-open long press, Edit, Copy (including clipboard contents), repeated opening, and outside dismissal. Desktop tests/builds, Tauri checks/tests, and the web build passed. The full local gate encountered unrelated Rust agent timeout failures; all 105 tests in the affected ACP test module passed when rerun serially. GitHub CI passed, including mobile tests, Swift tests, Android build, and iOS simulator/release builds. Codex code review completed without findings. No matching open issue or PR found. ### Before Synthetic message using the production long-press/menu components: Edit and Copy taps leave the menu open; tapping outside still dismisses it. https://github.com/user-attachments/assets/5e062d38-cbca-4be2-98b5-ca9a47b89df3 ### After With the keyboard open, the first Edit icon tap opens the editor. Reopening and tapping Copy copies the message and dismisses the menu. https://github.com/user-attachments/assets/49e11bfd-d531-4fa1-b551-5abd4fec83d7 Signed-off-by: Tom Brow --- .../Runner/NativeMessageActionSurface.swift | 2 + mobile/ios/RunnerTests/RunnerTests.swift | 36 +++++++++++ .../features/channels/message_actions.dart | 1 + .../message_action_popover.dart | 12 +--- .../native_message_action_selection.dart | 24 ++++++++ .../native_message_action_selection_test.dart | 60 +++++++++++++++++++ 6 files changed, 124 insertions(+), 11 deletions(-) create mode 100644 mobile/lib/features/channels/message_actions/native_message_action_selection.dart create mode 100644 mobile/test/features/channels/native_message_action_selection_test.dart diff --git a/mobile/ios/Runner/NativeMessageActionSurface.swift b/mobile/ios/Runner/NativeMessageActionSurface.swift index 4b0d637ced..f71889eb34 100644 --- a/mobile/ios/Runner/NativeMessageActionSurface.swift +++ b/mobile/ios/Runner/NativeMessageActionSurface.swift @@ -157,6 +157,8 @@ final class NativeMessageActionRowControl: UIControl { ) let iconColumn = UIView() + // Decorative content must leave the entire row tappable. + iconColumn.isUserInteractionEnabled = false iconColumn.translatesAutoresizingMaskIntoConstraints = false actionImageView.translatesAutoresizingMaskIntoConstraints = false actionTitleLabel.translatesAutoresizingMaskIntoConstraints = false diff --git a/mobile/ios/RunnerTests/RunnerTests.swift b/mobile/ios/RunnerTests/RunnerTests.swift index d58661396b..00ce050b4f 100644 --- a/mobile/ios/RunnerTests/RunnerTests.swift +++ b/mobile/ios/RunnerTests/RunnerTests.swift @@ -904,6 +904,42 @@ class RunnerTests: XCTestCase { XCTAssertTrue(selected) } + @MainActor + func testNativeMessageActionRowReceivesTapsAcrossItsWholeSurface() throws { + let definition = try XCTUnwrap( + NativeMessageActionDefinition( + arguments: [ + "id": "edit", "title": "Edit message", + "symbol": "pencil", "group": "primary", + ] + ) + ) + var selectionCount = 0 + let row = NativeMessageActionRowControl( + definition: definition, + foregroundColor: .label, + destructiveColor: .systemRed, + onSelected: { selectionCount += 1 } + ) + row.frame = CGRect(x: 0, y: 0, width: 288, height: 48) + row.layoutIfNeeded() + + let iconCenter = row.actionImageView.convert( + CGPoint(x: row.actionImageView.bounds.midX, y: row.actionImageView.bounds.midY), + to: row + ) + let labelCenter = row.actionTitleLabel.convert( + CGPoint(x: row.actionTitleLabel.bounds.midX, y: row.actionTitleLabel.bounds.midY), + to: row + ) + for point in [CGPoint(x: 4, y: 24), iconCenter, labelCenter, CGPoint(x: 284, y: 24)] { + let target = row.hitTest(point, with: nil) + XCTAssertTrue(target === row, "Tap at \(point) must reach the action control") + (target as? UIControl)?.sendActions(for: .touchUpInside) + } + XCTAssertEqual(selectionCount, 4) + } + @MainActor func testNativeMessageActionRowExpandsForAccessibilityTypography() throws { let traits = UITraitCollection( diff --git a/mobile/lib/features/channels/message_actions.dart b/mobile/lib/features/channels/message_actions.dart index 53dd4c30f6..a8f5bbe08f 100644 --- a/mobile/lib/features/channels/message_actions.dart +++ b/mobile/lib/features/channels/message_actions.dart @@ -30,6 +30,7 @@ import '../../shared/reminders/reminder_service.dart'; import 'channel_management_provider.dart'; import 'emoji_picker.dart'; import 'message_action_backdrop_state.dart'; +import 'message_actions/native_message_action_selection.dart'; import 'reaction_row.dart'; import 'recent_emoji_provider.dart'; import '../../shared/read_state/message_read_state.dart'; diff --git a/mobile/lib/features/channels/message_actions/message_action_popover.dart b/mobile/lib/features/channels/message_actions/message_action_popover.dart index f56fb7d3ac..863043307b 100644 --- a/mobile/lib/features/channels/message_actions/message_action_popover.dart +++ b/mobile/lib/features/channels/message_actions/message_action_popover.dart @@ -477,17 +477,7 @@ class _IosNativeMessageActionSurface extends HookWidget { @override Widget build(BuildContext context) { final viewId = useState(null); - useEffect(() { - final id = viewId.value; - if (id == null) return null; - final channel = MethodChannel('buzz/native_message_action_surface/$id'); - channel.setMethodCallHandler((call) async { - if (call.method != 'selected' || call.arguments is! Map) return; - final actionId = (call.arguments as Map)['id']; - if (actionId is String) onSelected(actionId); - }); - return () => channel.setMethodCallHandler(null); - }, [viewId.value, onSelected]); + useNativeMessageActionSelection(viewId.value, onSelected); return UiKitView( key: const ValueKey('ios-native-message-action-surface'), diff --git a/mobile/lib/features/channels/message_actions/native_message_action_selection.dart b/mobile/lib/features/channels/message_actions/native_message_action_selection.dart new file mode 100644 index 0000000000..70122c4ff0 --- /dev/null +++ b/mobile/lib/features/channels/message_actions/native_message_action_selection.dart @@ -0,0 +1,24 @@ +import 'package:flutter/services.dart'; +import 'package:flutter_hooks/flutter_hooks.dart'; + +/// Receives selections from the native message menu while [viewId] is mounted. +void useNativeMessageActionSelection( + int? viewId, + ValueChanged onSelected, +) { + final latestOnSelected = useRef(onSelected); + latestOnSelected.value = onSelected; + + // Callback changes must not replace this effect: the previous hook's cleanup + // can clear the new handler on the same channel after a keyboard-driven rebuild. + useEffect(() { + if (viewId == null) return null; + final channel = MethodChannel('buzz/native_message_action_surface/$viewId'); + channel.setMethodCallHandler((call) async { + if (call.method != 'selected' || call.arguments is! Map) return; + final actionId = (call.arguments as Map)['id']; + if (actionId is String) latestOnSelected.value(actionId); + }); + return () => channel.setMethodCallHandler(null); + }, [viewId]); +} diff --git a/mobile/test/features/channels/native_message_action_selection_test.dart b/mobile/test/features/channels/native_message_action_selection_test.dart new file mode 100644 index 0000000000..204950f3ad --- /dev/null +++ b/mobile/test/features/channels/native_message_action_selection_test.dart @@ -0,0 +1,60 @@ +import 'package:buzz/features/channels/message_actions/native_message_action_selection.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_hooks/flutter_hooks.dart'; +import 'package:flutter_test/flutter_test.dart'; + +Future _select(WidgetTester tester, int viewId, String action) async { + tester.binding.channelBuffers.push( + 'buzz/native_message_action_surface/$viewId', + const StandardMethodCodec().encodeMethodCall( + MethodCall('selected', {'id': action}), + ), + (_) {}, + ); + await tester.pump(); +} + +Widget _menu(int? viewId, ValueChanged onSelected) => HookBuilder( + builder: (_) { + useNativeMessageActionSelection(viewId, onSelected); + return const SizedBox(); + }, +); + +void main() { + testWidgets('keeps native selections connected through callback rebuilds', ( + tester, + ) async { + final selections = []; + await tester.pumpWidget(_menu(null, (id) => selections.add('initial:$id'))); + await tester.pumpWidget(_menu(41, (id) => selections.add('created:$id'))); + await _select(tester, 41, 'copyText'); + expect(selections, ['created:copyText']); + + // Keyboard inset changes rebuild the popover with a fresh callback. + for (var frame = 0; frame < 5; frame++) { + await tester.pumpWidget( + _menu(41, (id) => selections.add('frame$frame:$id')), + ); + } + await _select(tester, 41, 'edit'); + expect(selections, ['created:copyText', 'frame4:edit']); + }); + + testWidgets('moves the handler to a new view and removes it on dismissal', ( + tester, + ) async { + final selections = []; + await tester.pumpWidget(_menu(51, selections.add)); + await _select(tester, 51, 'copyText'); + await tester.pumpWidget(_menu(52, selections.add)); + await _select(tester, 51, 'edit'); + await _select(tester, 52, 'delete'); + expect(selections, ['copyText', 'delete']); + + await tester.pumpWidget(const SizedBox()); + await _select(tester, 52, 'edit'); + expect(selections, ['copyText', 'delete']); + }); +}