diff --git a/lib/screens/settings/logs_screen.dart b/lib/screens/settings/logs_screen.dart index 2bbf5729d..a26286bbb 100644 --- a/lib/screens/settings/logs_screen.dart +++ b/lib/screens/settings/logs_screen.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'package:device_info_plus/device_info_plus.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/rendering.dart' show SelectedContent; import 'package:plezy/utils/media_server_http_client.dart'; import 'package:material_symbols_icons/symbols.dart'; import 'package:flutter/services.dart'; @@ -46,6 +47,55 @@ class _CollapsingSelectionMoveAction e Object? invoke(T intent) => callingAction?.invoke(intent); } +/// `SelectionArea` concatenates adjacent selectables with no separator; each +/// log row (and the device header) is its own paragraph, so a copied +/// drag-selection ran records together. The scrollable owns the delegate that +/// does the concatenation (and the lazy-row/autoscroll bookkeeping), so the +/// boundary is added per record: every selected record ends its text with +/// `\n`, and [_RecordSelectionDelegate] drops the one dangling after the last +/// record so a partial drag copies exactly what was highlighted. +class _RecordBoundaryDelegate extends StaticSelectionContainerDelegate { + @override + SelectedContent? getSelectedContent() { + final text = super.getSelectedContent()?.plainText; + // A drag ending at offset 0 of a row selects nothing in it: no boundary. + if (text == null || text.isEmpty) return null; + return SelectedContent(plainText: '$text\n'); + } +} + +class _RecordSelectionDelegate extends StaticSelectionContainerDelegate { + @override + SelectedContent? getSelectedContent() { + final text = super.getSelectedContent()?.plainText; + if (text == null) return null; + return SelectedContent(plainText: text.endsWith('\n') ? text.substring(0, text.length - 1) : text); + } +} + +/// One selectable record: the device header or a single log entry. +class _LogRecord extends StatefulWidget { + const _LogRecord({required this.child}); + + final Widget child; + + @override + State<_LogRecord> createState() => _LogRecordState(); +} + +class _LogRecordState extends State<_LogRecord> { + final _RecordBoundaryDelegate _boundary = _RecordBoundaryDelegate(); + + @override + void dispose() { + _boundary.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) => SelectionContainer(delegate: _boundary, child: widget.child); +} + class LogsScreen extends StatefulWidget { const LogsScreen({super.key, this.httpClient, this.deviceInfoPlugin}); @@ -63,6 +113,7 @@ class _LogsScreenState extends State with MountedSetStateMixin { // skipTraversal: selection is pointer-driven; the region must not become a // D-pad/Tab stop between the action bar and the scrollable body. final FocusNode _selectionFocusNode = FocusNode(skipTraversal: true, debugLabel: 'logs-selection'); + final _RecordSelectionDelegate _recordSelection = _RecordSelectionDelegate(); MediaServerHttpClient get _httpClient => widget.httpClient ?? httpClient; @@ -128,6 +179,7 @@ class _LogsScreenState extends State with MountedSetStateMixin { void dispose() { _scrollController.dispose(); _selectionFocusNode.dispose(); + _recordSelection.dispose(); super.dispose(); } @@ -409,72 +461,81 @@ class _LogsScreenState extends State with MountedSetStateMixin { }, child: SelectionArea( focusNode: _selectionFocusNode, - child: CustomScrollView( - primary: true, - slivers: [ - CustomAppBar( - // Chrome is not log content; keep it out of drag-selection. - title: SelectionContainer.disabled(child: Text(t.screens.logs)), - pinned: true, - actions: [ - FocusableActionBar( - actions: [ - FocusableAction( - icon: Symbols.refresh_rounded, - tooltip: t.common.refresh, - onPressed: _loadLogs, - ), - FocusableAction( - icon: Symbols.upload_rounded, - tooltip: t.logs.uploadLogs, - onPressed: _hasDiagnostics ? _uploadLogs : null, - ), - FocusableAction( - icon: Symbols.content_copy_rounded, - tooltip: t.logs.copyLogs, - onPressed: _hasDiagnostics ? _copyAllLogs : null, - ), - FocusableAction( - icon: Symbols.delete_outline_rounded, - tooltip: t.logs.clearLogs, - onPressed: _hasDiagnostics ? _clearLogs : null, - ), - ], - ), - ], - ), - // A launch that failed the startup gate leaves nothing in the - // in-memory buffer — that process is gone. Show its record - // here, where the user can actually act on it (#1732). - ?_buildPreviousFailureBanner(theme), - if (_logs.isEmpty) - SliverFillRemaining(child: Center(child: Text(t.messages.noLogsAvailable))) - else ...[ - if (_deviceInfo.isNotEmpty) - SliverPadding( - padding: const EdgeInsets.fromLTRB(12, 12, 12, 0), - sliver: SliverToBoxAdapter( - child: Text.rich(TextSpan(style: _logTextStyle(theme), children: _buildDeviceInfoSpans())), + child: SelectionContainer( + delegate: _recordSelection, + child: CustomScrollView( + primary: true, + slivers: [ + CustomAppBar( + // Chrome is not log content; keep it out of drag-selection. + title: SelectionContainer.disabled(child: Text(t.screens.logs)), + pinned: true, + actions: [ + FocusableActionBar( + actions: [ + FocusableAction( + icon: Symbols.refresh_rounded, + tooltip: t.common.refresh, + onPressed: _loadLogs, + ), + FocusableAction( + icon: Symbols.upload_rounded, + tooltip: t.logs.uploadLogs, + onPressed: _hasDiagnostics ? _uploadLogs : null, + ), + FocusableAction( + icon: Symbols.content_copy_rounded, + tooltip: t.logs.copyLogs, + onPressed: _hasDiagnostics ? _copyAllLogs : null, + ), + FocusableAction( + icon: Symbols.delete_outline_rounded, + tooltip: t.logs.clearLogs, + onPressed: _hasDiagnostics ? _clearLogs : null, + ), + ], ), - ), - SliverPadding( - padding: EdgeInsets.fromLTRB(12, _deviceInfo.isEmpty ? 12 : 0, 12, 12), - // One widget per entry so only the visible slice is laid - // out. The buffer holds up to 5 MiB of text; as a single - // paragraph that was a multi-second frame and hundreds of - // MB of glyph data — an OOM kill on phones and TVs. - sliver: SliverList.builder( - itemCount: _logs.length, - itemBuilder: (context, index) => Text.rich( - TextSpan(style: _logTextStyle(theme), children: _buildEntrySpans(_logs[index])), - ), - ), + ], ), - // Only the log body needs it: the empty state already fills - // the viewport, so a trailing inset would just add slack. - const SliverSystemBottomInset(), + // A launch that failed the startup gate leaves nothing in the + // in-memory buffer — that process is gone. Show its record + // here, where the user can actually act on it (#1732). + ?_buildPreviousFailureBanner(theme), + if (_logs.isEmpty) + SliverFillRemaining(child: Center(child: Text(t.messages.noLogsAvailable))) + else ...[ + if (_deviceInfo.isNotEmpty) + SliverPadding( + padding: const EdgeInsets.fromLTRB(12, 12, 12, 0), + sliver: SliverToBoxAdapter( + child: _LogRecord( + child: Text.rich( + TextSpan(style: _logTextStyle(theme), children: _buildDeviceInfoSpans()), + ), + ), + ), + ), + SliverPadding( + padding: EdgeInsets.fromLTRB(12, _deviceInfo.isEmpty ? 12 : 0, 12, 12), + // One widget per entry so only the visible slice is laid + // out. The buffer holds up to 5 MiB of text; as a single + // paragraph that was a multi-second frame and hundreds of + // MB of glyph data — an OOM kill on phones and TVs. + sliver: SliverList.builder( + itemCount: _logs.length, + itemBuilder: (context, index) => _LogRecord( + child: Text.rich( + TextSpan(style: _logTextStyle(theme), children: _buildEntrySpans(_logs[index])), + ), + ), + ), + ), + // Only the log body needs it: the empty state already fills + // the viewport, so a trailing inset would just add slack. + const SliverSystemBottomInset(), + ], ], - ], + ), ), ), ), diff --git a/test/screens/settings/logs_screen_test.dart b/test/screens/settings/logs_screen_test.dart index e666a1274..b6c8f7336 100644 --- a/test/screens/settings/logs_screen_test.dart +++ b/test/screens/settings/logs_screen_test.dart @@ -1,6 +1,7 @@ import 'dart:convert'; import 'package:device_info_plus/device_info_plus.dart'; +import 'package:flutter/gestures.dart' show PointerDeviceKind; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -291,6 +292,17 @@ void main() { await tester.pumpAndSettle(); } + // Nested SelectionContainers (region > screen > scrollable > record > text) + // hand their selectables upward one level per frame through post-frame + // callbacks that do not schedule a frame themselves. The app always has + // frames in flight here (route transition); the test must pump them. + Future pumpSelectionRegistration(WidgetTester tester) async { + for (var i = 0; i < 3; i++) { + tester.binding.scheduleFrame(); + await tester.pump(); + } + } + testWidgets('lays out only the visible slice of a large buffer', (tester) async { // Rendering the whole buffer as one paragraph froze the frame for tens // of seconds and OOM-killed low-memory devices; offscreen entries must @@ -337,6 +349,88 @@ void main() { expect(clipboardText, isNot(contains('oldest-copy-marker'))); expect(tester.takeException(), isNull); }); + + testWidgets('selection copy separates the header and each record with a newline', (tester) async { + // Each row is its own paragraph; SelectionArea glues adjacent + // paragraphs together, so a select-all copy used to read + // `---[t] newer-row[t] older-row`. + String? clipboardText; + final messenger = TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + messenger.setMockMethodCallHandler(SystemChannels.platform, (call) async { + if (call.method == 'Clipboard.setData') { + clipboardText = (call.arguments as Map)['text'] as String?; + } + return null; + }); + addTearDown(() => messenger.setMockMethodCallHandler(SystemChannels.platform, null)); + + seedLogs(['older-row', 'newer-row']); + + await pumpLogs(tester); + await pumpSelectionRegistration(tester); + // `flutter test` runs as Android, whose text-editing shortcuts bind + // Ctrl+A / Ctrl+C; the region's own focus node is the shortcut target. + tester.widget(find.byType(SelectableRegion)).focusNode!.requestFocus(); + await tester.pump(); + await tester.sendKeyDownEvent(LogicalKeyboardKey.controlLeft); + await tester.sendKeyEvent(LogicalKeyboardKey.keyA); + await tester.pump(); + await tester.sendKeyEvent(LogicalKeyboardKey.keyC); + await tester.sendKeyUpEvent(LogicalKeyboardKey.controlLeft); + await tester.pump(); + + expect(clipboardText, isNotNull); + expect(clipboardText, startsWith('Plezy')); + expect(clipboardText, matches(RegExp(r'---\n\['))); + expect(clipboardText, matches(RegExp(r'newer-row\n\['))); + expect(clipboardText, endsWith('older-row')); + expect(clipboardText, isNot(contains('newer-row['))); + expect(tester.takeException(), isNull); + }); + + testWidgets('a partial drag copy ends at the highlighted text, not at a record boundary', (tester) async { + String? clipboardText; + final messenger = TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + messenger.setMockMethodCallHandler(SystemChannels.platform, (call) async { + if (call.method == 'Clipboard.setData') { + clipboardText = (call.arguments as Map)['text'] as String?; + } + return null; + }); + addTearDown(() => messenger.setMockMethodCallHandler(SystemChannels.platform, null)); + + seedLogs(['older-row', 'newer-row']); + + await pumpLogs(tester); + await pumpSelectionRegistration(tester); + final header = find.textContaining('Plezy', findRichText: true); + final newerRow = find.textContaining('newer-row', findRichText: true); + final gesture = await tester.startGesture( + tester.getTopLeft(header) + const Offset(1, 6), + kind: PointerDeviceKind.mouse, + ); + await tester.pump(); + // Stop inside the newer row's timestamp: the drag covers the header + // and part of one record only. + await gesture.moveTo(tester.getTopLeft(newerRow) + const Offset(40, 6)); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + tester.widget(find.byType(SelectableRegion)).focusNode!.requestFocus(); + await tester.pump(); + await tester.sendKeyDownEvent(LogicalKeyboardKey.controlLeft); + await tester.sendKeyEvent(LogicalKeyboardKey.keyC); + await tester.sendKeyUpEvent(LogicalKeyboardKey.controlLeft); + await tester.pump(); + + expect(clipboardText, isNotNull); + expect(clipboardText, matches(RegExp(r'---\n\['))); + expect(clipboardText, isNot(contains('newer-row'))); + expect(clipboardText, isNot(contains('older-row'))); + expect(clipboardText, isNot(endsWith('\n'))); + expect(tester.takeException(), isNull); + }); }); group('app-bar D-pad traversal', () {