mirror of
https://github.com/block/buzz.git
synced 2026-11-14 21:31:47 +00:00
fix(mobile): never pop Home when archiving from channel settings
Archive, Leave, Unarchive and Delete in channel settings waited for the relay and then popped whatever route was on top of the root navigator. If the user closed the settings sheet during that wait, the pop removed Home, leaving an empty navigator: a blank screen with no way back. Close only the sheet or page that offered the action, and only while it is still open. Make Home's route refuse pops, reporting them as errors, so no stray pop can empty the navigator again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
This commit is contained in:
@@ -35,6 +35,7 @@ import 'features/pairing/pairing_provider.dart';
|
||||
import 'features/channels/agent_activity/observer_subscription.dart';
|
||||
import 'features/channels/channel_detail_page.dart';
|
||||
import 'shared/widgets/app_route_observer.dart';
|
||||
import 'shared/widgets/root_page_route.dart';
|
||||
import 'features/profile/user_status_cache_provider.dart';
|
||||
import 'features/profile/settings_profile_header.dart';
|
||||
import 'features/profile/set_status_sheet.dart';
|
||||
@@ -423,6 +424,41 @@ class App extends HookConsumerWidget {
|
||||
});
|
||||
}
|
||||
|
||||
final home = authState.when(
|
||||
loading: () => const _SplashScreen(),
|
||||
// An internal build never offers pairing, even when auth state is
|
||||
// unreadable: pairing could make another relay the active community.
|
||||
error: (_, _) => OrganizationEnvironment.enabled
|
||||
? const OrganizationAppEntry()
|
||||
: const PairingPage(),
|
||||
data: (state) => switch (state.status) {
|
||||
AuthStatus.authenticated => HostedAppGate(
|
||||
authenticated: true,
|
||||
child: CoveredRouteKeyboardInsets(
|
||||
child: HomePage(
|
||||
onAgents: showAgents,
|
||||
defaultAgentBuilder: (context) => DefaultAgentLauncher(
|
||||
onChooseAgent: () => showAgents(context, createOnOpen: true),
|
||||
),
|
||||
mePageBuilder: (_, chat, workspace) => MePage(
|
||||
sliver: true,
|
||||
newChatAction: chat,
|
||||
newWorkspaceAction: workspace,
|
||||
),
|
||||
settingsPageBuilder: _buildSettingsPage,
|
||||
communityInvitePageBuilder: (_) => const CommunityInvitePage(),
|
||||
communityAppearancePageBuilder: (_) => const ThemePickerPage(),
|
||||
),
|
||||
),
|
||||
),
|
||||
_ => const HostedAppGate(
|
||||
authenticated: false,
|
||||
child: OnboardingAppEntry(),
|
||||
),
|
||||
},
|
||||
);
|
||||
final latestHome = useRef<Widget?>(null)..value = home;
|
||||
|
||||
return MaterialApp(
|
||||
navigatorKey: _mobileRootNavigatorKey,
|
||||
navigatorObservers: [appRouteObserver],
|
||||
@@ -466,39 +502,14 @@ class App extends HookConsumerWidget {
|
||||
),
|
||||
),
|
||||
},
|
||||
home: authState.when(
|
||||
loading: () => const _SplashScreen(),
|
||||
// An internal build never offers pairing, even when auth state is
|
||||
// unreadable: pairing could make another relay the active community.
|
||||
error: (_, _) => OrganizationEnvironment.enabled
|
||||
? const OrganizationAppEntry()
|
||||
: const PairingPage(),
|
||||
data: (state) => switch (state.status) {
|
||||
AuthStatus.authenticated => HostedAppGate(
|
||||
authenticated: true,
|
||||
child: CoveredRouteKeyboardInsets(
|
||||
child: HomePage(
|
||||
onAgents: showAgents,
|
||||
defaultAgentBuilder: (context) => DefaultAgentLauncher(
|
||||
onChooseAgent: () => showAgents(context, createOnOpen: true),
|
||||
),
|
||||
mePageBuilder: (_, chat, workspace) => MePage(
|
||||
sliver: true,
|
||||
newChatAction: chat,
|
||||
newWorkspaceAction: workspace,
|
||||
),
|
||||
settingsPageBuilder: _buildSettingsPage,
|
||||
communityInvitePageBuilder: (_) => const CommunityInvitePage(),
|
||||
communityAppearancePageBuilder: (_) => const ThemePickerPage(),
|
||||
),
|
||||
),
|
||||
),
|
||||
_ => const HostedAppGate(
|
||||
authenticated: false,
|
||||
child: OnboardingAppEntry(),
|
||||
),
|
||||
},
|
||||
),
|
||||
// A root route that refuses pops, so a stray pop can never leave an
|
||||
// empty navigator. It reads [home] when built, so it follows auth.
|
||||
onGenerateRoute: (settings) => settings.name == Navigator.defaultRouteName
|
||||
? RootPageRoute<void>(
|
||||
settings: settings,
|
||||
builder: (_) => latestHome.value!,
|
||||
)
|
||||
: null,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import 'package:buzz/shared/widgets/feedback_snackbar.dart';
|
||||
|
||||
import 'dart:async';
|
||||
|
||||
import 'package:flutter/material.dart';
|
||||
@@ -483,6 +484,10 @@ Future<void> _confirmAndRun(
|
||||
required Future<void> Function() action,
|
||||
}) async {
|
||||
final pageContext = Navigator.of(sheetContext, rootNavigator: true).context;
|
||||
// The action can take a relay round trip. Close only the sheet or page that
|
||||
// offered it, and only if it is still open: popping whatever is on top
|
||||
// would close Home if the user dismissed this route meanwhile.
|
||||
final ownRoute = ModalRoute.of(sheetContext);
|
||||
final confirmed = await showBuzzDialog<bool>(
|
||||
context: pageContext,
|
||||
builder: (dialogContext) => AlertDialog(
|
||||
@@ -503,7 +508,11 @@ Future<void> _confirmAndRun(
|
||||
if (confirmed != true) return;
|
||||
try {
|
||||
await action();
|
||||
if (pageContext.mounted) Navigator.of(pageContext).pop(true);
|
||||
if (ownRoute != null && ownRoute.isCurrent) {
|
||||
ownRoute.navigator!.pop(true);
|
||||
} else if (ownRoute != null && ownRoute.isActive) {
|
||||
ownRoute.navigator!.removeRoute(ownRoute, true);
|
||||
}
|
||||
} catch (error) {
|
||||
if (!pageContext.mounted) return;
|
||||
ScaffoldMessenger.of(pageContext).showFeedbackSnackBar(
|
||||
|
||||
28
mobile/lib/shared/widgets/root_page_route.dart
Normal file
28
mobile/lib/shared/widgets/root_page_route.dart
Normal file
@@ -0,0 +1,28 @@
|
||||
import 'package:flutter/material.dart';
|
||||
|
||||
/// The app's bottom route. It refuses to be popped: removing it would leave an
|
||||
/// empty navigator, a blank screen with nothing to return to.
|
||||
///
|
||||
/// A pop that reaches it is a caller bug (for example, popping "whatever is
|
||||
/// on top" after an await, when the route that meant to close is already
|
||||
/// gone). The pop is reported as an error, which fails tests, and ignored.
|
||||
class RootPageRoute<T> extends MaterialPageRoute<T> {
|
||||
RootPageRoute({required super.builder, super.settings});
|
||||
|
||||
// Not conditioned on [isFirst]: while a pop is in progress the route no
|
||||
// longer counts as present, so [isFirst] already reads false here.
|
||||
// Calling super would complete and remove this route, the pop it refuses.
|
||||
@override
|
||||
// ignore: must_call_super
|
||||
bool didPop(T? result) {
|
||||
FlutterError.reportError(
|
||||
FlutterErrorDetails(
|
||||
exception: StateError('Ignored a pop of the root route.'),
|
||||
stack: StackTrace.current,
|
||||
library: 'buzz navigation',
|
||||
context: ErrorDescription('while popping the root route'),
|
||||
),
|
||||
);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
@@ -823,6 +823,89 @@ void main() {
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('archive closes only its own settings sheet, never Home', (
|
||||
tester,
|
||||
) async {
|
||||
var archive = Completer<void>();
|
||||
await tester.pumpWidget(
|
||||
ProviderScope(
|
||||
overrides: [
|
||||
currentPubkeyProvider.overrideWith((ref) => _currentPubkey),
|
||||
channelMembersProvider('channel-id').overrideWith(
|
||||
(ref) async => [
|
||||
ChannelMember(
|
||||
pubkey: _currentPubkey,
|
||||
role: 'owner',
|
||||
joinedAt: DateTime(2025),
|
||||
),
|
||||
],
|
||||
),
|
||||
channelSectionsProvider.overrideWith(
|
||||
() => _FakeChannelSectionsNotifier(const ChannelSectionStore()),
|
||||
),
|
||||
agentOwnersProvider.overrideWithValue(
|
||||
const AsyncValue.data(<String, String>{}),
|
||||
),
|
||||
channelActionsProvider.overrideWith(
|
||||
(ref) => _FakeChannelActions(
|
||||
ref,
|
||||
onArchiveChannel: () => archive.future,
|
||||
),
|
||||
),
|
||||
],
|
||||
child: MaterialApp(
|
||||
theme: AppTheme.light(),
|
||||
home: Builder(
|
||||
builder: (context) => Scaffold(
|
||||
body: TextButton(
|
||||
onPressed: () => showChannelDetailsSheet(
|
||||
context: context,
|
||||
channel: _channel(),
|
||||
currentPubkey: _currentPubkey,
|
||||
onMemberTap: (_, _) {},
|
||||
personalChat: true,
|
||||
),
|
||||
child: const Text('Open settings'),
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
Future<void> confirmArchive() async {
|
||||
await tester.tap(find.text('Open settings'));
|
||||
await tester.pumpAndSettle();
|
||||
await tester.scrollUntilVisible(
|
||||
find.text('Archive channel'),
|
||||
200,
|
||||
scrollable: find.byType(Scrollable).last,
|
||||
);
|
||||
await tester.tap(find.text('Archive channel'));
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.text('Archive'));
|
||||
await tester.pumpAndSettle();
|
||||
}
|
||||
|
||||
// Closing the sheet while the archive is still in flight leaves Home in
|
||||
// place once the archive completes.
|
||||
await confirmArchive();
|
||||
expect(find.text('Archive channel'), findsOneWidget);
|
||||
await tester.tap(find.byTooltip('Close sheet'));
|
||||
await tester.pumpAndSettle();
|
||||
archive.complete();
|
||||
await tester.pumpAndSettle();
|
||||
expect(find.text('Open settings'), findsOneWidget);
|
||||
|
||||
// Left open, the sheet closes itself once the archive succeeds.
|
||||
archive = Completer<void>();
|
||||
await confirmArchive();
|
||||
archive.complete();
|
||||
await tester.pumpAndSettle();
|
||||
expect(find.text('Archive channel'), findsNothing);
|
||||
expect(find.text('Open settings'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('DM omits quick actions, then shows mute and copy rows', (
|
||||
tester,
|
||||
) async {
|
||||
@@ -883,16 +966,22 @@ class _FakeChannelActions extends ChannelActions {
|
||||
|
||||
final Future<void> Function(String)? onSetCanvas;
|
||||
|
||||
_FakeChannelActions(Ref ref, {this.onUpdateChannel, this.onSetCanvas})
|
||||
: super(
|
||||
ref: ref,
|
||||
session: ref.read(relaySessionProvider.notifier),
|
||||
signedEventRelay: SignedEventRelay(
|
||||
session: ref.read(relaySessionProvider.notifier),
|
||||
nsec: null,
|
||||
),
|
||||
currentPubkey: _currentPubkey,
|
||||
);
|
||||
final Future<void> Function()? onArchiveChannel;
|
||||
|
||||
_FakeChannelActions(
|
||||
Ref ref, {
|
||||
this.onUpdateChannel,
|
||||
this.onSetCanvas,
|
||||
this.onArchiveChannel,
|
||||
}) : super(
|
||||
ref: ref,
|
||||
session: ref.read(relaySessionProvider.notifier),
|
||||
signedEventRelay: SignedEventRelay(
|
||||
session: ref.read(relaySessionProvider.notifier),
|
||||
nsec: null,
|
||||
),
|
||||
currentPubkey: _currentPubkey,
|
||||
);
|
||||
|
||||
@override
|
||||
Future<void> updateChannel({
|
||||
@@ -916,6 +1005,11 @@ class _FakeChannelActions extends ChannelActions {
|
||||
@override
|
||||
Future<void> leaveChannel(String channelId) async {}
|
||||
|
||||
@override
|
||||
Future<void> archiveChannel(String channelId) async {
|
||||
await onArchiveChannel?.call();
|
||||
}
|
||||
|
||||
@override
|
||||
Future<void> unarchiveChannel(String channelId) async {
|
||||
unarchivedChannelId = channelId;
|
||||
|
||||
36
mobile/test/shared/widgets/root_page_route_test.dart
Normal file
36
mobile/test/shared/widgets/root_page_route_test.dart
Normal file
@@ -0,0 +1,36 @@
|
||||
import 'package:buzz/shared/widgets/root_page_route.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
|
||||
void main() {
|
||||
testWidgets('a stray pop never empties the navigator', (tester) async {
|
||||
final navigator = GlobalKey<NavigatorState>();
|
||||
await tester.pumpWidget(
|
||||
MaterialApp(
|
||||
navigatorKey: navigator,
|
||||
onGenerateRoute: (settings) => RootPageRoute<void>(
|
||||
settings: settings,
|
||||
builder: (_) => const Scaffold(body: Text('Home')),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
// Routes above Home still pop normally.
|
||||
navigator.currentState!.push(
|
||||
MaterialPageRoute<void>(
|
||||
builder: (_) => const Scaffold(body: Text('Chat')),
|
||||
),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
navigator.currentState!.pop();
|
||||
await tester.pumpAndSettle();
|
||||
expect(find.text('Chat'), findsNothing);
|
||||
expect(find.text('Home'), findsOneWidget);
|
||||
|
||||
// A pop that reaches Home is reported and ignored.
|
||||
navigator.currentState!.pop();
|
||||
await tester.pumpAndSettle();
|
||||
expect(tester.takeException(), isA<StateError>());
|
||||
expect(find.text('Home'), findsOneWidget);
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user