diff --git a/.claude/context/styling.md b/.claude/context/styling.md index af2158b..570aba6 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 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. +`MediaTextPreview` is an editable monospace `TextField` padded clear of the status bar, top bar and action bar; every text file opens read-only (plain text as-is, Markdown rendered) and follows Edit -> Save: an Edit/Preview toggle and, once dirty, a Save button live in the viewer's top bar as round icon-only `NooButton`s (`iconOnly`, 36px; a lone button is primary, with two Edit/Preview is secondary and Save primary; Save becomes a primary circle with a spinner while it saves). Offline copies are read-only with no buttons. 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 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. +Markdown files (`.md`/`.markdown`) in `MediaTextPreview` open rendered via `flutter_markdown_plus` (`Markdown`, styled from Noo tokens in `_markdownStyle`), with the same Edit/Preview 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 69f11b3..5025e9b 100644 --- a/lib/views/file_viewer_screen.dart +++ b/lib/views/file_viewer_screen.dart @@ -759,7 +759,7 @@ 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. +/// round [NooButton] look (primary for the main action, secondary beside it). class _TextActions extends StatelessWidget { final TextPreviewController controller; @@ -767,39 +767,92 @@ class _TextActions extends StatelessWidget { @override Widget build(BuildContext context) { - final colors = context.nooColors; + final toggle = controller.canToggleEditing; + final save = controller.saving || controller.showSave; + // One button is the primary action; with two, Save is and Edit/Preview + // steps back to secondary. + final both = toggle && save; return Row( mainAxisSize: MainAxisSize.min, children: [ - if (controller.canToggleEditing) - NooTopBarButton( + if (toggle) + _ActionButton( icon: controller.editing ? LucideIcons.eye : LucideIcons.pencil, tooltip: controller.editing ? 'Preview' : 'Edit', + variant: both + ? NooButtonVariant.secondary + : NooButtonVariant.primary, onTap: controller.toggleEditing, ), + if (both) const SizedBox(width: 8), 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( + const _SavingIndicator() + else if (save) + _ActionButton( icon: LucideIcons.save, tooltip: 'Save', + variant: NooButtonVariant.primary, onTap: controller.save, ), ], ); } } + +/// A round icon-only [NooButton] with the tooltip and semantic label an +/// icon-only button needs. +class _ActionButton extends StatelessWidget { + final IconData icon; + final String tooltip; + final NooButtonVariant variant; + final VoidCallback onTap; + + const _ActionButton({ + required this.icon, + required this.tooltip, + required this.variant, + required this.onTap, + }); + + @override + Widget build(BuildContext context) { + return Tooltip( + message: tooltip, + child: Semantics( + button: true, + label: tooltip, + excludeSemantics: true, + child: NooButton( + icon: icon, + iconOnly: true, + variant: variant, + onTap: onTap, + ), + ), + ); + } +} + +/// Stands in for the Save button while it saves: the same primary circle, +/// with a spinner, so it can't be tapped twice. +class _SavingIndicator extends StatelessWidget { + const _SavingIndicator(); + + @override + Widget build(BuildContext context) { + final colors = context.nooColors; + return Semantics( + label: 'Saving', + child: Container( + width: 36, + height: 36, + alignment: Alignment.center, + decoration: BoxDecoration(color: colors.accent, shape: BoxShape.circle), + child: const SizedBox.square( + dimension: 16, + child: CircularProgressIndicator(strokeWidth: 2, color: Colors.white), + ), + ), + ); + } +} diff --git a/lib/widgets/noo/core/noo_button.dart b/lib/widgets/noo/core/noo_button.dart index 53a19d2..473443e 100644 --- a/lib/widgets/noo/core/noo_button.dart +++ b/lib/widgets/noo/core/noo_button.dart @@ -1,7 +1,15 @@ import 'package:flutter/material.dart'; import '../../../theme/design_tokens.dart'; -enum NooButtonVariant { primary, tonal, secondary, danger, outline, text, textDanger } +enum NooButtonVariant { + primary, + tonal, + secondary, + danger, + outline, + text, + textDanger, +} enum NooButtonSize { cta, card, field, toolbar, compact, xs } @@ -37,6 +45,11 @@ class NooButton extends StatefulWidget { final bool disabled; final VoidCallback? onTap; + /// A round, icon-only button (no label) - [icon] centred in a circle as + /// wide as the size's height, instead of the pill's side padding. Callers + /// are responsible for a tooltip/semantic label, since there's no text. + final bool iconOnly; + const NooButton({ super.key, this.variant = NooButtonVariant.primary, @@ -46,6 +59,7 @@ class NooButton extends StatefulWidget { this.fullWidth = false, this.disabled = false, this.onTap, + this.iconOnly = false, }); @override @@ -87,7 +101,8 @@ class _NooButtonState extends State { final colors = context.nooColors; final s = _sizes[widget.size]!; final (bg, fg, border) = _colors(colors); - final hasLeadingIcon = widget.icon != null && widget.child != null && !_isTextVariant; + final hasLeadingIcon = + widget.icon != null && widget.child != null && !_isTextVariant; final content = Row( mainAxisSize: MainAxisSize.min, @@ -107,7 +122,9 @@ class _NooButtonState extends State { child: GestureDetector( onTapDown: widget.disabled ? null : (_) => setState(() => _down = true), onTapUp: widget.disabled ? null : (_) => setState(() => _down = false), - onTapCancel: widget.disabled ? null : () => setState(() => _down = false), + onTapCancel: widget.disabled + ? null + : () => setState(() => _down = false), onTap: widget.disabled ? null : widget.onTap, child: AnimatedScale( scale: _down && !widget.disabled ? NooMotion.pressScale : 1, @@ -115,8 +132,10 @@ class _NooButtonState extends State { curve: NooMotion.ease, child: Container( height: s.height, - width: widget.fullWidth ? double.infinity : null, - padding: _isTextVariant + width: widget.iconOnly + ? s.height + : (widget.fullWidth ? double.infinity : null), + padding: _isTextVariant || widget.iconOnly ? EdgeInsets.zero : EdgeInsets.only( left: hasLeadingIcon ? s.px - 4 : s.px, @@ -128,7 +147,9 @@ class _NooButtonState extends State { // of shrink-wrapping to it, which is what stretched this button // edge-to-edge instead of sizing to its content (see NooFab's // doc comment for the same bug). - alignment: widget.fullWidth ? Alignment.center : null, + alignment: widget.fullWidth || widget.iconOnly + ? Alignment.center + : null, decoration: BoxDecoration( color: bg, borderRadius: BorderRadius.circular(NooRadii.pill), diff --git a/lib/widgets/viewer/media_text_preview.dart b/lib/widgets/viewer/media_text_preview.dart index af0640d..7d280df 100644 --- a/lib/widgets/viewer/media_text_preview.dart +++ b/lib/widgets/viewer/media_text_preview.dart @@ -89,8 +89,9 @@ class TextPreviewController extends ChangeNotifier { /// Text/markdown file preview and editor for [FileViewerScreen]'s static /// (non-swipeable) path (`_textPreviewExtensions`). Content is always -/// 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 +/// editable when online - after tapping Edit; every file opens read-only +/// (Markdown rendered) with an Edit/Preview toggle - and Save appears once it +/// has changed. 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. @@ -125,12 +126,15 @@ class _MediaTextPreviewState extends State { String? _error; bool _isSaving = false; bool _editing = false; + final _focus = FocusNode(); bool get _isMarkdown { final name = widget.item.name.toLowerCase(); return name.endsWith('.md') || name.endsWith('.markdown'); } + /// Markdown shows rendered until the user taps Edit. Plain text has no + /// rendered form, so it just shows its (read-only) text until then. bool get _showRendered => _isMarkdown && !_editing; bool get _readOnly => widget.localPathResolver != null; @@ -150,6 +154,7 @@ class _MediaTextPreviewState extends State { @override void dispose() { widget.controller?._unbind(); + _focus.dispose(); _controller.dispose(); super.dispose(); } @@ -159,7 +164,7 @@ class _MediaTextPreviewState extends State { /// initState, where notifying the bar's listeners would be illegal. void _syncToolbar() { widget.controller?._update( - canToggleEditing: _saved != null && _isMarkdown && !_readOnly, + canToggleEditing: _saved != null && !_readOnly, editing: _editing, dirty: _dirty, saving: _isSaving, @@ -169,6 +174,12 @@ class _MediaTextPreviewState extends State { void _toggleEditing() { setState(() => _editing = !_editing); _syncToolbar(); + // Tapping Edit should put the cursor in the text, not just unlock it. + if (_editing) { + WidgetsBinding.instance.addPostFrameCallback( + (_) => _focus.requestFocus(), + ); + } } Future _load() async { @@ -262,7 +273,10 @@ class _MediaTextPreviewState extends State { padding: padding, child: TextField( controller: _controller, - readOnly: _readOnly, + focusNode: _focus, + // Read-only until Edit is tapped (always, for the + // Offline copy). + readOnly: _readOnly || !_editing, maxLines: null, keyboardType: TextInputType.multiline, style: style, diff --git a/test/views/file_viewer_screen_test.dart b/test/views/file_viewer_screen_test.dart index 9e083a2..4a47fed 100644 --- a/test/views/file_viewer_screen_test.dart +++ b/test/views/file_viewer_screen_test.dart @@ -9,7 +9,7 @@ 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/noo/core/noo_button.dart'; import 'package:noo/widgets/viewer/media_action_bar.dart'; import 'package:provider/provider.dart'; @@ -49,9 +49,11 @@ class _Operations implements ItemOperations { dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); } -Finder _button(String tooltip) => find.byWidgetPredicate( - (widget) => widget is NooTopBarButton && widget.tooltip == tooltip, -); +Finder _button(String tooltip) => find.byTooltip(tooltip); + +/// The [NooButton] behind the top-bar action tagged [tooltip]. +Finder _noo(String tooltip) => + find.descendant(of: _button(tooltip), matching: find.byType(NooButton)); void main() { setUpNooTests(); @@ -118,6 +120,12 @@ void main() { } inHeader('Edit'); + // A lone action is the primary one, and a compact round icon button. + expect( + tester.widget(_noo('Edit')).variant, + NooButtonVariant.primary, + ); + expect(tester.getSize(_noo('Edit')), const Size(36, 36)); expect(find.byType(Markdown), findsOneWidget); await tester.tap(_button('Edit')); await tester.pumpAndSettle(); @@ -125,6 +133,15 @@ void main() { await tester.enterText(find.byType(TextField), '# Changed'); await tester.pump(); inHeader('Save'); + // With two, Save is primary and the toggle steps back to secondary. + expect( + tester.widget(_noo('Save')).variant, + NooButtonVariant.primary, + ); + expect( + tester.widget(_noo('Preview')).variant, + NooButtonVariant.secondary, + ); await tester.tap(_button('Preview')); await tester.pumpAndSettle(); expect(tester.widget(find.byType(Markdown)).data, '# Changed'); @@ -156,7 +173,7 @@ void main() { }); } - testWidgets('plain text has Save without the markdown toggle', ( + testWidgets('plain text opens read-only and follows Edit -> Save too', ( tester, ) async { final service = await mount( @@ -166,11 +183,35 @@ void main() { ); service.loaded.complete(utf8.encode('Original')); await tester.pumpAndSettle(); - expect(_button('Edit'), findsNothing); - expect(_button('Preview'), findsNothing); + + bool readOnly() => + tester.widget(find.byType(TextField)).readOnly; + expect(readOnly(), isTrue, reason: 'not directly editable'); + expect(_button('Edit'), findsOneWidget); + expect(_button('Save'), findsNothing); + expect( + tester.widget(_noo('Edit')).variant, + NooButtonVariant.primary, + ); + + await tester.tap(_button('Edit')); + await tester.pumpAndSettle(); + expect(readOnly(), isFalse); + expect(_button('Preview'), findsOneWidget); await tester.enterText(find.byType(TextField), 'Changed'); await tester.pump(); expect(_button('Save'), findsOneWidget); + expect( + tester.widget(_noo('Save')).variant, + NooButtonVariant.primary, + ); + + // Preview goes back to the read-only view, keeping the change unsaved. + await tester.tap(_button('Preview')); + await tester.pumpAndSettle(); + expect(readOnly(), isTrue); + expect(find.text('Changed'), findsOneWidget); + expect(_button('Save'), findsOneWidget); await tester.pumpWidget(const SizedBox.shrink()); });