feat(catalog): address review of GameCover (labels, tests, spec notes)

Refs #2 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-11-03 15:26:38 +01:00 · 2026-11-03 15:26:38 +01:00 · f99c82b1e0
commit f99c82b1e0
parent 8a68d4f449
5 changed files with 199 additions and 32 deletions
--- a/app/lib/core/widgets/game_cover.dart
+++ b/app/lib/core/widgets/game_cover.dart
@ -36,6 +36,17 @@ class GameCover extends StatelessWidget {
  static const double aspectRatio = 3 / 4;

  /// Resolved image URL; `null` or empty shows the placeholder.
+  ///
+  /// Must come from the file access layer (own Supabase Storage), never from
+  /// `source_url` or other third-party URLs (privacy: the IP address would
+  /// reach third parties).
+  ///
+  /// Notes for the callers (#3, #4), not implemented here:
+  /// - The loading indicator animates endlessly, so `pumpAndSettle` in tests
+  ///   times out while an image loads (use `pump`). It does not honor
+  ///   `MediaQuery.disableAnimations` yet.
+  /// - A private bucket would need signed URLs or auth headers.
+  /// - Self-hosted Storage on the web needs CORS for the app origin.
  final String? imageUrl;

  /// Game title, used for the semantics label only.
@ -52,9 +63,23 @@ class GameCover extends StatelessWidget {
    final url = imageUrl;
    final hasUrl = url != null && url.isNotEmpty;
    final height = size.width / aspectRatio;
+    final l10n = context.l10n;

-    Widget placeholder({bool loading = false}) =>
-        _CoverPlaceholder(size: size, loading: loading);
+    // The label follows what is visible: placeholder vs. loaded image.
+    Widget labeled(String label, Widget child) => titleVisibleNearby
+        ? child
+        : Semantics(
+            container: true,
+            image: true,
+            label: label,
+            excludeSemantics: true,
+            child: child,
+          );
+
+    Widget placeholder({bool loading = false}) => labeled(
+      l10n.gameCoverPlaceholderLabel(title),
+      _CoverPlaceholder(size: size, loading: loading),
+    );

    final Widget visual = hasUrl
        ? Image.network(
@ -62,12 +87,14 @@ class GameCover extends StatelessWidget {
            fit: BoxFit.cover,
            width: size.width,
            height: height,
+            cacheWidth: (size.width * MediaQuery.devicePixelRatioOf(context))
+                .round(),
            excludeFromSemantics: true,
            // No frame decoded yet (also before the first byte arrives).
            frameBuilder: (context, child, frame, wasSynchronouslyLoaded) =>
                frame == null && !wasSynchronouslyLoaded
                ? placeholder(loading: true)
-                : child,
+                : labeled(l10n.gameCoverLabel(title), child),
            errorBuilder: (context, error, stackTrace) => placeholder(),
          )
        : placeholder();
@ -78,20 +105,7 @@ class GameCover extends StatelessWidget {
      height: height,
      child: ClipRRect(borderRadius: AppRadius.smAll, child: visual),
    );
-
-    if (titleVisibleNearby) {
-      return ExcludeSemantics(child: framed);
-    }
-    final l10n = context.l10n;
-    return Semantics(
-      container: true,
-      image: true,
-      label: hasUrl
-          ? l10n.gameCoverLabel(title)
-          : l10n.gameCoverPlaceholderLabel(title),
-      excludeSemantics: true,
-      child: framed,
-    );
+    return titleVisibleNearby ? ExcludeSemantics(child: framed) : framed;
  }
 }

--- a/app/lib/l10n/app_de.arb
+++ b/app/lib/l10n/app_de.arb
@ -77,7 +77,7 @@
  "authResetTitle": "Neues Passwort festlegen",
  "authResetSubmit": "Passwort speichern",
  "authResetCancel": "Abbrechen",
+  "authPasswordChanged": "Passwort geändert. Andere Geräte werden abgemeldet; das kann bis zu einer Stunde dauern.",
  "gameCoverPlaceholderLabel": "Kein Cover für {title}",
-  "gameCoverLabel": "Cover von {title}",
-  "authPasswordChanged": "Passwort geändert. Andere Geräte werden abgemeldet; das kann bis zu einer Stunde dauern."
+  "gameCoverLabel": "Cover von {title}"
 }
