fix(logs): let D-pad leave the app-bar back button on the logs screen
On TV, pressing Up on the Logs screen put focus on the back button and no
direction key could move it again, so the upload/copy actions were out of
reach (edde746/plezy#2242). The SelectionArea added in 8fe7e4bcf installs
always-enabled caret-movement actions that Android's default text-editing
shortcuts map plain arrow keys to; the keys were consumed as no-op
selection moves and never reached DirectionalFocusIntent.
Override those two intents above the region with an action that is
disabled for the collapsing (unshifted) variants and defers to the
region's own handler otherwise, so Shift+Arrow selection keeps working.
This commit is contained in:
edde746
@@ -31,6 +31,21 @@ import '../../widgets/system_bottom_inset.dart';
|
||||
|
||||
const previousStartupFailureKey = Key('logs-previous-startup-failure');
|
||||
|
||||
/// `SelectionArea` claims plain arrow keys as no-op caret moves, which starves
|
||||
/// `DirectionalFocusIntent` and pins D-pad focus on the app-bar back button.
|
||||
/// Only the collapsing (unshifted) variants are disabled; Shift+Arrow selection
|
||||
/// still reaches the region's own handler via [callingAction].
|
||||
class _CollapsingSelectionMoveAction<T extends DirectionalCaretMovementIntent> extends Action<T> {
|
||||
@override
|
||||
bool isEnabled(T intent) => !intent.collapseSelection && (callingAction?.isEnabled(intent) ?? false);
|
||||
|
||||
@override
|
||||
bool consumesKey(T intent) => callingAction?.consumesKey(intent) ?? false;
|
||||
|
||||
@override
|
||||
Object? invoke(T intent) => callingAction?.invoke(intent);
|
||||
}
|
||||
|
||||
class LogsScreen extends StatefulWidget {
|
||||
const LogsScreen({super.key, this.httpClient, this.deviceInfoPlugin});
|
||||
|
||||
@@ -386,73 +401,81 @@ class _LogsScreenState extends State<LogsScreen> with MountedSetStateMixin {
|
||||
child: IosStatusBarTapScrollToTop(
|
||||
controller: _scrollController,
|
||||
child: Scaffold(
|
||||
body: 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,
|
||||
body: Actions(
|
||||
actions: <Type, Action<Intent>>{
|
||||
ExtendSelectionByCharacterIntent: _CollapsingSelectionMoveAction<ExtendSelectionByCharacterIntent>(),
|
||||
ExtendSelectionVerticallyToAdjacentLineIntent:
|
||||
_CollapsingSelectionMoveAction<ExtendSelectionVerticallyToAdjacentLineIntent>(),
|
||||
},
|
||||
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())),
|
||||
),
|
||||
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())),
|
||||
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])),
|
||||
),
|
||||
),
|
||||
),
|
||||
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(),
|
||||
// Only the log body needs it: the empty state already fills
|
||||
// the viewport, so a trailing inset would just add slack.
|
||||
const SliverSystemBottomInset(),
|
||||
],
|
||||
],
|
||||
],
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
|
||||
@@ -16,6 +16,8 @@ import 'package:plezy/services/log_upload_service.dart';
|
||||
import 'package:plezy/services/startup_diagnostics.dart';
|
||||
import 'package:plezy/utils/app_logger.dart';
|
||||
import 'package:plezy/utils/media_server_http_client.dart';
|
||||
import 'package:plezy/utils/platform_detector.dart';
|
||||
import 'package:plezy/widgets/app_bar_back_button.dart';
|
||||
|
||||
void main() {
|
||||
setUpAll(() {
|
||||
@@ -336,4 +338,98 @@ void main() {
|
||||
expect(tester.takeException(), isNull);
|
||||
});
|
||||
});
|
||||
|
||||
group('app-bar D-pad traversal', () {
|
||||
// Android TV: `defaultTargetPlatform` is already Android under
|
||||
// `flutter test`, which is what selects the Android text-editing shortcuts.
|
||||
setUp(() async {
|
||||
TvDetectionService.debugSetAppleTVOverride(null);
|
||||
await TvDetectionService.getInstance(forceTv: true);
|
||||
TvDetectionService.setForceTVSync(true);
|
||||
PackageInfo.setMockInitialValues(
|
||||
appName: 'Plezy',
|
||||
packageName: 'com.plezy.test',
|
||||
version: '1.2.3',
|
||||
buildNumber: '45',
|
||||
buildSignature: '',
|
||||
);
|
||||
});
|
||||
|
||||
tearDown(() {
|
||||
TvDetectionService.debugSetAppleTVOverride(null);
|
||||
TvDetectionService.setForceTVSync(false);
|
||||
});
|
||||
|
||||
testWidgets('ArrowRight from the app-bar back button reaches the action bar on TV', (tester) async {
|
||||
// The back button only exists on a pushed route, which is how the
|
||||
// screen is always reached from Settings.
|
||||
await tester.pumpWidget(
|
||||
TranslationProvider(
|
||||
child: InputModeTracker(
|
||||
child: MaterialApp(
|
||||
initialRoute: '/logs',
|
||||
routes: {'/': (_) => const Scaffold(), '/logs': (_) => const LogsScreen()},
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
final backButton = find.byType(AppBarBackButton);
|
||||
expect(backButton, findsOneWidget);
|
||||
final backNode = Focus.of(tester.element(find.descendant(of: backButton, matching: find.byType(Tooltip))));
|
||||
backNode.requestFocus();
|
||||
await tester.pumpAndSettle();
|
||||
expect(backNode.hasPrimaryFocus, isTrue);
|
||||
|
||||
await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
final focused = FocusManager.instance.primaryFocus;
|
||||
expect(
|
||||
focused?.context?.findAncestorWidgetOfExactType<FocusableActionBar>(),
|
||||
isNotNull,
|
||||
reason: 'focus stayed on ${focused?.debugLabel}; Right should move it into the action bar',
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('Shift+ArrowRight from the app-bar back button is still consumed by the selection region', (
|
||||
tester,
|
||||
) async {
|
||||
await tester.pumpWidget(
|
||||
TranslationProvider(
|
||||
child: InputModeTracker(
|
||||
child: MaterialApp(
|
||||
initialRoute: '/logs',
|
||||
routes: {'/': (_) => const Scaffold(), '/logs': (_) => const LogsScreen()},
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
final backButton = find.byType(AppBarBackButton);
|
||||
final backNode = Focus.of(tester.element(find.descendant(of: backButton, matching: find.byType(Tooltip))));
|
||||
backNode.requestFocus();
|
||||
await tester.pumpAndSettle();
|
||||
expect(backNode.hasPrimaryFocus, isTrue);
|
||||
|
||||
// Nothing else on this screen handles Right, so "handled" can only come
|
||||
// from the SelectionArea's extend-selection action: the override must
|
||||
// defer to it rather than swallow every arrow.
|
||||
await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft);
|
||||
final handled = await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight);
|
||||
await tester.sendKeyUpEvent(LogicalKeyboardKey.shiftLeft);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(handled, isTrue, reason: 'Shift+Right is a selection gesture and must reach the SelectionArea');
|
||||
expect(backNode.hasPrimaryFocus, isTrue);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user