Restructure mobile Settings into a two-level category menu
On mobile, AccountView used to render all 9 Settings sections inline in one long, individually-collapsible column (_MobileList). Replace that with a native-style menu: the account card stays pinned at the top, and every other section becomes a NooSettingsRow in a NooGroupedList that pushes a dedicated screen (NooTopBar + NooTopBarBack) holding just that section's content. This removes the scroll-depth problem outright instead of working around it with per-section collapsing, so SettingsSection no longer needs NooGroupedList's collapsible mode on mobile. Desktop's 2-column grid is unchanged - it already shows every section at once. Audited every lib/widgets/settings/*.dart file for the reported "description and controls are flipped" row-layout bug: every row goes through NooSettingsRow directly, or - for the few hand-rolled rows (_SavedAccountRow, _ThemeRow/_BottomBarStyleRow, _CacheIntervalRow) - preserves its icon/description-then-control order (the stacked control-below-label shape used by _ThemeRow and _CacheIntervalRow is an intentional, spec'd variant, not a flip). Found no instance of the bug; no fix was needed. Updates DESIGN_SYSTEM.md's Settings recipe and styling.md's NooGroupedList notes to describe the new menu-then-pushed-screen pattern, and adds a widget test covering the category menu and the push/pop navigation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Claude Sonnet 5
@@ -0,0 +1,198 @@
|
||||
import 'dart:convert';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:google_fonts/google_fonts.dart';
|
||||
import 'package:provider/provider.dart';
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
import 'package:noo/models/saved_account.dart';
|
||||
import 'package:noo/providers/connectivity_controller.dart';
|
||||
import 'package:noo/providers/files_controller.dart';
|
||||
import 'package:noo/providers/session_controller.dart';
|
||||
import 'package:noo/providers/settings_controller.dart';
|
||||
import 'package:noo/providers/sync_status_controller.dart';
|
||||
import 'package:noo/theme/app_theme.dart';
|
||||
import 'package:noo/views/account_view.dart';
|
||||
import 'package:noo/widgets/noo/nav/noo_top_bar.dart';
|
||||
|
||||
/// Covers the mobile Settings navigation restructure: a top-level menu of
|
||||
/// category rows (`account_view.dart`'s `_MobileMenu`) that pushes a
|
||||
/// single-section screen per row, replacing the old design where every
|
||||
/// section rendered inline in one long scrolling column. This only covers
|
||||
/// the menu/push/back mechanics - each section's own content already has
|
||||
/// (or doesn't need) its own coverage elsewhere.
|
||||
void main() {
|
||||
const secureStorageChannel = MethodChannel(
|
||||
'plugins.it_nomads.com/flutter_secure_storage',
|
||||
);
|
||||
const connectivityChannel = MethodChannel(
|
||||
'dev.fluttercommunity.plus/connectivity',
|
||||
);
|
||||
const connectivityStatusChannel = EventChannel(
|
||||
'dev.fluttercommunity.plus/connectivity_status',
|
||||
);
|
||||
|
||||
const accountId = 'server_example_com__alice';
|
||||
const passwordKey = 'nc_app_password_$accountId';
|
||||
const password = 'app-password';
|
||||
|
||||
setUpAll(() {
|
||||
GoogleFonts.config.allowRuntimeFetching = false;
|
||||
TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
|
||||
.setMockMethodCallHandler(secureStorageChannel, (call) async {
|
||||
if (call.method == 'readAll') {
|
||||
return <String, String>{passwordKey: password};
|
||||
}
|
||||
if (call.method == 'read') {
|
||||
final key = (call.arguments as Map)['key'] as String?;
|
||||
return key == passwordKey ? password : null;
|
||||
}
|
||||
return null;
|
||||
});
|
||||
// Offline, like `session_controller_test.dart`'s setup: keeps
|
||||
// SessionController in a provisional login (no real HTTP calls) and
|
||||
// stops FilesController/SyncStatusController from starting their
|
||||
// network-fetch/periodic-refresh machinery, which would otherwise
|
||||
// leave timers pending forever in a test.
|
||||
TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
|
||||
.setMockMethodCallHandler(connectivityChannel, (call) async {
|
||||
if (call.method == 'check') return <String>['none'];
|
||||
return null;
|
||||
});
|
||||
TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
|
||||
.setMockStreamHandler(
|
||||
connectivityStatusChannel,
|
||||
MockStreamHandler.inline(onListen: (arguments, events) {}),
|
||||
);
|
||||
});
|
||||
|
||||
setUp(() {
|
||||
SharedPreferences.setMockInitialValues({
|
||||
'account_migration_v1_done': true,
|
||||
'accounts_list': jsonEncode([
|
||||
const SavedAccount(
|
||||
id: accountId,
|
||||
serverUrl: 'https://server.example.com',
|
||||
username: 'alice',
|
||||
).toJson(),
|
||||
]),
|
||||
'active_account_id': accountId,
|
||||
});
|
||||
});
|
||||
|
||||
Future<void> pumpSettings(WidgetTester tester) async {
|
||||
await tester.binding.setSurfaceSize(const Size(400, 800));
|
||||
addTearDown(() => tester.binding.setSurfaceSize(null));
|
||||
|
||||
await tester.pumpWidget(
|
||||
MultiProvider(
|
||||
providers: [
|
||||
ChangeNotifierProvider(create: (_) => ConnectivityController()),
|
||||
ChangeNotifierProvider(
|
||||
create: (context) => SessionController(context.read()),
|
||||
),
|
||||
ChangeNotifierProvider(create: (_) => SettingsController()),
|
||||
ChangeNotifierProvider(
|
||||
create: (context) => FilesController(context.read()),
|
||||
),
|
||||
ChangeNotifierProvider(
|
||||
create: (context) =>
|
||||
SyncStatusController(context.read(), context.read()),
|
||||
),
|
||||
],
|
||||
child: MaterialApp(
|
||||
theme: AppTheme.light(AppTheme.defaultAccent, useDynamicColor: false),
|
||||
home: const AccountView(),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
// Not `pumpAndSettle`: once the saved account is ready,
|
||||
// FilesController/SyncStatusController start their own periodic
|
||||
// refresh timers, which `pumpAndSettle` would spin on forever. A
|
||||
// bounded number of small pumps is enough to flush the async gaps in
|
||||
// SessionController's prefs restore and the dependent controllers'
|
||||
// one-shot account-ready reactions (mirrors
|
||||
// `session_controller_test.dart`'s `pumpUntil` loop).
|
||||
for (var i = 0; i < 30; i++) {
|
||||
await tester.pump(const Duration(milliseconds: 10));
|
||||
}
|
||||
}
|
||||
|
||||
/// Advances exactly far enough to finish a push/pop transition
|
||||
/// (`MaterialPageRoute`'s default is 300ms) without risking
|
||||
/// `pumpAndSettle` picking up a pending periodic timer.
|
||||
Future<void> settleNav(WidgetTester tester) async {
|
||||
await tester.pump();
|
||||
for (var i = 0; i < 10; i++) {
|
||||
await tester.pump(const Duration(milliseconds: 50));
|
||||
}
|
||||
}
|
||||
|
||||
const categoryTitles = [
|
||||
'Accounts',
|
||||
'Security',
|
||||
'File sync',
|
||||
'Files cache',
|
||||
'Appearance',
|
||||
'Tabs',
|
||||
'Action bar',
|
||||
'Swipe on a file',
|
||||
];
|
||||
|
||||
testWidgets(
|
||||
'shows a top-level menu of category rows, not every section inline',
|
||||
(tester) async {
|
||||
await pumpSettings(tester);
|
||||
|
||||
// The account card (pinned, not a category row of its own) ...
|
||||
expect(find.text('alice'), findsOneWidget);
|
||||
// ... plus exactly one row per section.
|
||||
for (final title in categoryTitles) {
|
||||
expect(find.text(title), findsOneWidget);
|
||||
}
|
||||
// A section's own content isn't rendered until its row is tapped -
|
||||
// this is a menu, not the old all-sections-inline column.
|
||||
expect(
|
||||
find.text("Require this device's PIN or biometric to open Noo"),
|
||||
findsNothing,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
testWidgets(
|
||||
'tapping a category row pushes just that section, with a way back',
|
||||
(tester) async {
|
||||
await pumpSettings(tester);
|
||||
|
||||
await tester.tap(find.text('Security'));
|
||||
await settleNav(tester);
|
||||
|
||||
// Landed on a separate, pushed Security screen: its own content -
|
||||
// not shown anywhere on the top-level menu - is now visible. (The
|
||||
// menu screen below it may stay mounted per `PageRoute.maintainState`,
|
||||
// so this checks for the pushed screen's content rather than the
|
||||
// menu's absence.)
|
||||
expect(
|
||||
find.text("Require this device's PIN or biometric to open Noo"),
|
||||
findsOneWidget,
|
||||
);
|
||||
// At least one `NooTopBarBack` now leads back - the pushed screen's
|
||||
// own, on top of `AccountView`'s own (for returning to the shell).
|
||||
expect(find.byType(NooTopBarBack), findsAtLeastNWidgets(1));
|
||||
|
||||
// Tapping the topmost back button pops the pushed screen back off,
|
||||
// taking its content with it - unlike the underlying menu, a popped
|
||||
// route is actually removed, so this absence check is meaningful.
|
||||
await tester.tap(find.byType(NooTopBarBack).last);
|
||||
await settleNav(tester);
|
||||
|
||||
expect(
|
||||
find.text("Require this device's PIN or biometric to open Noo"),
|
||||
findsNothing,
|
||||
);
|
||||
expect(find.text('Security'), findsOneWidget);
|
||||
expect(find.text('alice'), findsOneWidget);
|
||||
},
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user