fix(logs): keep record boundaries in copied log selections
Copying a drag or keyboard selection from the logs screen produced one run-on line: every log entry and the device header is its own paragraph, and Flutter's selection area concatenates adjacent paragraphs with no separator. Each record is now its own selection container whose selected text ends with a newline, and the screen-level container drops the one dangling after the last selected record, so a partial drag copies exactly what was highlighted. Rows stay lazily built with their geometry unchanged, and the toolbar's copy-all export keeps its own format.
This commit is contained in:
@@ -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<T extends DirectionalCaretMovementIntent> 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<LogsScreen> 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<LogsScreen> with MountedSetStateMixin {
|
||||
void dispose() {
|
||||
_scrollController.dispose();
|
||||
_selectionFocusNode.dispose();
|
||||
_recordSelection.dispose();
|
||||
super.dispose();
|
||||
}
|
||||
|
||||
@@ -409,72 +461,81 @@ class _LogsScreenState extends State<LogsScreen> 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(),
|
||||
],
|
||||
],
|
||||
],
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
|
||||
@@ -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<void> 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<Object?, Object?>)['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<SelectableRegion>(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<Object?, Object?>)['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<SelectableRegion>(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', () {
|
||||
|
||||
Reference in New Issue
Block a user