From a8b14b8744d3898a644da873b9c2f1e54ad1abfc Mon Sep 17 00:00:00 2001 From: ayushya Date: Wed, 30 Sep 2026 16:15:32 -0400 Subject: [PATCH] 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 --- .../context/design-system/DESIGN_SYSTEM.md | 26 ++- .claude/context/styling.md | 8 +- lib/views/account_view.dart | 182 ++++++++++++++-- lib/widgets/settings/settings_section.dart | 14 +- test/views/account_view_test.dart | 198 ++++++++++++++++++ 5 files changed, 391 insertions(+), 37 deletions(-) create mode 100644 test/views/account_view_test.dart diff --git a/.claude/context/design-system/DESIGN_SYSTEM.md b/.claude/context/design-system/DESIGN_SYSTEM.md index 3ddbc59..7a4c738 100644 --- a/.claude/context/design-system/DESIGN_SYSTEM.md +++ b/.claude/context/design-system/DESIGN_SYSTEM.md @@ -358,15 +358,23 @@ Sidebar items are 38px tall with radius 12, an 18px icon and a 14/500 label. The slots, the rest sit behind "More". See §2 "Selection action bar". 9. Swipe on a file - Mobile uses one column of grouped lists, each individually collapsible - (tap its label, expanded by default - `NooGroupedList`'s `collapsible` - param) so the full list can be collapsed down instead of needing a - separate way to navigate it; an earlier version had a trailing jump rail - (one small icon per section, pinned where the scrollbar would sit) - instead, dropped for adding a second, redundant navigation method without - shortening the page. Desktop uses a 2-column grid of cards with a 1px - line and radius 20, wide enough to see most sections without scrolling, - so it gets neither. + Mobile is a two-level menu, the way native iOS/Android Settings apps + work: the account card stays pinned at the top of a single top-level + list, and every other section (2-9 above) becomes one tappable + `NooSettingsRow` - icon, title, chevron - in a `NooGroupedList` below it. + Tapping a row pushes a new screen (`NooTopBar`/`NooTopBarBack`) holding + just that section's own content full-screen, so no page is ever more + than one category deep and no section needs to be individually + collapsible any more. Two earlier designs were tried and dropped: a + trailing jump rail (one small icon per section, pinned where the + scrollbar would sit), and - after that - one long column of every + section inline, each individually collapsible (`NooGroupedList`'s + `collapsible` param) so the page could at least be collapsed down. Both + scrolled the *same* page to or past an anchor; a genuinely separate + pushed screen per category removes the scroll-depth problem outright + instead of just working around it. Desktop is unchanged: a 2-column grid + of cards with a 1px line and radius 20, wide enough to see most sections + without scrolling, so it gets neither a menu nor collapsing. - **Share sheet / dialog:** sections in this order: 1. Header: file tile, name, size · folder, and close. 2. **Share with people:** an input ("Name, email or group"), then the people with access. The owner comes first; the others each have a permission pill ("Can edit ▾"). diff --git a/.claude/context/styling.md b/.claude/context/styling.md index 0220b14..316ccf1 100644 --- a/.claude/context/styling.md +++ b/.claude/context/styling.md @@ -90,8 +90,12 @@ Gotchas: - `NooGroupedList` draws dividers by showing `line` through 1px gaps, so each child must paint its own surface (`NooSettingsRow` and `NooTabOrderRow` do). Its `collapsible`/`initiallyExpanded` params (off by default) make - `label` a tap target that shows/hides the card - `SettingsSection` is the - only caller that opts in, for Settings' mobile sections. + `label` a tap target that shows/hides the card - no current caller opts + in (Settings' mobile sections used to, when every section rendered + inline in one long column; now each section is its own pushed screen - + see `account_view.dart`'s doc comment - so there's nothing left to + collapse). The params stay on the component itself since it's otherwise + generic. - `NooSwipeAction` only reveals its action. The user has to tap the block to trigger it; a full swipe never deletes. - Window chrome (macOS traffic lights, the Windows 40px title bar) isn't diff --git a/lib/views/account_view.dart b/lib/views/account_view.dart index d0c3def..cdb1a0e 100644 --- a/lib/views/account_view.dart +++ b/lib/views/account_view.dart @@ -1,5 +1,8 @@ import 'package:flutter/material.dart'; +import 'package:lucide_icons_flutter/lucide_icons.dart'; import '../theme/design_tokens.dart'; +import '../widgets/noo/lists/noo_grouped_list.dart'; +import '../widgets/noo/lists/noo_settings_row.dart'; import '../widgets/noo/nav/noo_top_bar.dart'; import '../widgets/noo/nav/noo_toolbar.dart'; import '../widgets/noo/noo_layout.dart'; @@ -15,18 +18,32 @@ import '../widgets/settings/settings_tabs.dart'; /// Settings, pushed on top of the shell (DESIGN_SYSTEM.md 4's 9-section /// order: account card, accounts, security, file sync, files cache, -/// appearance, tabs, action bar, swipe on a file). One column of -/// [SettingsSection]s (and [SettingsActionBarSection], which has no option -/// rows of its own to put in one - just the reorder list) on mobile, each -/// individually collapsible (expanded by default, tap its label to -/// collapse - see `SettingsSection`/`NooGroupedList`'s `collapsible` param) -/// so a long Settings screen can be collapsed down rather than needing a -/// separate jump rail (an earlier version had one; it added a second, -/// redundant way to navigate on top of plain scrolling and still didn't -/// shorten the page). Desktop uses a 2-column grid of cards instead, wide -/// enough to see most sections without scrolling, so it gets neither. See -/// each `widgets/settings/*.dart` file for a section's own content and any -/// setting that had to be slotted in or grouped under "Advanced appearance". +/// appearance, tabs, action bar, swipe on a file). +/// +/// Desktop is unchanged: a 2-column grid of cards ([_DesktopGrid]) wide +/// enough to see every section at once, so it has no scroll-depth problem +/// and needs no menu. +/// +/// Mobile is a two-level menu, the way native iOS/Android Settings apps +/// work: [SettingsAccountCard] (the account summary, not a settings picker) +/// stays pinned at the top of a single top-level list ([_MobileMenu]), and +/// every other section becomes one tappable [NooSettingsRow] - icon, title, +/// chevron - in a [NooGroupedList] below it. Tapping a row pushes a new +/// [_SettingsCategoryScreen] with its own [NooTopBar]/[NooTopBarBack], +/// containing just that section's content full-screen. This replaces an +/// earlier design where every section rendered inline in one long +/// collapsible-sections column (`_MobileList`, since removed) - and before +/// that, a trailing jump rail (an even earlier version) that scrolled that +/// *same* page to an anchor. Both were rejected: the jump rail added a +/// second, redundant way to navigate on top of plain scrolling without +/// shortening the page, and the collapsible-sections column still left a +/// long page to scroll past even collapsed. A genuinely separate pushed +/// screen per category removes the scroll-depth problem outright, so +/// neither a jump rail nor per-section collapsing is needed any more - see +/// [SettingsSection]'s doc comment for how that reflects in its mobile +/// layout. See each `widgets/settings/*.dart` file for a section's own +/// content and any setting that had to be slotted in or grouped under +/// "Advanced appearance". class AccountView extends StatelessWidget { const AccountView({super.key}); @@ -42,6 +59,49 @@ class AccountView extends StatelessWidget { SettingsSwipeSection(), ]; + static final _categories = <_SettingsCategory>[ + _SettingsCategory( + title: 'Accounts', + icon: LucideIcons.users, + builder: (_) => const SettingsAccountsSection(), + ), + _SettingsCategory( + title: 'Security', + icon: LucideIcons.lock, + builder: (_) => const SettingsSecuritySection(), + ), + _SettingsCategory( + title: 'File sync', + icon: LucideIcons.cloud, + builder: (_) => const SettingsFileSyncSection(), + ), + _SettingsCategory( + title: 'Files cache', + icon: LucideIcons.database, + builder: (_) => const SettingsFilesCacheSection(), + ), + _SettingsCategory( + title: 'Appearance', + icon: LucideIcons.sunMoon, + builder: (_) => const SettingsAppearanceSection(), + ), + _SettingsCategory( + title: 'Tabs', + icon: LucideIcons.layoutGrid, + builder: (_) => const SettingsTabsSection(), + ), + _SettingsCategory( + title: 'Action bar', + icon: LucideIcons.slidersHorizontal, + builder: (_) => const SettingsActionBarSection(), + ), + _SettingsCategory( + title: 'Swipe on a file', + icon: LucideIcons.chevronsLeftRight, + builder: (_) => const SettingsSwipeSection(), + ), + ]; + @override Widget build(BuildContext context) { final colors = context.nooColors; @@ -60,18 +120,49 @@ class AccountView extends StatelessWidget { top: false, child: desktop ? _DesktopGrid(sections: _sections) - : _MobileList(sections: _sections), + : _MobileMenu(categories: _categories), ), ); } } -class _MobileList extends StatelessWidget { - final List sections; - const _MobileList({required this.sections}); +/// One row of [AccountView]'s mobile top-level menu: a title, a leading +/// icon (reused from that section's own first/most-representative row, so +/// the menu icon and the content the user lands on agree), and a builder +/// for the section content shown on [_SettingsCategoryScreen]. +class _SettingsCategory { + final String title; + final IconData icon; + final WidgetBuilder builder; + + _SettingsCategory({ + required this.title, + required this.icon, + required this.builder, + }); +} + +/// The mobile top-level Settings list: [SettingsAccountCard] pinned above a +/// single [NooGroupedList] of category rows, one per [AccountView._categories] +/// entry - the menu half of the menu-then-pushed-screen pattern described on +/// [AccountView]'s own doc comment. +class _MobileMenu extends StatelessWidget { + final List<_SettingsCategory> categories; + const _MobileMenu({required this.categories}); + + void _open(BuildContext context, _SettingsCategory category) { + Navigator.push( + context, + MaterialPageRoute( + builder: (_) => _SettingsCategoryScreen(category: category), + ), + ); + } @override Widget build(BuildContext context) { + final colors = context.nooColors; + return ListView( padding: const EdgeInsets.fromLTRB( NooSpace.sm, @@ -81,15 +172,66 @@ class _MobileList extends StatelessWidget { ), physics: const BouncingScrollPhysics(), children: [ - for (final section in sections) ...[ - section, - const SizedBox(height: NooSpace.xl), - ], + const SettingsAccountCard(), + const SizedBox(height: NooSpace.xl), + NooGroupedList( + children: [ + for (final category in categories) + NooSettingsRow( + icon: category.icon, + label: Text(category.title), + trailing: Icon( + LucideIcons.chevronRight, + size: 18, + color: colors.fg3, + ), + onTap: () => _open(context, category), + ), + ], + ), ], ); } } +/// A pushed, single-category Settings screen: [NooTopBar] titled with the +/// category, a back button, and just that section's own content - the +/// pushed half of [AccountView]'s mobile menu-then-screen pattern. Always +/// built in a mobile-width context (desktop never opens this screen; it +/// shows every section inline in its own grid instead), so the section +/// widgets inside render their normal mobile layout unchanged. +class _SettingsCategoryScreen extends StatelessWidget { + final _SettingsCategory category; + const _SettingsCategoryScreen({required this.category}); + + @override + Widget build(BuildContext context) { + final colors = context.nooColors; + + return Scaffold( + backgroundColor: colors.bg, + appBar: NooTopBar( + style: NooLayout.navStyle(context), + title: category.title, + leading: const NooTopBarBack(), + ), + body: SafeArea( + top: false, + child: ListView( + padding: const EdgeInsets.fromLTRB( + NooSpace.sm, + NooSpace.sm, + NooSpace.sm, + NooSpace.xxl, + ), + physics: const BouncingScrollPhysics(), + children: [category.builder(context)], + ), + ), + ); + } +} + /// Splits the sections between two columns rather than a strict grid, since /// each card's content height varies a lot (the tab reorder list and the /// accounts list can both run much taller than, say, Security) - a fixed diff --git a/lib/widgets/settings/settings_section.dart b/lib/widgets/settings/settings_section.dart index 7ceae70..d2a8407 100644 --- a/lib/widgets/settings/settings_section.dart +++ b/lib/widgets/settings/settings_section.dart @@ -5,7 +5,7 @@ import '../noo/noo_layout.dart'; import '../noo/overlays/noo_dialog.dart'; import '../noo/overlays/noo_sheet.dart'; -/// One block of Settings (DESIGN_SYSTEM.md 4's 8-part order): a +/// One block of Settings (DESIGN_SYSTEM.md 4's 9-part order): a /// [NooGroupedList] on mobile (label above a radius-20 card), or a titled, /// bordered radius-20 card holding a flat row group on desktop /// ("Mobile uses one column of grouped lists. Desktop uses a 2-column grid @@ -31,11 +31,13 @@ class SettingsSection extends StatelessWidget { return NooGroupedList( label: title, footer: subtitle != null ? Text(subtitle!) : null, - // Every mobile Settings section is individually collapsible, - // expanded by default - replaces the old trailing jump rail (see - // `account_view.dart`'s doc comment) as the way to navigate a long - // Settings screen quickly. - collapsible: true, + // Not collapsible: each mobile Settings section now renders on its + // own pushed screen (see `account_view.dart`'s doc comment for the + // menu-then-pushed-screen pattern), so there's no long single-scroll + // page left to collapse sections *within* - an earlier design had + // every section inline in one column and made them individually + // collapsible for exactly that reason; that's gone now that each + // one is already isolated on its own screen. children: children, ); } diff --git a/test/views/account_view_test.dart b/test/views/account_view_test.dart new file mode 100644 index 0000000..fe3d76b --- /dev/null +++ b/test/views/account_view_test.dart @@ -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 {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 ['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 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 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); + }, + ); +}