From b73f451fff7f9975711535213acde612a38dc7db Mon Sep 17 00:00:00 2001 From: ayushya Date: Tue, 29 Sep 2026 10:45:05 -0400 Subject: [PATCH] Fix pull-to-refresh, header tint, sheet controls, and Shares tap - Every tab's RefreshIndicator+CustomScrollView was missing an explicit AlwaysScrollableScrollPhysics, so pull-to-refresh silently couldn't be triggered once the list was short enough to fit the viewport (empty, or a single item) - the platform default physics only allows the overscroll drag RefreshIndicator listens for once content overflows. - StickyHeaderDelegate (Files'/Photos'/Favorites' pinned controls row) painted its background from Theme.of(context).colorScheme.surface - Flutter's own Material 3 scheme, reseeded by the user's accent color choice - instead of context.nooColors.bg like the rest of the screen, showing as a stray colored cast behind the row. - NooSegmentedControl gains onSurface: true wherever one sits inside a sheet (Files'/Photos' sort and type-filter controls) - it defaulted to the same surface fill as the sheet itself, so the control had no visible boundary showing it as a grouped unit. - Shares tab: removed the per-row "..." overflow menu on every scope (With you/By you/Links); tapping a row now opens the full Share sheet for that item instead, fetched via the new FilesController.fetchItemAtPath (a share only carries enough metadata for its own row, not what the sheet's header needs). - fetchShares() now reuses _shareFromJson instead of its own thinner inline parser, so url/token/permissions/expireDate are actually populated - a public-link share could never show "Copy link" before. Co-Authored-By: Claude Sonnet 5 --- .../context/design-system/DESIGN_SYSTEM.md | 9 +- .claude/context/styling.md | 18 +++ lib/providers/files_controller.dart | 25 +++- lib/views/activity_view.dart | 10 +- lib/views/favorites_view.dart | 3 + lib/views/files_view.dart | 5 + lib/views/move_copy_destination_picker.dart | 4 + lib/views/photos_view.dart | 5 + lib/views/recent_view.dart | 12 +- lib/views/share_upload_view.dart | 4 + lib/views/shares_view.dart | 134 ++++-------------- lib/views/trash_view.dart | 3 + lib/widgets/files_controls_row.dart | 2 + lib/widgets/sticky_header_delegate.dart | 19 ++- 14 files changed, 140 insertions(+), 113 deletions(-) diff --git a/.claude/context/design-system/DESIGN_SYSTEM.md b/.claude/context/design-system/DESIGN_SYSTEM.md index 48bfceb..276c04b 100644 --- a/.claude/context/design-system/DESIGN_SYSTEM.md +++ b/.claude/context/design-system/DESIGN_SYSTEM.md @@ -330,7 +330,14 @@ Sidebar items are 38px tall with radius 12, an 18px icon and a 14/500 label. The photo grid grouped by month. - **Favorites:** a file list with a star as the trailing icon. - **Offline:** a summary card (or 4 stat cards on desktop), then a list whose rows show sync status icons and status meta. -- **Shares:** a segmented control (With you, By you, Links), then a list. The meta reads "Owner · Permission". +- **Shares:** a segmented control (With you, By you, Links), then a list. The + meta reads "Owner · Permission". No per-row overflow menu on any of the + three scopes - tapping a row (file or folder) opens the full Share sheet + for that item (fetched fresh via `FilesController.fetchItemAtPath`, since + a share only carries enough metadata for its own row), the same sheet + Files/Photos open from their own Share action - that's already where + copying a link or removing access lives, so a second, row-local menu here + was redundant. - **Recent:** a list grouped into Today, Yesterday and This week. The meta is the action plus the location. - **Activity:** the feed grouped by day. Desktop limits it to 760px wide. - **Trash:** a retention banner, then a list. Mobile rows get a restore icon; desktop rows get a tonal "Restore" pill. diff --git a/.claude/context/styling.md b/.claude/context/styling.md index c40a2bf..dc75393 100644 --- a/.claude/context/styling.md +++ b/.claude/context/styling.md @@ -96,6 +96,24 @@ Gotchas: trigger it; a full swipe never deletes. - Window chrome (macOS traffic lights, the Windows 40px title bar) isn't built yet. `NooSidebar.windowControls` is the slot for it. +- Always read colors through `context.nooColors`, never + `Theme.of(context).colorScheme` - the latter is Flutter's own Material 3 + scheme, reseeded by the user's accent color choice (`AppTheme.light`/ + `.dark`), so anything painted from it carries a faint hue of whatever + accent is picked instead of the app's neutral palette. `sticky_header_delegate.dart` + shipped with this bug once already (a pinned controls-row header tinted + by the accent instead of matching its screen's plain `colors.bg`). +- `NooSegmentedControl` defaults to `onSurface: false` (`colors.surface` + fill) - right when it's sitting directly on `colors.bg` (List/Grid + toggle, Shares' scope switcher), but pass `onSurface: true` for one + placed inside a sheet or dialog (already `colors.surface`), or its pill + track blends invisibly into the sheet instead of reading as a grouped + control (Files'/Photos' sort and type-filter sheets do this). +- Any `RefreshIndicator` needs `physics: const AlwaysScrollableScrollPhysics()` + on its scrollable child, or pull-to-refresh silently can't be triggered + once the list is short enough to fit the viewport (empty, or one item) - + every tab's `CustomScrollView` sets this explicitly for exactly that + reason. ## Reusable chrome diff --git a/lib/providers/files_controller.dart b/lib/providers/files_controller.dart index 71d2f78..af0013d 100644 --- a/lib/providers/files_controller.dart +++ b/lib/providers/files_controller.dart @@ -266,7 +266,8 @@ class FilesController extends ChangeNotifier /// sort, cache policy) - hooked to `addAccountReadyListener` as well as /// activation so the Offline tab, which reads these same prefs, respects /// them even on a provisional/offline login where activation never fires. - Future _restoreDisplayPrefs() => _prefsRestore ??= _doRestoreDisplayPrefs(); + Future _restoreDisplayPrefs() => + _prefsRestore ??= _doRestoreDisplayPrefs(); /// Completes once this account's cache policy/display prefs have been /// loaded from storage - lets `SyncStatusController` schedule background @@ -334,7 +335,9 @@ class FilesController extends ChangeNotifier defaultCacheIntervalMinutes; notifyListeners(); } catch (e) { - debugPrint('[FilesController] Account-activation prefs restore failed: $e'); + debugPrint( + '[FilesController] Account-activation prefs restore failed: $e', + ); } } } @@ -401,7 +404,9 @@ class FilesController extends ChangeNotifier notifyListeners(); } - debugPrint('[FilesController] Refreshing data for path: $_currentFolderPath'); + debugPrint( + '[FilesController] Refreshing data for path: $_currentFolderPath', + ); try { final items = await service.fetchDirectory(_currentFolderPath); @@ -549,6 +554,20 @@ class FilesController extends ChangeNotifier return session.service?.fetchDirectory(path) ?? Future.value([]); } + /// Finds the single item at [path] by listing its parent folder and + /// matching on the exact path - there's no WebDAV call here for stat'ing + /// one path directly outside a directory PROPFIND. Used by the Shares tab + /// to open the full Share sheet for a [NextcloudShare], which only carries + /// enough metadata for its own row, not the size/dates `ShareSheet`'s + /// header needs. + Future fetchItemAtPath(String path) async { + final normalized = path.startsWith('/') ? path : '/$path'; + final lastSlash = normalized.lastIndexOf('/'); + final parent = lastSlash <= 0 ? '/' : normalized.substring(0, lastSlash); + final items = await fetchFolderListing(parent); + return items.where((i) => i.path == normalized).firstOrNull; + } + void invalidateCache() => _directoryCache.clear(); /// Persists a per-account browsing pref under its `acct__`-namespaced diff --git a/lib/views/activity_view.dart b/lib/views/activity_view.dart index 8121ee0..90f693e 100644 --- a/lib/views/activity_view.dart +++ b/lib/views/activity_view.dart @@ -73,9 +73,12 @@ class ActivityView extends StatelessWidget { ? act.subject : null, time: DateFormat.jm().format(act.timestamp), - avatarColor: NooPhotoTile.paletteColor(act.author.hashCode), + avatarColor: NooPhotoTile.paletteColor( + act.author.hashCode, + ), currentUser: - act.author.toLowerCase() == session.username.toLowerCase(), + act.author.toLowerCase() == + session.username.toLowerCase(), ), ), ], @@ -94,6 +97,9 @@ class ActivityView extends StatelessWidget { final scrollView = CustomScrollView( controller: scrollController, + // See files_view.dart's identical fix - without this, pull-to- + // refresh can't be triggered on an empty or single-item feed. + physics: const AlwaysScrollableScrollPhysics(), slivers: contentSlivers, ); diff --git a/lib/views/favorites_view.dart b/lib/views/favorites_view.dart index e27eab2..cae784c 100644 --- a/lib/views/favorites_view.dart +++ b/lib/views/favorites_view.dart @@ -654,6 +654,9 @@ class _FavoritesViewState extends State { onRefresh: favoritesController.fetchAll, child: CustomScrollView( controller: widget.scrollController, + // See files_view.dart's identical fix - without this, pull-to- + // refresh can't be triggered on an empty or single-item list. + physics: const AlwaysScrollableScrollPhysics(), slivers: contentSlivers, ), ), diff --git a/lib/views/files_view.dart b/lib/views/files_view.dart index 4ef84e9..a64f1bc 100644 --- a/lib/views/files_view.dart +++ b/lib/views/files_view.dart @@ -838,6 +838,11 @@ class _FilesViewState extends State { }, child: CustomScrollView( controller: widget.scrollController, + // Pull-to-refresh needs a scroll physics that allows dragging + // past the edge even when content doesn't fill the viewport - + // an empty or single-item list otherwise can't be pulled at all + // under the platform default physics. + physics: const AlwaysScrollableScrollPhysics(), slivers: contentSlivers, ), ), diff --git a/lib/views/move_copy_destination_picker.dart b/lib/views/move_copy_destination_picker.dart index 78fd460..b07022b 100644 --- a/lib/views/move_copy_destination_picker.dart +++ b/lib/views/move_copy_destination_picker.dart @@ -359,6 +359,10 @@ class _MoveCopyDestinationPickerState extends State { onRefresh: () => _fetch(_currentPath), child: CustomScrollView( controller: _scrollController, + // See files_view.dart's identical fix - without this, pull- + // to-refresh can't be triggered on an empty or single-folder + // listing. + physics: const AlwaysScrollableScrollPhysics(), slivers: slivers, ), ), diff --git a/lib/views/photos_view.dart b/lib/views/photos_view.dart index 2a3122a..4fa9152 100644 --- a/lib/views/photos_view.dart +++ b/lib/views/photos_view.dart @@ -364,6 +364,9 @@ class _PhotosViewState extends State { onRefresh: photosController.fetchAllMedia, child: CustomScrollView( controller: widget.scrollController, + // See files_view.dart's identical fix - without this, pull-to- + // refresh can't be triggered on an empty or single-item list. + physics: const AlwaysScrollableScrollPhysics(), slivers: contentSlivers, ), ), @@ -432,6 +435,7 @@ class _PhotosViewState extends State { children: [ NooSegmentedControl( fill: true, + onSurface: true, value: photos.sortAscending, onChanged: (asc) { if (asc != photos.sortAscending) photos.toggleSortOrder(); @@ -501,6 +505,7 @@ class _PhotosViewState extends State { // the two screens' filter sheets look and behave the same. NooSegmentedControl<_PhotoTypeFilter>( fill: true, + onSurface: true, labelOnlySelected: true, value: _typeFilter, onChanged: (filter) => setState(() { diff --git a/lib/views/recent_view.dart b/lib/views/recent_view.dart index e8484f6..02af7e3 100644 --- a/lib/views/recent_view.dart +++ b/lib/views/recent_view.dart @@ -85,6 +85,9 @@ class _RecentViewState extends State { onRefresh: recent.fetchAll, child: CustomScrollView( controller: widget.scrollController, + // See files_view.dart's identical fix - without this, pull-to- + // refresh can't be triggered on an empty or single-item list. + physics: const AlwaysScrollableScrollPhysics(), slivers: contentSlivers, ), ), @@ -101,7 +104,9 @@ class _RecentViewState extends State { delegate: SliverChildBuilderDelegate((context, index) { final group = groups[index]; return Padding( - padding: EdgeInsets.only(bottom: index == groups.length - 1 ? 0 : 18), + padding: EdgeInsets.only( + bottom: index == groups.length - 1 ? 0 : 18, + ), child: NooGroupedList( label: group.label, children: [ @@ -132,7 +137,10 @@ class _RecentViewState extends State { crossAxisAlignment: CrossAxisAlignment.start, children: [ TabGroupLabel(group.label), - const NooFileTableHeader(col2Label: 'Modified', col3Label: 'Location'), + const NooFileTableHeader( + col2Label: 'Modified', + col3Label: 'Location', + ), for (final item in group.items) _buildDesktopRow(context, item), ], ), diff --git a/lib/views/share_upload_view.dart b/lib/views/share_upload_view.dart index 35e6178..c7181e2 100644 --- a/lib/views/share_upload_view.dart +++ b/lib/views/share_upload_view.dart @@ -278,6 +278,10 @@ class _ShareUploadViewState extends State { onRefresh: files.refreshData, child: CustomScrollView( controller: _scrollController, + // See files_view.dart's identical fix - without this, pull-to- + // refresh can't be triggered on an empty or single-folder + // listing. + physics: const AlwaysScrollableScrollPhysics(), slivers: slivers, ), ), diff --git a/lib/views/shares_view.dart b/lib/views/shares_view.dart index acb342a..e833975 100644 --- a/lib/views/shares_view.dart +++ b/lib/views/shares_view.dart @@ -1,21 +1,16 @@ import 'package:flutter/material.dart'; -import 'package:flutter/services.dart'; import 'package:provider/provider.dart'; import 'package:lucide_icons_flutter/lucide_icons.dart'; -import '../models/app_tab.dart'; import '../models/nextcloud_share.dart'; import '../providers/files_controller.dart'; -import '../providers/settings_controller.dart'; import '../providers/shares_controller.dart'; import '../theme/design_tokens.dart'; import '../widgets/noo/core/noo_segmented_control.dart'; import '../widgets/noo/files/noo_file_kind.dart'; import '../widgets/noo/files/noo_file_row.dart'; import '../widgets/noo/files/noo_file_table.dart'; -import '../widgets/noo/lists/noo_grouped_list.dart'; -import '../widgets/noo/lists/noo_settings_row.dart'; import '../widgets/noo/noo_layout.dart'; -import '../widgets/noo/overlays/noo_sheet.dart'; +import '../widgets/share_sheet.dart'; import '../widgets/tabs/tab_state_slivers.dart'; /// Which slice of [SharesController.shares] is on screen - `With you` maps @@ -136,6 +131,9 @@ class _SharesViewState extends State { onRefresh: sharesController.fetchAll, child: CustomScrollView( controller: widget.scrollController, + // See files_view.dart's identical fix - without this, pull-to- + // refresh can't be triggered on an empty or single-item list. + physics: const AlwaysScrollableScrollPhysics(), slivers: contentSlivers, ), ), @@ -197,9 +195,8 @@ class _SharesViewState extends State { meta: '${share.ownerDisplayName} · ${_permissionLabel(share.permissions)}', iosStyle: NooLayout.iosStyle(context), - onTap: share.isFolder ? () => _openFolder(context, share) : null, + onTap: () => _openShareSheet(context, share), trailing: Icon(_shareTypeIcon(share.shareType), size: 16), - onMore: () => _showShareActions(context, share), ); final isFirst = index == 0; final isLast = index == shares.length - 1; @@ -247,112 +244,41 @@ class _SharesViewState extends State { name: share.name, col2: share.ownerDisplayName, col3: _permissionLabel(share.permissions), - onTap: share.isFolder ? () => _openFolder(context, share) : null, - onMore: () => _showShareActions(context, share), + onTap: () => _openShareSheet(context, share), ); }, childCount: shares.length + 1), ), ); } - /// Only folders are safely tappable - unlike Recent/Favorites, a share - /// doesn't carry the size/mime-type `FileViewerScreen` needs, so opening - /// a file share here would have to synthesize that data instead of - /// reading it, same as the original screen (no tap action at all). - void _openFolder(BuildContext context, NextcloudShare share) { - context.read().navigateToAbsoluteFolder(share.path); - context.read().requestTab(AppTab.files); - } - - /// The row/table overflow menu - used to jump straight to the unshare - /// confirmation with no menu at all, so a link share had no way to copy - /// its own URL short of opening it and finding "Copy link" inside the - /// full Share sheet. A link share gets that shortcut here too now; - /// everything else just gets the one remove/unshare action. - void _showShareActions(BuildContext context, NextcloudShare share) { - final isLink = share.shareType == ShareType.publicLink && share.url != null; - showNooSheet( - context, - children: [ - NooGroupedList( - children: [ - if (isLink) - NooSettingsRow( - icon: LucideIcons.copy, - label: const Text('Copy link'), - onTap: () { - Navigator.pop(context); - _copyShareLink(context, share); - }, - ), - NooSettingsRow( - icon: LucideIcons.trash2, - label: Text(share.sharedWithMe ? 'Remove share' : 'Unshare'), - danger: true, - onTap: () { - Navigator.pop(context); - _confirmUnshare(context, share); - }, - ), - ], - ), - ], - ); - } - - void _copyShareLink(BuildContext context, NextcloudShare share) { - Clipboard.setData(ClipboardData(text: share.url!)); - ScaffoldMessenger.of(context).showSnackBar( - const SnackBar( - content: Text('Link copied'), - behavior: SnackBarBehavior.floating, - ), - ); - } - - Future _confirmUnshare( + /// Every row's whole-row tap - a share only carries enough metadata for + /// its own row, not the size/dates the Share sheet's header needs, so + /// this fetches the real item first (see `FilesController.fetchItemAtPath`) + /// rather than opening the sheet straight from [share]. Reused by every + /// scope (With you/By you/Links) - the Share sheet is the one place that + /// already knows how to copy a link or remove access, so there's no + /// separate overflow menu here any more. + Future _openShareSheet( BuildContext context, NextcloudShare share, ) async { - final confirmed = await showDialog( - context: context, - builder: (dialogContext) { - return AlertDialog( - title: Text(share.sharedWithMe ? 'Remove share' : 'Unshare'), - content: Text( - share.sharedWithMe - ? 'Remove "${share.name}" shared with you by ${share.ownerDisplayName}?' - : 'Stop sharing "${share.name}"?', - ), - actions: [ - TextButton( - onPressed: () => Navigator.pop(dialogContext, false), - child: const Text('Cancel'), - ), - FilledButton( - style: FilledButton.styleFrom( - backgroundColor: Theme.of(dialogContext).colorScheme.error, - ), - onPressed: () => Navigator.pop(dialogContext, true), - child: const Text('Remove'), - ), - ], - ); - }, - ); - if (confirmed != true || !context.mounted) return; - + final files = context.read(); final messenger = ScaffoldMessenger.of(context); - final success = await context.read().deleteShare(share); - messenger.showSnackBar( - SnackBar( - content: Text( - success - ? 'Removed share for ${share.name}' - : 'Failed to remove share for ${share.name}', + final item = await files.fetchItemAtPath(share.path); + if (!context.mounted) return; + if (item == null) { + messenger.showSnackBar( + const SnackBar( + content: Text('Could not find this item - it may have moved'), + behavior: SnackBarBehavior.floating, ), - behavior: SnackBarBehavior.floating, - ), - ); + ); + return; + } + await ShareSheet.show(context, item); + // The sheet manages shares through `ItemOperations`, not this tab's own + // `SharesController` (see its `deleteShare`'s doc comment) - refetch so + // a share added/removed from inside it doesn't leave this list stale. + if (context.mounted) context.read().fetchAll(); } } diff --git a/lib/views/trash_view.dart b/lib/views/trash_view.dart index 2f919ce..6a23610 100644 --- a/lib/views/trash_view.dart +++ b/lib/views/trash_view.dart @@ -100,6 +100,9 @@ class _TrashViewState extends State { onRefresh: trashController.fetchAll, child: CustomScrollView( controller: widget.scrollController, + // See files_view.dart's identical fix - without this, pull-to- + // refresh can't be triggered on an empty or single-item list. + physics: const AlwaysScrollableScrollPhysics(), slivers: contentSlivers, ), ), diff --git a/lib/widgets/files_controls_row.dart b/lib/widgets/files_controls_row.dart index faeb5e3..abde3ee 100644 --- a/lib/widgets/files_controls_row.dart +++ b/lib/widgets/files_controls_row.dart @@ -109,6 +109,7 @@ class FilesControlsRow extends StatelessWidget { children: [ NooSegmentedControl( fill: true, + onSurface: true, value: files.sortAscendingFor(folderPath), onChanged: (asc) { if (asc != files.sortAscendingFor(folderPath)) { @@ -172,6 +173,7 @@ class FilesControlsRow extends StatelessWidget { // the two screens' filter sheets look and behave the same. NooSegmentedControl( fill: true, + onSurface: true, labelOnlySelected: true, value: files.filesTypeFilter, onChanged: files.setFilesTypeFilter, diff --git a/lib/widgets/sticky_header_delegate.dart b/lib/widgets/sticky_header_delegate.dart index 4559422..342386e 100644 --- a/lib/widgets/sticky_header_delegate.dart +++ b/lib/widgets/sticky_header_delegate.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import '../theme/design_tokens.dart'; /// Wraps [child] as a sliver that pins to the top of the scroll view when /// used with `SliverPersistentHeader(pinned: true, ...)`, or scrolls away @@ -24,7 +25,23 @@ class StickyHeaderDelegate extends SliverPersistentHeaderDelegate { double shrinkOffset, bool overlapsContent, ) { - return Material(color: Theme.of(context).colorScheme.surface, child: child); + // `context.nooColors.bg`, not `Theme.of(context).colorScheme.surface` - + // the latter is Flutter's own Material 3 scheme, seeded from the user's + // accent color choice (see AppTheme.light/dark), so it carried a faint + // hue of whatever accent is picked, and didn't match the plain + // `colors.bg` every one of these screens' own `ColoredBox` background + // uses. `bg`, not `surface`, so the sort/filter chips and List/Grid + // toggle riding on top of this (each already `colors.surface`-filled) + // still pop against it, the same as they do everywhere else in the app + // - matching `surface` here would make this pinned header the one place + // they'd flatten into their background instead. `surfaceTintColor: + // Colors.transparent` guards against the same M3 elevation-tint + // behavior even if this ever gets a non-zero elevation. + return Material( + color: context.nooColors.bg, + surfaceTintColor: Colors.transparent, + child: child, + ); } @override