From 5fa20ff3e98beff5d438070e95ac69fcd4437298 Mon Sep 17 00:00:00 2001 From: Ayushya Amitabh Date: Tue, 6 Oct 2026 20:38:19 -0400 Subject: [PATCH] Media viewer: Edit/Preview and Save move to the top bar A TextPreviewController (owned by the viewer, bound by MediaTextPreview) mirrors the editing/dirty/saving state out so the top bar draws the buttons; Save shows a spinner while saving. Located and tested via a Codex consult (its end-to-end widget test, with the provider fixed and the saving state adapted); the controller design replaces its builder-wrapping patch. Co-Authored-By: Claude Sonnet 5.5 --- .claude/context/styling.md | 4 +- lib/views/file_viewer_screen.dart | 62 ++++++- lib/widgets/viewer/media_text_preview.dart | 142 +++++++++++---- test/views/file_viewer_screen_test.dart | 195 +++++++++++++++++++++ 4 files changed, 369 insertions(+), 34 deletions(-) create mode 100644 test/views/file_viewer_screen_test.dart diff --git a/.claude/context/styling.md b/.claude/context/styling.md index b4913bc..af2158b 100644 --- a/.claude/context/styling.md +++ b/.claude/context/styling.md @@ -391,11 +391,11 @@ blocks are noted where they matter: ## Text/markdown viewer -`MediaTextPreview` is an editable monospace `TextField` padded clear of the status bar, top bar and action bar; a Save button appears when dirty (read-only for offline copies). `NooPersonAccessRow.trailing` replaces the owner label/permission pill (used by share-search results). Sheets whose close button lives in `showNooSheet` children must pop via a `Builder` context, not the caller's. +`MediaTextPreview` is an editable monospace `TextField` padded clear of the status bar, top bar and action bar; a Save icon button appears in the viewer's top bar when dirty (a spinner while it saves; read-only for offline copies). The preview owns the text, the editing mode and the save; a `TextPreviewController` (owned by `FileViewerScreen`, passed to `MediaTextPreview`) mirrors that state out so the screen's top bar can draw - and trigger - the buttons, which keeps the preview's widget tree unchanged. `NooPersonAccessRow.trailing` replaces the owner label/permission pill (used by share-search results). Sheets whose close button lives in `showNooSheet` children must pop via a `Builder` context, not the caller's. `showNooSheet` insets its body by the keyboard (`viewInsets.bottom`) so focused fields stay visible; the body's widget structure must not change when the keyboard opens, or the sheet content is rebuilt and loses focus. -Markdown files (`.md`/`.markdown`) in `MediaTextPreview` open rendered via `flutter_markdown_plus` (`Markdown`, styled from Noo tokens in `_markdownStyle`), with a top-right Edit/Preview toggle (hidden for read-only offline copies). Other text files go straight to the editor. +Markdown files (`.md`/`.markdown`) in `MediaTextPreview` open rendered via `flutter_markdown_plus` (`Markdown`, styled from Noo tokens in `_markdownStyle`), with an Edit/Preview icon toggle in the viewer's top bar (hidden for read-only offline copies). Other text files go straight to the editor. `ShareSheet`: focusing the people search field does not scroll; once the user types, `_revealPeopleSection` animates the "Share with people" section (keyed by `_peopleKey` on the `NooShareSection`, not the inner column) to just below the sheet's top edge with a small gap. diff --git a/lib/views/file_viewer_screen.dart b/lib/views/file_viewer_screen.dart index 32e22f0..69f11b3 100644 --- a/lib/views/file_viewer_screen.dart +++ b/lib/views/file_viewer_screen.dart @@ -178,6 +178,10 @@ class _FileViewerScreenState extends State { final _panelKey = GlobalKey(); + /// Edit/Preview + Save for a text/markdown file: the preview owns the + /// editing, this just lets the top bar draw (and trigger) its buttons. + final _textActions = TextPreviewController(); + void _openDetails() { if (NooLayout.isDesktop(context)) { DetailsSheet.show(context, _currentItem); @@ -229,6 +233,7 @@ class _FileViewerScreenState extends State { @override void dispose() { + _textActions.dispose(); _pageController.dispose(); super.dispose(); } @@ -532,7 +537,14 @@ class _FileViewerScreenState extends State { ), ), ), - const SizedBox(width: 10), + // Edit/Preview and Save for text/markdown files - + // empty (zero-width) for every other file type. + ListenableBuilder( + listenable: _textActions, + builder: (context, _) => + _TextActions(controller: _textActions), + ), + const SizedBox(width: 4), ], ), ), @@ -731,6 +743,7 @@ class _FileViewerScreenState extends State { return MediaTextPreview( item: widget.item, session: session, + controller: _textActions, localPathResolver: widget.localPathResolver, ); } @@ -743,3 +756,50 @@ class _FileViewerScreenState extends State { } } } + +/// The text viewer's top-bar buttons: Edit/Preview (markdown only) and Save +/// (once the content changed; a spinner while it saves). Same +/// [NooTopBarButton] look as the back button next to it. +class _TextActions extends StatelessWidget { + final TextPreviewController controller; + + const _TextActions({required this.controller}); + + @override + Widget build(BuildContext context) { + final colors = context.nooColors; + return Row( + mainAxisSize: MainAxisSize.min, + children: [ + if (controller.canToggleEditing) + NooTopBarButton( + icon: controller.editing ? LucideIcons.eye : LucideIcons.pencil, + tooltip: controller.editing ? 'Preview' : 'Edit', + onTap: controller.toggleEditing, + ), + if (controller.saving) + Semantics( + label: 'Saving', + child: SizedBox.square( + dimension: 48, + child: Center( + child: SizedBox.square( + dimension: 20, + child: CircularProgressIndicator( + strokeWidth: 2, + color: colors.fg1, + ), + ), + ), + ), + ) + else if (controller.showSave) + NooTopBarButton( + icon: LucideIcons.save, + tooltip: 'Save', + onTap: controller.save, + ), + ], + ); + } +} diff --git a/lib/widgets/viewer/media_text_preview.dart b/lib/widgets/viewer/media_text_preview.dart index 8dde92d..af0640d 100644 --- a/lib/widgets/viewer/media_text_preview.dart +++ b/lib/widgets/viewer/media_text_preview.dart @@ -2,11 +2,9 @@ import 'dart:convert'; import 'dart:io'; import 'package:flutter/material.dart'; import 'package:flutter_markdown_plus/flutter_markdown_plus.dart'; -import 'package:lucide_icons_flutter/lucide_icons.dart'; import '../../models/nextcloud_item.dart'; import '../../providers/session_controller.dart'; import '../../theme/design_tokens.dart'; -import '../noo/core/noo_button.dart'; /// Space the viewer's floating top bar and bottom action bar cover, so the /// text sits clear of them (and the status bar) instead of scrolling @@ -14,16 +12,96 @@ import '../noo/core/noo_button.dart'; const double _kTopBarClearance = 72; const double _kActionBarClearance = 112; +/// What the viewer's top bar needs from [MediaTextPreview]: whether to show +/// the Edit/Preview toggle and the Save button, their current state, and a way +/// to trigger them. The preview owns the text, the editing mode and the save; +/// this just mirrors that out so `FileViewerScreen` can draw the buttons in +/// its own top bar (they used to float inside the preview). +class TextPreviewController extends ChangeNotifier { + bool _canToggleEditing = false; + bool _editing = false; + bool _dirty = false; + bool _saving = false; + bool _disposed = false; + VoidCallback? _onToggleEditing; + Future Function()? _onSave; + + /// Markdown, writable (not the read-only Offline copy) and loaded - the + /// Edit/Preview toggle only exists then. + bool get canToggleEditing => _canToggleEditing; + + /// Markdown is currently showing its raw text (the toggle then offers + /// "Preview"); false while rendered (it offers "Edit"). + bool get editing => _editing; + + /// The text differs from what's saved on the server. + bool get dirty => _dirty; + bool get saving => _saving; + + /// The Save button is shown once the content has changed, and stays (as a + /// spinner) while the save is in flight. + bool get showSave => _dirty || _saving; + + void toggleEditing() => _onToggleEditing?.call(); + Future save() async => _onSave?.call(); + + void _bind(VoidCallback toggleEditing, Future Function() save) { + _onToggleEditing = toggleEditing; + _onSave = save; + } + + /// The preview went away: nothing to toggle or save any more. Quiet (no + /// notification) - it happens while the tree is being torn down. + void _unbind() { + _onToggleEditing = null; + _onSave = null; + _canToggleEditing = false; + _editing = false; + _dirty = false; + _saving = false; + } + + void _update({ + required bool canToggleEditing, + required bool editing, + required bool dirty, + required bool saving, + }) { + if (_canToggleEditing == canToggleEditing && + _editing == editing && + _dirty == dirty && + _saving == saving) { + return; + } + _canToggleEditing = canToggleEditing; + _editing = editing; + _dirty = dirty; + _saving = saving; + if (!_disposed) notifyListeners(); + } + + @override + void dispose() { + _disposed = true; + super.dispose(); + } +} + /// Text/markdown file preview and editor for [FileViewerScreen]'s static /// (non-swipeable) path (`_textPreviewExtensions`). Content is always -/// editable when online; a Save button appears once it has changed. Markdown -/// files open rendered, with an Edit/Preview toggle. Opened -/// from the Offline tab ([localPathResolver] set) it is read-only, since the -/// local copy has no server to write back to. +/// editable when online; Save appears once it has changed. Markdown files open +/// rendered, with an Edit/Preview toggle. Both controls live in the viewer's +/// top bar, driven through [controller]. Opened from the Offline tab +/// ([localPathResolver] set) it is read-only, since the local copy has no +/// server to write back to. class MediaTextPreview extends StatefulWidget { final NextcloudItem item; final SessionController session; + /// Receives this preview's editing/save state and is how the top bar's + /// buttons reach it. Optional so the preview also works on its own. + final TextPreviewController? controller; + /// When set, read bytes from the local path it resolves to instead of /// fetching from the server - see /// `FileViewerScreen.localPathResolver`'s doc comment. @@ -33,6 +111,7 @@ class MediaTextPreview extends StatefulWidget { super.key, required this.item, required this.session, + this.controller, this.localPathResolver, }); @@ -60,16 +139,38 @@ class _MediaTextPreviewState extends State { @override void initState() { super.initState(); - _controller.addListener(() => setState(() {})); + widget.controller?._bind(_toggleEditing, _save); + _controller.addListener(() { + setState(() {}); + _syncToolbar(); + }); _load(); } @override void dispose() { + widget.controller?._unbind(); _controller.dispose(); super.dispose(); } + /// Mirrors the editing/save state out to the viewer's top bar. Only called + /// from event handlers and async completions - never from build or + /// initState, where notifying the bar's listeners would be illegal. + void _syncToolbar() { + widget.controller?._update( + canToggleEditing: _saved != null && _isMarkdown && !_readOnly, + editing: _editing, + dirty: _dirty, + saving: _isSaving, + ); + } + + void _toggleEditing() { + setState(() => _editing = !_editing); + _syncToolbar(); + } + Future _load() async { try { final List bytes; @@ -89,6 +190,7 @@ class _MediaTextPreviewState extends State { final text = utf8.decode(bytes, allowMalformed: true); _controller.text = text; setState(() => _saved = text); + _syncToolbar(); } } catch (e) { if (mounted) setState(() => _error = e.toString()); @@ -99,6 +201,7 @@ class _MediaTextPreviewState extends State { final messenger = ScaffoldMessenger.of(context); final text = _controller.text; setState(() => _isSaving = true); + _syncToolbar(); var ok = false; try { ok = await widget.session.service!.putBytes( @@ -111,6 +214,7 @@ class _MediaTextPreviewState extends State { _isSaving = false; if (ok) _saved = text; }); + _syncToolbar(); messenger.showSnackBar( SnackBar( content: Text(ok ? 'Saved ${widget.item.name}' : 'Could not save file'), @@ -171,30 +275,6 @@ class _MediaTextPreviewState extends State { ), ), ), - if (_isMarkdown && !_readOnly) - Positioned( - right: 20, - top: safe.top + _kTopBarClearance - 8, - child: NooButton( - variant: NooButtonVariant.tonal, - size: NooButtonSize.compact, - icon: _editing ? LucideIcons.eye : LucideIcons.pencil, - onTap: () => setState(() => _editing = !_editing), - child: Text(_editing ? 'Preview' : 'Edit'), - ), - ), - if (_dirty) - Positioned( - right: 20, - bottom: safe.bottom + _kActionBarClearance, - child: NooButton( - size: NooButtonSize.card, - icon: LucideIcons.save, - disabled: _isSaving, - onTap: _isSaving ? null : _save, - child: Text(_isSaving ? 'Saving…' : 'Save'), - ), - ), ], ), ); diff --git a/test/views/file_viewer_screen_test.dart b/test/views/file_viewer_screen_test.dart new file mode 100644 index 0000000..9e083a2 --- /dev/null +++ b/test/views/file_viewer_screen_test.dart @@ -0,0 +1,195 @@ +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter/material.dart'; +import 'package:flutter_markdown_plus/flutter_markdown_plus.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:noo/models/nextcloud_item.dart'; +import 'package:noo/providers/item_operations.dart'; +import 'package:noo/providers/session_controller.dart'; +import 'package:noo/services/nextcloud_service.dart'; +import 'package:noo/views/file_viewer_screen.dart'; +import 'package:noo/widgets/noo/nav/noo_top_bar.dart'; +import 'package:noo/widgets/viewer/media_action_bar.dart'; +import 'package:provider/provider.dart'; + +import '../widgets/noo/noo_test_utils.dart'; + +class _Service extends NextcloudService { + _Service() : super(serverUrl: '', username: '', password: ''); + + final loaded = Completer>(); + Completer? saving; + String? written; + + @override + Future> fetchBytes(String itemPath) => loaded.future; + + @override + Future putBytes(String itemPath, List bytes) { + written = utf8.decode(bytes); + saving = Completer(); + return saving!.future; + } +} + +// These stand-ins avoid starting session restoration and platform channels. +class _Session extends ChangeNotifier implements SessionController { + _Session(this.service); + + @override + final NextcloudService service; + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +class _Operations implements ItemOperations { + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +Finder _button(String tooltip) => find.byWidgetPredicate( + (widget) => widget is NooTopBarButton && widget.tooltip == tooltip, +); + +void main() { + setUpNooTests(); + + Future<_Service> mount( + WidgetTester tester, + Size size, { + String name = 'notes.md', + bool offline = false, + }) async { + final service = _Service(); + final session = _Session(service); + addTearDown(session.dispose); + await tester.binding.setSurfaceSize(size); + addTearDown(() => tester.binding.setSurfaceSize(null)); + await tester.pumpWidget( + MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: session), + Provider.value(value: _Operations()), + ], + child: MaterialApp( + theme: nooTheme(Brightness.light), + home: FileViewerScreen( + item: NextcloudItem( + id: '1', + name: name, + path: '/$name', + type: NextcloudItemType.file, + size: 12, + lastModified: DateTime(2026), + ), + localPathResolver: offline ? (_) async => null : null, + ), + ), + ), + ); + return service; + } + + for (final width in [390.0, 1200.0]) { + testWidgets('text actions use the top bar at width $width', (tester) async { + final service = await mount(tester, Size(width, 800)); + expect(_button('Edit'), findsNothing); + expect(_button('Save'), findsNothing); + service.loaded.complete(utf8.encode('# Original')); + await tester.pumpAndSettle(); + + void inHeader(String label) { + final row = find + .ancestor(of: _button('Back'), matching: find.byType(Row)) + .first; + expect( + find.descendant(of: row, matching: _button(label)), + findsOneWidget, + ); + expect( + find.descendant( + of: find.byType(MediaActionBar), + matching: _button(label), + ), + findsNothing, + ); + } + + inHeader('Edit'); + expect(find.byType(Markdown), findsOneWidget); + await tester.tap(_button('Edit')); + await tester.pumpAndSettle(); + inHeader('Preview'); + await tester.enterText(find.byType(TextField), '# Changed'); + await tester.pump(); + inHeader('Save'); + await tester.tap(_button('Preview')); + await tester.pumpAndSettle(); + expect(tester.widget(find.byType(Markdown)).data, '# Changed'); + inHeader('Save'); + await tester.tap(_button('Save')); + await tester.pump(); + expect(service.written, '# Changed'); + // While the save is in flight the Save button gives way to a spinner, + // so it can't be tapped twice. + expect(_button('Save'), findsNothing); + expect( + find.byWidgetPredicate( + (w) => w is Semantics && w.properties.label == 'Saving', + ), + findsOneWidget, + ); + service.saving!.complete(false); + await tester.pumpAndSettle(); + inHeader('Save'); + expect(find.text('Could not save file'), findsOneWidget); + await tester.tap(_button('Save')); + await tester.pump(); + service.saving!.complete(true); + await tester.pumpAndSettle(); + expect(_button('Save'), findsNothing); + expect(_button('Edit'), findsOneWidget); + expect(tester.takeException(), isNull); + await tester.pumpWidget(const SizedBox.shrink()); + }); + } + + testWidgets('plain text has Save without the markdown toggle', ( + tester, + ) async { + final service = await mount( + tester, + const Size(390, 800), + name: 'notes.txt', + ); + service.loaded.complete(utf8.encode('Original')); + await tester.pumpAndSettle(); + expect(_button('Edit'), findsNothing); + expect(_button('Preview'), findsNothing); + await tester.enterText(find.byType(TextField), 'Changed'); + await tester.pump(); + expect(_button('Save'), findsOneWidget); + await tester.pumpWidget(const SizedBox.shrink()); + }); + + for (final offline in [false, true]) { + testWidgets('no text actions for offline=$offline unsupported files', ( + tester, + ) async { + await mount( + tester, + const Size(390, 800), + name: offline ? 'notes.md' : 'data.bin', + offline: offline, + ); + await tester.pumpAndSettle(); + expect(_button('Edit'), findsNothing); + expect(_button('Preview'), findsNothing); + expect(_button('Save'), findsNothing); + expect(tester.takeException(), isNull); + await tester.pumpWidget(const SizedBox.shrink()); + }); + } +}