--- a/app/lib/l10n/app_en.arb
+++ b/app/lib/l10n/app_en.arb
@ -123,6 +123,7 @@
  "authResetTitle": "Set a new password",
  "authResetSubmit": "Save password",
  "authResetCancel": "Cancel",
+  "authPasswordChanged": "Password changed. Other devices will be signed out; this can take up to an hour.",
  "gameCoverPlaceholderLabel": "No cover for {title}",
  "@gameCoverPlaceholderLabel": {
    "description": "Semantics label of the uniform placeholder cover",
@ -132,6 +133,5 @@
  "@gameCoverLabel": {
    "description": "Semantics label of a game cover image",
    "placeholders": {"title": {"type": "String"}}
-  },
-  "authPasswordChanged": "Password changed. Other devices will be signed out; this can take up to an hour."
+  }
 }
--- a/app/test/core/widgets/game_cover_test.dart
+++ b/app/test/core/widgets/game_cover_test.dart
@ -1,3 +1,8 @@
+import 'dart:async';
+import 'dart:convert';
+import 'dart:io';
+import 'dart:typed_data';
+
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:shuffle/core/widgets/game_cover.dart';
@ -13,8 +18,86 @@ double _contrast(Color a, Color b) {
  return (hi + 0.05) / (lo + 0.05);
 }

