From ffc30db2e5283e71aa3d9b32f681636099fe0c72 Mon Sep 17 00:00:00 2001 From: cschlaefke Date: Tue, 3 Nov 2026 13:17:09 +0100 Subject: [PATCH] fix(auth): address review findings for semantics split Refs #41 Co-Authored-By: Claude Sonnet 5.5 --- app/lib/core/widgets/app_form_scaffold.dart | 2 + .../auth/presentation/auth_ui_support.dart | 34 +++++++----- .../check_email/check_email_screen_test.dart | 51 ++++++++++-------- .../email_link/email_link_screen_test.dart | 54 +++++++++++-------- .../reset_password_screen_test.dart | 15 ++---- docs/design/auth-screens.md | 2 +- docs/specs/1-auth-registrierung-anmeldung.md | 6 ++- 7 files changed, 94 insertions(+), 70 deletions(-) diff --git a/app/lib/core/widgets/app_form_scaffold.dart b/app/lib/core/widgets/app_form_scaffold.dart index 0db0df1..ecdac5a 100644 --- a/app/lib/core/widgets/app_form_scaffold.dart +++ b/app/lib/core/widgets/app_form_scaffold.dart @@ -38,6 +38,8 @@ class AppFormScaffold extends StatelessWidget { // nodes it folds title, text and buttons of the page into // ONE node (heading + button in one), so screen readers // find no button (issue #41). + // Alternative: Card(semanticContainer: false); the explicit + // wrapper keeps the Card's own container semantics. child: Semantics( explicitChildNodes: true, child: Padding( diff --git a/app/test/features/auth/presentation/auth_ui_support.dart b/app/test/features/auth/presentation/auth_ui_support.dart index 63478b6..44746a2 100644 --- a/app/test/features/auth/presentation/auth_ui_support.dart +++ b/app/test/features/auth/presentation/auth_ui_support.dart @@ -128,6 +128,24 @@ List visibleTexts(WidgetTester tester) => [ /// Wide layout (>= 600 dp): the screen sits in a card (issue #41). const wideScreenSize = Size(1280, 720); +/// Asserts that [target] is an enabled button node of its own with [label]. +/// +/// `isSemantics` only checks the given properties, so extra flags +/// (focusable, ...) added by future Flutter versions do not break it. +void expectButtonSemantics(WidgetTester tester, Finder target, String label) { + expect( + tester.getSemantics(target), + isSemantics( + label: label, + isButton: true, + isEnabled: true, + hasEnabledState: true, + hasTapAction: true, + ), + reason: 'button must be a button node of its own', + ); +} + /// Asserts that title, body text and button are separate semantics nodes /// (heading / text / button with label), not folded into one node (issue #41). void expectSeparateSemantics( @@ -140,25 +158,15 @@ void expectSeparateSemantics( }) { expect( tester.getSemantics(find.text(title).first), - matchesSemantics(label: title, isHeader: true), + isSemantics(label: title, isHeader: true, isButton: false), reason: 'title must be a heading of its own', ); if (bodyKey != null) { expect( tester.getSemantics(find.byKey(bodyKey)), - matchesSemantics(label: bodyLabel), + isSemantics(label: bodyLabel, isHeader: false, isButton: false), reason: 'body text must be a text node of its own', ); } - expect( - tester.getSemantics(find.byKey(buttonKey)), - matchesSemantics( - label: buttonLabel, - isButton: true, - isEnabled: true, - hasEnabledState: true, - hasTapAction: true, - ), - reason: 'button must be a button node of its own', - ); + expectButtonSemantics(tester, find.byKey(buttonKey), buttonLabel); } diff --git a/app/test/features/auth/presentation/check_email/check_email_screen_test.dart b/app/test/features/auth/presentation/check_email/check_email_screen_test.dart index 5df656d..312bdbb 100644 --- a/app/test/features/auth/presentation/check_email/check_email_screen_test.dart +++ b/app/test/features/auth/presentation/check_email/check_email_screen_test.dart @@ -291,6 +291,23 @@ void main() { }); group('QUE-06 (issue #41): separate semantics nodes in the wide card', () { + testWidgets('narrow layout: heading, text and resend button', ( + tester, + ) async { + final handle = tester.ensureSemantics(); + await open(tester); + + expectSeparateSemantics( + tester, + title: _de.authCheckEmailTitle, + bodyKey: const Key('check_email_body'), + bodyLabel: _de.authCheckEmailBody, + buttonKey: const Key('check_email_resend'), + buttonLabel: _de.authCheckEmailResend, + ); + handle.dispose(); + }); + testWidgets('confirmation: heading, text and resend button', ( tester, ) async { @@ -319,32 +336,22 @@ void main() { expect( tester.getSemantics(find.text(_de.authCheckEmailTitle).first), - matchesSemantics(label: _de.authCheckEmailTitle, isHeader: true), + isSemantics(label: _de.authCheckEmailTitle, isHeader: true), ); expect( tester.getSemantics(find.byKey(const Key('check_email_body'))), - matchesSemantics(label: _de.authCheckEmailBodyReset), + isSemantics(label: _de.authCheckEmailBodyReset, isButton: false), + ); + expectButtonSemantics( + tester, + find.byKey(const Key('check_email_different_address')), + _de.authCheckEmailDifferentAddress, + ); + expectButtonSemantics( + tester, + find.byKey(const Key('check_email_back_to_login')), + _de.authBackToLogin, ); - for (final (key, label) in [ - ( - const Key('check_email_different_address'), - _de.authCheckEmailDifferentAddress, - ), - (const Key('check_email_back_to_login'), _de.authBackToLogin), - ]) { - expect( - tester.getSemantics(find.byKey(key)), - matchesSemantics( - label: label, - isButton: true, - isEnabled: true, - hasEnabledState: true, - hasTapAction: true, - hasFocusAction: true, - isFocusable: true, - ), - ); - } handle.dispose(); }); }); diff --git a/app/test/features/auth/presentation/email_link/email_link_screen_test.dart b/app/test/features/auth/presentation/email_link/email_link_screen_test.dart index 7037872..008aa57 100644 --- a/app/test/features/auth/presentation/email_link/email_link_screen_test.dart +++ b/app/test/features/auth/presentation/email_link/email_link_screen_test.dart @@ -302,7 +302,10 @@ void main() { handle.dispose(); }); - testWidgets('invalid link: banner action is a button of its own', ( + // Checks the banner structure (live region + action button as own + // nodes). This also holds without the card fix, because AppBanner is a + // semantics container itself; the folding tests are the ones above. + testWidgets('invalid link: banner live region and action are own nodes', ( tester, ) async { final handle = tester.ensureSemantics(); @@ -313,20 +316,30 @@ void main() { ); expect( - tester.getSemantics(find.text(_de.authConfirmTitle).first), - matchesSemantics(label: _de.authConfirmTitle, isHeader: true), + find.bySemanticsLabel('Fehler: ${_de.authErrorLinkInvalidOrExpired}'), + findsOneWidget, ); - expect( - tester.getSemantics(find.text(_de.authActionRequestNewLink)), - matchesSemantics( - label: _de.authActionRequestNewLink, - isButton: true, - isEnabled: true, - hasEnabledState: true, - hasTapAction: true, - hasFocusAction: true, - isFocusable: true, - ), + expectButtonSemantics( + tester, + find.text(_de.authActionRequestNewLink), + _de.authActionRequestNewLink, + ); + handle.dispose(); + }); + + testWidgets('narrow layout: heading, text and button are separate nodes', ( + tester, + ) async { + final handle = tester.ensureSemantics(); + await open(tester); + + expectSeparateSemantics( + tester, + title: _de.authConfirmTitle, + bodyKey: const Key('email_link_body'), + bodyLabel: _de.authConfirmBody, + buttonKey: const Key('email_link_submit'), + buttonLabel: _de.authConfirmSubmit, ); handle.dispose(); }); @@ -342,15 +355,10 @@ void main() { find.bySemanticsLabel('Hinweis: ${_de.authConfirmSuccess}'), findsOneWidget, ); - expect( - tester.getSemantics(find.byKey(const Key('email_link_continue'))), - matchesSemantics( - label: _de.authConfirmContinue, - isButton: true, - isEnabled: true, - hasEnabledState: true, - hasTapAction: true, - ), + expectButtonSemantics( + tester, + find.byKey(const Key('email_link_continue')), + _de.authConfirmContinue, ); handle.dispose(); }); diff --git a/app/test/features/auth/presentation/reset_password/reset_password_screen_test.dart b/app/test/features/auth/presentation/reset_password/reset_password_screen_test.dart index d8065dc..e97cca7 100644 --- a/app/test/features/auth/presentation/reset_password/reset_password_screen_test.dart +++ b/app/test/features/auth/presentation/reset_password/reset_password_screen_test.dart @@ -384,21 +384,16 @@ void main() { expect( tester.getSemantics(find.text(_de.authResetTitle).first), - matchesSemantics(label: _de.authResetTitle, isHeader: true), + isSemantics(label: _de.authResetTitle, isHeader: true), ); expect( find.bySemanticsLabel('Hinweis: ${_de.authPasswordChanged}'), findsOneWidget, ); - expect( - tester.getSemantics(find.byKey(const Key('reset_password_continue'))), - matchesSemantics( - label: _de.authConfirmContinue, - isButton: true, - isEnabled: true, - hasEnabledState: true, - hasTapAction: true, - ), + expectButtonSemantics( + tester, + find.byKey(const Key('reset_password_continue')), + _de.authConfirmContinue, ); handle.dispose(); }); diff --git a/docs/design/auth-screens.md b/docs/design/auth-screens.md index 833ad46..b61e82d 100644 --- a/docs/design/auth-screens.md +++ b/docs/design/auth-screens.md @@ -28,7 +28,7 @@ Schlüssel sind `lowerCamelCase` mit Präfix `auth`. mit Label (`AppPrimaryButton`, `TextButton`). Das gilt in jeder Breite, auch in der Karte ab 600 dp. - Ursache des Fehlers: `Card` ist ein Semantics-Container ohne `explicitChildNodes`. Alle Kinder ohne eigenen Container (Titel, `Text`, Knopf) werden in dessen einen Knoten gefaltet. Seiten mit Eingabefeld fielen nicht auf, - weil `AppTextField` & Co. eigene Container sind. Im Web und ab 600 dp trat es auf, auf dem Handy (ohne Karte) nicht. + weil `AppTextField` & Co. eigene Container sind. Der Fehler trat auf allen Plattformen ab 600 dp (Karte) auf und wurde im E2E (Web) sichtbar; auf dem Handy ohne Karte nicht. - Deshalb kapselt `AppFormScaffold` den Karteninhalt in `Semantics(explicitChildNodes: true)`. Neue Layouts mit `Card`/`Semantics(container: true)` um Seiteninhalt müssen das ebenfalls tun; `MergeSemantics` nur gezielt für zusammengehörige Einzelelemente, nie um ganze Seiten. diff --git a/docs/specs/1-auth-registrierung-anmeldung.md b/docs/specs/1-auth-registrierung-anmeldung.md index 938af36..4966f24 100644 --- a/docs/specs/1-auth-registrierung-anmeldung.md +++ b/docs/specs/1-auth-registrierung-anmeldung.md @@ -515,7 +515,7 @@ Nach der Freigabe am 2026-11-02 eingearbeitet. Die Texte der Akzeptanzkriterien ### 2026-11-03 – T3 (Issue #38, Branch `feature/38-auth-e2e`) 1. **7.1 Krit. 9 (web), Rezept korrigiert:** Das Setzen von `expires_at` im `localStorage` erzwingt keinen Refresh, weil `Session.expiresAt` in `supabase_flutter` aus dem JWT-Claim `exp` berechnet wird. Der Test verschiebt stattdessen die Browser-Uhr (`Date`) um 2 h (`shiftBrowserClock` in `e2e/support/web.ts`); der Server rechnet mit der echten Zeit. -2. **A11y-Befund (QUE-06):** Auf `/auth/confirm` und der CheckEmail-Seite faltet der Semantics-Baum Titel, Text und Knopf zu einem Knoten zusammen; die E2E-Tests bedienen den Knopf per Tab und Enter (`pressFirstButton`). +2. **A11y-Befund (QUE-06):** Auf `/auth/confirm` und der CheckEmail-Seite faltet der Semantics-Baum Titel, Text und Knopf zu einem Knoten zusammen; die E2E-Tests bedienen den Knopf per Tab und Enter (`pressFirstButton`). **Behoben in #41:** `AppFormScaffold` kapselt den Karteninhalt mit `explicitChildNodes`, die E2E-Tests klicken über `getByRole('button')` (`pressFirstButton` entfällt). ### 2026-11-03 – Umsetzung 01c (Branch `feature/1-01c-passwort-reset`), Abweichungen zur Freigabe durch Chris @@ -529,3 +529,7 @@ Nach der Freigabe am 2026-11-02 eingearbeitet. Die Texte der Akzeptanzkriterien 1. **Entscheidung Chris (Option 1):** Bei `recover`, `resend` und `signUp` zeigt die App für jeden Fehler, der weder Eingabe- noch Transportfehler noch IP-Drosselung ist (insbesondere HTTP 5xx / `unexpected_failure`), dieselbe neutrale Seite wie bei Erfolg. Umsetzung als Allowlist in `openEmailInputFailureFor` und `resendFailureFor`; der Mapper liefert 5xx mit Status bei diesen Aufrufen als `unexpected` (statt `network`), ein `signUp`-500 ohne bekannte Trigger-Meldung ebenfalls (statt `registrationRejected`). 2. **Login-Resend (Krit. 6), Entscheidung Orchestrator (S4, Chris wird informiert):** Dort bleibt `unexpected` **sichtbar** (allgemeiner Fehler mit „Erneut versuchen“), weil das Konto durch das richtige Passwort bereits bewiesen ist und Neutralisieren nur einen echten SMTP-Ausfall verdecken würde. `emailSendLimited` zeigt dort weiter den Drosselungshinweis (F12). Neutral bleibt es auf CheckEmail-Resend, Forgot und Register. `registrationRejected` ist nur mit bekanntem Grund sichtbar. 3. **4.4, R17, F13** ergänzt; Betriebshinweis in `docs/auth-konfiguration.md`. + +### 2026-11-03 – A11y-Fix Semantics-Split (Issue #41, Branch `fix/41-semantics-split`) + +1. **Ursache und Fix:** Die `Card` im Layout ab 600 dp faltete Titel, Text und Knopf in einen Semantics-Knoten. `AppFormScaffold` setzt `Semantics(explicitChildNodes: true)` um den Karteninhalt (Regel in `docs/design/auth-screens.md`). Der Befund aus T3 (A11y-Befund QUE-06) ist damit behoben; `pressFirstButton` ist entfernt.