From 2d18a49fc260049f30604fca0bd06f06cab9a8e8 Mon Sep 17 00:00:00 2001 From: cschlaefke Date: Wed, 4 Nov 2026 01:25:23 +0100 Subject: [PATCH] fix(catalog): keyboard reach of the result list, tab order, stable select focus Tiles are tab stops again, tab order is search field, filters, list (also with the side bar), selects keep focus after a choice. Failure kind is logged with stable names. Refs #3 Co-Authored-By: Claude Sonnet 5.5 --- .../data/supabase_catalog_repository.dart | 13 +- .../search/catalog_search_view.dart | 65 +++++---- .../search/widgets/catalog_filter_panel.dart | 35 ++--- .../search/widgets/catalog_game_tile.dart | 9 +- .../search/catalog_search_a11y_test.dart | 2 +- .../search/catalog_search_behavior_test.dart | 127 ++++++++++++++++-- .../search/catalog_search_screen_test.dart | 4 +- .../search/catalog_search_test_support.dart | 48 +++++++ .../search/catalog_search_view_test.dart | 44 +----- 9 files changed, 240 insertions(+), 107 deletions(-) create mode 100644 app/test/features/catalog/presentation/search/catalog_search_test_support.dart diff --git a/app/lib/features/catalog/data/supabase_catalog_repository.dart b/app/lib/features/catalog/data/supabase_catalog_repository.dart index 2edee36..ab153c4 100644 --- a/app/lib/features/catalog/data/supabase_catalog_repository.dart +++ b/app/lib/features/catalog/data/supabase_catalog_repository.dart @@ -101,13 +101,16 @@ class SupabaseCatalogRepository implements CatalogRepository { } } - /// Maps and logs an error. Logs the failure kind and the error type only. + /// Maps and logs an error. Logs the failure kind only. CatalogFailure _fail(String operation, Object error) { final failure = mapCatalogError(error); - _log.warning( - 'Catalog $operation failed: ${failure.runtimeType} ' - '(${error.runtimeType})', - ); + // Stable names (runtime type names are minified on the web). + final kind = switch (failure) { + CatalogNetworkFailure() => 'network', + CatalogInvalidInputFailure() => 'invalidInput', + CatalogUnexpectedFailure() => 'unexpected', + }; + _log.warning('Catalog $operation failed: $kind'); return failure; } } diff --git a/app/lib/features/catalog/presentation/search/catalog_search_view.dart b/app/lib/features/catalog/presentation/search/catalog_search_view.dart index c4a112e..62c1d76 100644 --- a/app/lib/features/catalog/presentation/search/catalog_search_view.dart +++ b/app/lib/features/catalog/presentation/search/catalog_search_view.dart @@ -263,37 +263,52 @@ class _CatalogSearchViewState extends ConsumerState { child: Column( crossAxisAlignment: CrossAxisAlignment.stretch, children: [ - Padding( - padding: EdgeInsets.fromLTRB( - padding.left, - padding.top, - padding.right, - AppSpacing.sm, + FocusTraversalOrder( + order: const NumericFocusOrder(1), + child: Padding( + padding: EdgeInsets.fromLTRB( + padding.left, + padding.top, + padding.right, + AppSpacing.sm, + ), + child: _buildHeader(state, showFilterButton: !expanded), + ), + ), + Expanded( + child: FocusTraversalOrder( + order: const NumericFocusOrder(3), + child: _buildResults(state, padding), ), - child: _buildHeader(state, showFilterButton: !expanded), ), - Expanded(child: _buildResults(state, padding)), ], ), ), ); - if (!expanded) { - return main; - } - return Row( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - SizedBox( - width: _sidebarWidth, - child: SingleChildScrollView( - key: const Key('catalog_filter_sidebar'), - padding: EdgeInsets.all(padding.left), - child: _buildSidebar(state), - ), - ), - const VerticalDivider(width: 1), - Expanded(child: main), - ], + // Tab order (Spec #3 ยง 6): search field, filters, list. With the side + // bar left of the field the visual order alone would be wrong. + return FocusTraversalGroup( + policy: OrderedTraversalPolicy(), + child: expanded + ? Row( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + FocusTraversalOrder( + order: const NumericFocusOrder(2), + child: SizedBox( + width: _sidebarWidth, + child: SingleChildScrollView( + key: const Key('catalog_filter_sidebar'), + padding: EdgeInsets.all(padding.left), + child: _buildSidebar(state), + ), + ), + ), + const VerticalDivider(width: 1), + Expanded(child: main), + ], + ) + : main, ); }, ); diff --git a/app/lib/features/catalog/presentation/search/widgets/catalog_filter_panel.dart b/app/lib/features/catalog/presentation/search/widgets/catalog_filter_panel.dart index 411a078..5e02f3b 100644 --- a/app/lib/features/catalog/presentation/search/widgets/catalog_filter_panel.dart +++ b/app/lib/features/catalog/presentation/search/widgets/catalog_filter_panel.dart @@ -118,24 +118,29 @@ class _SingleChoice extends StatelessWidget { @override Widget build(BuildContext context) { + // A controlled drop-down (no form field that keeps its own value), so a + // selection does not rebuild the field and keyboard focus stays on it. return KeyedSubtree( key: fieldKey, - child: DropdownButtonFormField( - key: ValueKey(value), - // The field keeps its own value; a new key follows outside changes - // (reset, URL). - initialValue: value, - isExpanded: true, + child: InputDecorator( + // "Any" is a value too, so the label always floats. + isEmpty: false, decoration: InputDecoration(labelText: label), - items: [ - DropdownMenuItem(child: Text(anyLabel)), - for (final option in options) - DropdownMenuItem( - value: option, - child: Text(optionLabel(option)), - ), - ], - onChanged: onChanged, + child: DropdownButtonHideUnderline( + child: DropdownButton( + value: value, + isExpanded: true, + items: [ + DropdownMenuItem(child: Text(anyLabel)), + for (final option in options) + DropdownMenuItem( + value: option, + child: Text(optionLabel(option)), + ), + ], + onChanged: onChanged, + ), + ), ), ); } diff --git a/app/lib/features/catalog/presentation/search/widgets/catalog_game_tile.dart b/app/lib/features/catalog/presentation/search/widgets/catalog_game_tile.dart index 4cbda29..c709982 100644 --- a/app/lib/features/catalog/presentation/search/widgets/catalog_game_tile.dart +++ b/app/lib/features/catalog/presentation/search/widgets/catalog_game_tile.dart @@ -8,9 +8,10 @@ import 'package:shuffle/features/catalog/domain/catalog_game_summary.dart'; /// /// Shows a [GameCover] placeholder (no image URL in #3), title, year, /// publishers and short facts; missing facts are left out. [onTap] is `null` -/// in #3: the tile is then not a button (no dead link) but still focusable -/// programmatically ([focusNode]) so keyboard focus can move to the first -/// newly loaded tile. +/// in #3: the tile is then not a button (no dead link) but stays a keyboard +/// stop with a visible focus ring, so keyboard users can walk the list (which +/// scrolls it and loads more) and focus can move to the first newly loaded +/// tile ([focusNode]). class CatalogGameTile extends StatefulWidget { const CatalogGameTile({ required this.game, @@ -147,8 +148,6 @@ class _CatalogGameTileState extends State { final onTap = widget.onTap; return Focus( focusNode: widget.focusNode, - // Not a tab stop while the tile does nothing, but focusable by code. - skipTraversal: onTap == null, onFocusChange: (focused) => setState(() => _focused = focused), child: Semantics( container: true, diff --git a/app/test/features/catalog/presentation/search/catalog_search_a11y_test.dart b/app/test/features/catalog/presentation/search/catalog_search_a11y_test.dart index f8dd4f8..a60a35c 100644 --- a/app/test/features/catalog/presentation/search/catalog_search_a11y_test.dart +++ b/app/test/features/catalog/presentation/search/catalog_search_a11y_test.dart @@ -3,7 +3,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:shuffle/features/catalog/domain/catalog_search_query.dart'; import '../../catalog_test_support.dart'; -import 'catalog_search_view_test.dart'; +import 'catalog_search_test_support.dart'; void main() { group('announcements', () { diff --git a/app/test/features/catalog/presentation/search/catalog_search_behavior_test.dart b/app/test/features/catalog/presentation/search/catalog_search_behavior_test.dart index 4fdbcb8..05663f2 100644 --- a/app/test/features/catalog/presentation/search/catalog_search_behavior_test.dart +++ b/app/test/features/catalog/presentation/search/catalog_search_behavior_test.dart @@ -5,7 +5,7 @@ import 'package:shuffle/features/catalog/domain/catalog_failure.dart'; import 'package:shuffle/features/catalog/domain/catalog_search_query.dart'; import '../../catalog_test_support.dart'; -import 'catalog_search_view_test.dart'; +import 'catalog_search_test_support.dart'; Future openFilterSheet(WidgetTester tester) async { await tester.tap(find.byKey(const Key('catalog_filter_button'))); @@ -184,25 +184,128 @@ void main() { }); group('keyboard', () { - testWidgets('QUE-06 Kriterium 4: Tab leads from the field to the filters', ( + Future tab(WidgetTester tester) async { + await tester.sendKeyEvent(LogicalKeyboardKey.tab); + await tester.pump(); + } + + bool focusIsIn(Finder finder) { + final focused = FocusManager.instance.primaryFocus?.context; + return focused != null && + find + .descendant( + of: finder, + matching: find.byElementPredicate((e) => e == focused), + ) + .evaluate() + .isNotEmpty; + } + + /// The tile's `Focus` sits above its keyed card. + bool focusOnTile(String id) { + final focused = FocusManager.instance.primaryFocus?.context; + return focused != null && + find + .ancestor( + of: find.byKey(Key('catalog_tile_$id')), + matching: find.byElementPredicate((e) => e == focused), + ) + .evaluate() + .isNotEmpty; + } + + double scrollOffset(WidgetTester tester) => tester + .state( + find.descendant( + of: find.byKey(const Key('catalog_result_list')), + matching: find.byType(Scrollable), + ), + ) + .position + .pixels; + + testWidgets( + 'QUE-06 Kriterium 4: Tab walks field, filters, list (compact)', + (tester) async { + final repository = FakeCatalogRepository(all: List.generate(75, game)); + await pumpView(tester, repository); + + await tester.tap(searchInput); + await tester.pump(); + await tab(tester); + expect( + focusIsIn(find.byKey(const Key('catalog_filter_button'))), + isTrue, + ); + + await tab(tester); + expect(focusOnTile('game-0'), isTrue); + + // Walking on scrolls the list (and so loads more). + for (var i = 0; i < 12; i++) { + await tab(tester); + } + expect(scrollOffset(tester), greaterThan(0)); + expect(focusOnTile('game-12'), isTrue); + }, + ); + + testWidgets( + 'QUE-06 Kriterium 4: Tab order is field, filters, list (side bar)', + (tester) async { + final repository = FakeCatalogRepository(all: List.generate(75, game)); + await pumpView(tester, repository, width: 1200); + + await tester.tap(searchInput); + await tester.pump(); + await tab(tester); + expect( + focusIsIn(find.byKey(const Key('catalog_filter_sidebar'))), + isTrue, + reason: 'the filters come right after the search field', + ); + + // Through the filters (3 selects, 2 chip groups, reset) to the list. + var steps = 0; + while (!focusOnTile('game-0') && steps++ < 30) { + await tab(tester); + } + expect(focusOnTile('game-0'), isTrue); + expect(steps, lessThan(30)); + }, + ); + + testWidgets('a selection keeps keyboard focus on the select', ( tester, ) async { final repository = FakeCatalogRepository(all: [game(1)]); await pumpView(tester, repository); + await openFilterSheet(tester); - await tester.tap(searchInput); - await tester.pump(); - await tester.sendKeyEvent(LogicalKeyboardKey.tab); - await tester.pump(); - final focused = FocusManager.instance.primaryFocus!.context; + await tester.tap(find.byKey(const Key('catalog_filter_players'))); + await tester.pumpAndSettle(); + await tester.tap(find.text('4 Spieler').last); + await tester.pumpAndSettle(); + + expect(repository.calls.last.query.filters.players, 4); expect( - find.descendant( - of: find.byKey(const Key('catalog_filter_button')), - matching: find.byElementPredicate((e) => e == focused), - ), - findsOneWidget, + focusIsIn(find.byKey(const Key('catalog_filter_players'))), + isTrue, ); }); + + testWidgets('the sheet stays open while filters change the URL state', ( + tester, + ) async { + final repository = FakeCatalogRepository(all: [game(1)]); + await pumpView(tester, repository); + await openFilterSheet(tester); + + await tester.tap(find.byKey(const Key('catalog_term_$genreFamily'))); + await tester.pumpAndSettle(); + + expect(find.byKey(const Key('catalog_filter_sheet')), findsOneWidget); + }); }); group('layout', () { diff --git a/app/test/features/catalog/presentation/search/catalog_search_screen_test.dart b/app/test/features/catalog/presentation/search/catalog_search_screen_test.dart index f9398f0..331c5f1 100644 --- a/app/test/features/catalog/presentation/search/catalog_search_screen_test.dart +++ b/app/test/features/catalog/presentation/search/catalog_search_screen_test.dart @@ -13,7 +13,7 @@ import 'package:shuffle/features/catalog/presentation/search/catalog_search_view import '../../../../helpers/pump_app.dart'; import '../../../../helpers/router_helpers.dart'; import '../../catalog_test_support.dart'; -import 'catalog_search_view_test.dart'; +import 'catalog_search_test_support.dart'; void main() { group('route /catalog', () { @@ -187,6 +187,8 @@ void main() { final params = currentUri(router).queryParameters; expect(params['q'], 'alt'); expect(params['genre'], genreFamily); + // Writing the URL does not close the open filter sheet. + expect(find.byKey(const Key('catalog_filter_sheet')), findsOneWidget); }); testWidgets('outside the web nothing is written', (tester) async { diff --git a/app/test/features/catalog/presentation/search/catalog_search_test_support.dart b/app/test/features/catalog/presentation/search/catalog_search_test_support.dart new file mode 100644 index 0000000..9e70a1c --- /dev/null +++ b/app/test/features/catalog/presentation/search/catalog_search_test_support.dart @@ -0,0 +1,48 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shuffle/features/catalog/domain/catalog_search_query.dart'; +import 'package:shuffle/features/catalog/presentation/search/catalog_search_view.dart'; + +import '../../../../helpers/pump_app.dart'; +import '../../catalog_test_support.dart'; + +/// Sets a logical window size for the test (device pixel ratio 1). +void useWindow(WidgetTester tester, double width, [double height = 900]) { + tester.view.physicalSize = Size(width, height); + tester.view.devicePixelRatio = 1; + addTearDown(tester.view.reset); +} + +Future pumpView( + WidgetTester tester, + FakeCatalogRepository repository, { + CatalogSearchQuery initialQuery = const CatalogSearchQuery(), + ValueChanged? onReportGame, + Locale locale = const Locale('de'), + double? textScale, + double width = 420, + bool settle = true, +}) async { + useWindow(tester, width); + await pumpApp( + tester, + locale: locale, + textScale: textScale, + overrides: catalogOverrides(repository), + settle: false, + home: Scaffold( + body: CatalogSearchView( + initialQuery: initialQuery, + onReportGame: onReportGame, + ), + ), + ); + if (settle) { + await tester.pumpAndSettle(); + } +} + +Finder get searchInput => find.descendant( + of: find.byKey(const Key('catalog_search_field')), + matching: find.byType(TextField), +); diff --git a/app/test/features/catalog/presentation/search/catalog_search_view_test.dart b/app/test/features/catalog/presentation/search/catalog_search_view_test.dart index dbc882e..99cf86d 100644 --- a/app/test/features/catalog/presentation/search/catalog_search_view_test.dart +++ b/app/test/features/catalog/presentation/search/catalog_search_view_test.dart @@ -4,51 +4,9 @@ import 'package:shuffle/core/widgets/game_cover.dart'; import 'package:shuffle/features/catalog/domain/catalog_failure.dart'; import 'package:shuffle/features/catalog/domain/catalog_search_query.dart'; import 'package:shuffle/features/catalog/presentation/search/catalog_search_controller.dart'; -import 'package:shuffle/features/catalog/presentation/search/catalog_search_view.dart'; -import '../../../../helpers/pump_app.dart'; import '../../catalog_test_support.dart'; - -/// Sets a logical window size for the test (device pixel ratio 1). -void useWindow(WidgetTester tester, double width, [double height = 900]) { - tester.view.physicalSize = Size(width, height); - tester.view.devicePixelRatio = 1; - addTearDown(tester.view.reset); -} - -Future pumpView( - WidgetTester tester, - FakeCatalogRepository repository, { - CatalogSearchQuery initialQuery = const CatalogSearchQuery(), - ValueChanged? onReportGame, - Locale locale = const Locale('de'), - double? textScale, - double width = 420, - bool settle = true, -}) async { - useWindow(tester, width); - await pumpApp( - tester, - locale: locale, - textScale: textScale, - overrides: catalogOverrides(repository), - settle: false, - home: Scaffold( - body: CatalogSearchView( - initialQuery: initialQuery, - onReportGame: onReportGame, - ), - ), - ); - if (settle) { - await tester.pumpAndSettle(); - } -} - -Finder get searchInput => find.descendant( - of: find.byKey(const Key('catalog_search_field')), - matching: find.byType(TextField), -); +import 'catalog_search_test_support.dart'; void main() { group('list and tiles', () {