+// 1x1 transparent PNG.
+final Uint8List _png = base64Decode(
+  'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==',
+);
+
+class _FakeHeaders extends Fake implements HttpHeaders {
+  @override
+  void add(String name, Object value, {bool preserveHeaderCase = false}) {}
+}
+
+class _FakeResponse extends Fake implements HttpClientResponse {
+  @override
+  int get statusCode => 200;
+  @override
+  int get contentLength => _png.length;
+  @override
+  HttpClientResponseCompressionState get compressionState =>
+      HttpClientResponseCompressionState.notCompressed;
+  @override
+  StreamSubscription<List<int>> listen(
+    void Function(List<int> event)? onData, {
+    Function? onError,
+    void Function()? onDone,
+    bool? cancelOnError,
+  }) => Stream<List<int>>.value(_png).listen(
+    onData,
+    onError: onError,
+    onDone: onDone,
+    cancelOnError: cancelOnError,
+  );
+}
+
+class _FakeRequest extends Fake implements HttpClientRequest {
+  @override
+  HttpHeaders get headers => _FakeHeaders();
+  @override
+  Future<HttpClientResponse> close() async => _FakeResponse();
+}
+
+class _FakeClient extends Fake implements HttpClient {
+  @override
+  bool autoUncompress = true;
+  @override
+  Future<HttpClientRequest> getUrl(Uri url) async => _FakeRequest();
+}
+
+const _url = 'https://example.test/c.png';
+// Separate URL: a failed load can stay in the image cache's live images.
+const _errorUrl = 'https://example.test/broken.png';
+
+Future<void> _pumpImage(WidgetTester tester, Widget child) async {
+  await pumpWidgetUnderTest(tester, child, settle: false);
+  await tester.runAsync(
+    () => Future<void>.delayed(const Duration(milliseconds: 50)),
+  );
+  await tester.pump();
+  await tester.pump();
+}
+
+/// NetworkImage caches one shared HttpClient, so `HttpOverrides` would only
+/// work for the first test; the debug provider is the supported hook. It is
+/// reset in `finally` because the framework verifies it before tearDowns run.
+void _testWithFakeNetwork(
+  String description,
+  Future<void> Function(WidgetTester tester) body,
+) {
+  testWidgets(description, (tester) async {
+    debugNetworkImageHttpClientProvider = _FakeClient.new;
+    try {
+      await body(tester);
+    } finally {
+      debugNetworkImageHttpClientProvider = null;
+    }
+  });
+}
+
 void main() {
-  group('GameCover', () {
+  setUp(() => PaintingBinding.instance.imageCache.clear());
+
+  group('KAT-05 Kriterium 10: GameCover placeholder', () {
    testWidgets('without imageUrl shows the uniform placeholder', (
      tester,
    ) async {
@ -33,33 +116,94 @@ void main() {
      expect(find.byType(Image), findsNothing);
    });

-    testWidgets('shows a subtle loading indicator while the image loads', (
+    testWidgets('loading shows placeholder, indicator and placeholder label', (
      tester,
    ) async {
+      final handle = tester.ensureSemantics();
      await pumpWidgetUnderTest(
        tester,
-        const GameCover(title: 'Catan', imageUrl: 'https://example.test/c.png'),
+        const GameCover(title: 'Catan', imageUrl: _url),
        settle: false,
      );
      expect(find.byKey(const Key('game_cover_placeholder')), findsOneWidget);
      expect(find.byKey(const Key('game_cover_loading')), findsOneWidget);
+      expect(find.bySemanticsLabel('Kein Cover für Catan'), findsOneWidget);
+      expect(find.bySemanticsLabel('Cover von Catan'), findsNothing);
+      handle.dispose();
    });

-    testWidgets('a load error falls back to the placeholder without error', (
+    testWidgets('a load error falls back to the placeholder and its label', (
      tester,
    ) async {
      // flutter_test answers all HTTP requests with status 400.
-      await pumpWidgetUnderTest(
+      final handle = tester.ensureSemantics();
+      await _pumpImage(
        tester,
-        const GameCover(title: 'Catan', imageUrl: 'https://example.test/c.png'),
-        settle: false,
+        const GameCover(title: 'Catan', imageUrl: _errorUrl),
      );
-      await tester.runAsync(() => Future<void>.delayed(Duration.zero));
-      await tester.pump();
-      await tester.pump();
      expect(find.byKey(const Key('game_cover_placeholder')), findsOneWidget);
      expect(find.byKey(const Key('game_cover_loading')), findsNothing);
+      expect(find.bySemanticsLabel('Kein Cover für Catan'), findsOneWidget);
+      expect(find.bySemanticsLabel('Cover von Catan'), findsNothing);
      expect(tester.takeException(), isNull);
+      handle.dispose();
+    });
+
+    group('loaded image', () {
+      for (final (locale, expected) in [
+        (const Locale('de'), 'Cover von Catan'),
+        (const Locale('en'), 'Cover of Catan'),
+      ]) {
+        _testWithFakeNetwork(
+          'shows the image with its label in ${locale.languageCode}',
+          (tester) async {
+            final handle = tester.ensureSemantics();
+            tester.view.physicalSize = const Size(400, 800);
+            tester.view.devicePixelRatio = 1;
+            addTearDown(tester.view.reset);
+            await pumpApp(
+              tester,
+              locale: locale,
+              settle: false,
+              home: const Scaffold(
+                body: GameCover(title: 'Catan', imageUrl: _url),
+              ),
+            );
+            await tester.runAsync(
+              () => Future<void>.delayed(const Duration(milliseconds: 50)),
+            );
+            await tester.pump();
+            await tester.pump();
+            expect(find.byType(Image), findsOneWidget);
+            expect(
+              find.byKey(const Key('game_cover_placeholder')),
+              findsNothing,
+            );
+            expect(find.bySemanticsLabel(expected), findsOneWidget);
+            handle.dispose();
+          },
+        );
+      }
+
+      _testWithFakeNetwork('a cached image is shown synchronously', (
+        tester,
+      ) async {
+        final handle = tester.ensureSemantics();
+        const cover = GameCover(title: 'Catan', imageUrl: _url);
+        await _pumpImage(tester, cover);
+        expect(find.byKey(const Key('game_cover_placeholder')), findsNothing);
+
+        // New element with the same URL: served from the image cache.
+        await pumpWidgetUnderTest(
+          tester,
+          const GameCover(key: Key('second'), title: 'Catan', imageUrl: _url),
+          settle: false,
+        );
+        expect(find.byKey(const Key('game_cover_placeholder')), findsNothing);
+        expect(find.byKey(const Key('game_cover_loading')), findsNothing);
+        expect(find.bySemanticsLabel('Cover von Catan'), findsOneWidget);
+        handle.dispose();
+      });
    });

    testWidgets('size variants differ and keep the aspect ratio', (
@ -76,7 +220,9 @@ void main() {
      expect(small.width / small.height, closeTo(GameCover.aspectRatio, 0.01));
      expect(large.width / large.height, closeTo(GameCover.aspectRatio, 0.01));
    });
+  });

+  group('QUE-05 Kriterium 2: GameCover labels', () {
    for (final (locale, expected) in [
      (const Locale('de'), 'Kein Cover für Catan'),
      (const Locale('en'), 'No cover for Catan'),
@ -94,7 +240,9 @@ void main() {
        handle.dispose();
      });
    }
+  });

+  group('QUE-06 Kriterium 1, 2: GameCover accessibility', () {
    testWidgets('is excluded from semantics when the title is nearby', (
      tester,
    ) async {
@ -121,7 +269,7 @@ void main() {
        final scheme = Theme.of(
          tester.element(find.byKey(const Key('game_cover'))),
        ).colorScheme;
-        // Non-text contrast of the symbol against the surface (QUE-06): 3:1.
+        // Non-text contrast of the symbol against the surface: 3:1.
        expect(
          _contrast(scheme.onSurfaceVariant, scheme.surfaceContainerHighest),
          greaterThanOrEqualTo(3),
--- a/docs/specs/2-katalog-daten.md
+++ b/docs/specs/2-katalog-daten.md
@ -474,6 +474,7 @@ KAT-05 hat in R1 keine eigenen Screens (keine Admin-UI, Klärung 44). Die App-An
 - Zustände: kein Bild → Platzhalter; Bild lädt → Platzhalter mit dezenter Ladeanzeige; Fehler beim Laden → Platzhalter (kein Fehlerzustand, ein fehlendes Cover ist kein Fehler).
 - Platzhalter (Spec-F9): **ein einheitlicher, neutraler Platzhalter für alle Spiele**, mit Flutter gezeichnet (Fläche in Theme-Farbe, neutrales Symbol), keine Bilddatei von Dritten, kein neues Paket.
 - Semantics: Bild ausgeblendet aus dem Semantics-Baum, wenn der Titel daneben steht (`excludeSemantics`-Variante); sonst Beschriftung aus ARB `gameCoverPlaceholderLabel` („Kein Cover für {title}“ / „No cover for {title}“).
+- Ergänzung bei der Umsetzung: zusätzlicher ARB-Schlüssel `gameCoverLabel` („Cover von {title}“ / „Cover of {title}“) für ein tatsächlich geladenes Bild; die Beschriftung folgt dem, was sichtbar ist (Platzhalter beim Laden und nach einem Ladefehler, sonst `gameCoverLabel`). Der Parameter für die `excludeSemantics`-Variante heißt `titleVisibleNearby`. `imageUrl` muss aus der Dateizugriffsschicht (eigener Storage) stammen, nie aus `source_url` oder Fremd-URLs (Datenschutz); Hinweise für #3/#4 (animierte Ladeanzeige und `pumpAndSettle`, signierte URLs bei privatem Bucket, CORS beim Self-Hosting) stehen im Doc-Kommentar des Widgets.
 - Keine Geschäftslogik, keine Domänenmodelle in 2a. Das Domänenmodell `CatalogGame` (freezed) und das Repository legt KAT-01 (#3) an, auf Basis der Spalten von `catalog_games`.

 ## 6. Plattform-Besonderheiten
@ -685,3 +686,7 @@ Chris hat die Spec freigegeben und alle Fragen wie empfohlen entschieden. Eingea
 ### 2026-11-03 – Bestätigung der Designwahlen

 Chris hat W1, W4, W6, W7, H1/H2 und H7 (§ 9.3) einzeln und ohne Änderung bestätigt (Kommentar im Issue #2). § 9.3 trägt jetzt je Zeile den Status „bestätigt (Chris, 2026-11-03)“; die Hinweise „bitte bestätigen“ in Status, E6 und 3.3 sind entfernt; 9.4 entsprechend angepasst.
+
+### 2026-11-03 – Umsetzung PR 2a, Flutter-Anteil
+
+- `GameCover` umgesetzt; Review-Nacharbeit: Semantics-Label folgt dem sichtbaren Zustand, zusätzlicher ARB-Schlüssel `gameCoverLabel`, Parameter `titleVisibleNearby`, `cacheWidth` nach Gerätepixeln, Hinweise für #3/#4 (5).