diff --git a/ONBOARDING.md b/ONBOARDING.md new file mode 100644 index 0000000..2f46b86 --- /dev/null +++ b/ONBOARDING.md @@ -0,0 +1,177 @@ +# app4training — Onboarding + +Welcome to **app4training** — the offline mobile app for [4training.net](https://www.4training.net), built in Dart/Flutter by [holydevelopers.net](https://holydevelopers.net/). + +This document is the entry point for new contributors. It gives you a high-level mental model of the app and points to deep-dive docs in `docs/` for each subsystem. + +______________________________________________________________________ + +## 1. What this app does (in 60 seconds) + +- **Offline content viewer** for the discipleship/training worksheets hosted at 4training.net. +- Each translation lives in a **separate GitHub repository** (`html-` for HTML and `pdf-` for PDFs). The user picks which languages to download; the app fetches the repo as a zip, unpacks it locally, and renders pages from disk. +- **No backend of our own.** GitHub serves the content, the GitHub Commits API tells us when a language has updates, and the app renders the HTML using `flutter_html`. +- Users can **read worksheets**, **switch translations** of the current page, **share** a PDF or link, and **manage** the languages stored on their device. + +The supported language list, content URLs, and update-check endpoints are all defined in `lib/data/globals.dart` (the `Globals` class). + +______________________________________________________________________ + +## 2. Getting started + +### Prerequisites + +- **Flutter 3.41.6** (pinned in `.fvmrc`). Either use [FVM](https://fvm.app/) or any compatible Flutter SDK. +- **Dart SDK ^3.7.0** (set by `pubspec.yaml`). +- For Android builds: standard Android SDK + emulator. +- For iOS builds: Xcode + CocoaPods. + +### First run + +```bash +flutter pub get +flutter run +``` + +### Pre-commit checks (must pass for CI to be green) + +```bash +dart format . +dart analyze +flutter test +``` + +The CI workflow at `.github/workflows/main.yaml` runs all of these on every push/PR plus an Android emulator integration test. + +> **Note:** the README mentions `dart run custom_lint`. As of the current version, `custom_lint` was removed in favor +> of `riverpod_lint 3.1.3+` which now uses `analysis_server_plugin` directly. There is no separate lint step to run. + +______________________________________________________________________ + +## 3. The 30-second mental model + +``` + +┌───────────────────────────────────────────────────────────────────┐ +│ main.dart │ +│ → installs flutter_html error filter (see main.dart docstring) │ +│ → loads SharedPreferences + PackageInfo │ +│ → wraps App4Training in ProviderScope (Riverpod) │ +└──────────────────────────┬────────────────────────────────────────┘ + │ + ┌────────────▼─────────────┐ + │ MaterialApp │ named routes via + │ initialRoute: '/' │ generateRoutes() + └────────────┬─────────────┘ + │ + ┌──────────────▼───────────────────────────┐ + │ '/' StartupPage → decides where to go │ + │ - first time → /onboarding/1 │ + │ - missing app lang → /onboarding/2 │ + │ - else → /home or /view// │ + └──────────────┬───────────────────────────┘ + │ + ▼ + /home (table of contents) ─► /view// (HtmlView) + /settings /about /onboarding/{1,2,3} + +``` + +State is **all Riverpod** (v3, no codegen). The two big Notifiers are `LanguageController` (per-language download/load) +and `LanguageStatusNotifier` (per-language update check). The rest of state is small Notifiers/Providers around them. + +______________________________________________________________________ + +## 4. Repo layout + +| Path | Purpose | +| --- | --- | +| `lib/main.dart` | Entry point, app widget, error-filter setup | +| `lib/routes/` | One file per route/page. `routes.dart` is the named-route dispatcher | +| `lib/routes/onboarding/` | Three onboarding screens (`/onboarding/1..3`) | +| `lib/widgets/` | Reusable UI: drawer, html viewer, language buttons, dropdowns | +| `lib/features/share/` | Share menu (PDF/link/open in browser) and `ShareService` wrapper | +| `lib/data/` | Domain layer — Riverpod providers + immutable models (Language, Page, etc.) | +| `lib/background/` | `workmanager` background task + scheduler + result detection | +| `lib/design/theme.dart` | Theming (FlexColorScheme red) + small UI constants | +| `lib/l10n/` | Localization: `.arb` source, generated delegates, helper extension | +| `assets/` | Static images (logo, share icons) | +| `test/` | Widget + unit tests, plus fixture data in `assets-de/`, `assets-en/` | +| `integration_test/` | Real-device test for background isolate behavior | +| `android/`, `ios/` | Standard Flutter platform folders | +| `.github/workflows/main.yaml` | CI: format, analyze, test, integration test | + +______________________________________________________________________ + +## 5. Where to read next + +The detailed docs live in `docs/`. Read them in this order if you're new: + +1. **[docs/architecture.md](docs/architecture.md)** — high-level architecture, request flow, providers map. +1. **[docs/dependencies.md](docs/dependencies.md)** — every package in `pubspec.yaml` and what it does. +1. **[docs/state-management.md](docs/state-management.md)** — the Riverpod model: every provider, what it depends on, how it's overridden in tests. +1. **[docs/routing.md](docs/routing.md)** — Navigator 1.0 named routes, deep linking, the route table. +1. **[docs/data-layer.md](docs/data-layer.md)** — `Language`/`Page` models, GitHub download flow, file system layout, exceptions. +1. **[docs/content-rendering.md](docs/content-rendering.md)** — HTML rendering, the `sanitize()` workarounds, `flutter_html_table` filter in `main.dart`. +1. **[docs/onboarding-flow.md](docs/onboarding-flow.md)** — the three onboarding pages and their persistence side-effects. +1. **[docs/features.md](docs/features.md)** — pages, widgets, drawer, share menu, language switcher. +1. **[docs/background-tasks.md](docs/background-tasks.md)** — `workmanager` isolate, the result-sync trick, why parts are commented out. +1. **[docs/localization.md](docs/localization.md)** — `.arb` workflow, `context.l10n` extension, in-app language vs. content language. +1. **[docs/testing.md](docs/testing.md)** — unit/widget tests, fixtures, integration test, CI. +1. **[docs/conventions.md](docs/conventions.md)** — coding conventions, lints, do/don't. + +______________________________________________________________________ + +## 6. Things that will surprise you + +These are the load-bearing oddities — read these before changing code in their area. + +- **Riverpod v3 internal import.** A few places (`languages.dart`, `updates.dart`) import `package:riverpod/src/framework.dart` to access `$RefArg` for `family` providers when overriding in tests. This is intentional, suppressed via `// ignore` comments. +- **`MustOverrideProvider`** in `globals.dart` is a custom helper that throws unless overridden in a `ProviderScope`. It is used for `sharedPrefsProvider` and `packageInfoProvider` so that test environments **must** provide explicit fakes. +- **`flutter_html` error filter.** `main.dart` installs a `FlutterError.onError` shim that swallows four specific assertions thrown by `flutter_html_table` 3.0.0. Read the long docstring there before touching it — the four assertion variants are documented in detail. +- **HTML pre-processing.** `widgets/html_view.dart` calls `sanitize()` to fix bugs in `flutter_html` (e.g. percent table widths, fuzzy translations, stylized subtitles). Some workarounds are also in the HTML generator (`pywikitools`) upstream. +- **Background task is half-disabled.** Big chunks of `BackgroundScheduler.schedule()` and `Workmanager().initialize` in `main.dart` are commented out, gated on a "version 0.9" milestone. Don't delete them — they are the working scaffolding for the next release. +- **No package on pub.dev.** `pubspec.yaml` has `publish_to: 'none'`. +- **License is AGPL** with an Apple App Store exception. See `LICENSE` and `COPYING.iOS`. +- **`download_assets` is pinned to 4.0.0** because 4.1.0 breaks GitHub archive URLs (Content-Length check). +- **`test` is pinned to ^1.29.0** because 1.30+ needs a newer `test_api` than `flutter_test` allows. + +______________________________________________________________________ + +## 7. Common workflows + +### Add a new language + +1. Add the two-letter code to `availableLanguagesProvider` in `lib/data/globals.dart`. +1. Bump `countAvailableLanguages` in the same file. +1. Add a `language_` entry to both `lib/l10n/locales/app_en.arb` and `app_de.arb`. +1. Add the code to the map in `lib/l10n/l10n.dart` (`getLanguageName`). +1. Confirm `https://github.com/4training/html-` and `pdf-` repos exist and follow the standard structure. +1. If the language is RTL, add it to `Globals.rtlLanguages` in `globals.dart`. + +### Add a new worksheet category or worksheet + +- Worksheet categories: edit `Category` enum and the `worksheetCategories` map in `lib/data/categories.dart`. Add localized strings for the category name to both `.arb` files. +- The actual worksheets come from each language repo's `structure/contents.json` — they aren't listed in app code. + +### Add a new route + +- Add a builder branch in `generateRoutes()` (`lib/routes/routes.dart`). +- Place the page widget in `lib/routes/`. +- Update `docs/routing.md`. + +### Run a single test + +```bash +flutter test test/view_page_test.dart +``` + +______________________________________________________________________ + +## 8. Roadmap context (from README) + +- **0.9**: enable automatic background updates (scaffolding present but commented out). +- **1.0**: solid release. +- iOS planned for 2024. + +If you see code with `TODO for version 0.9`, that's why. diff --git a/README.md b/README.md index a0ce2e7..6809bbf 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,10 @@ This app is an offline version of the website [4training.net](www.4training.net) Both, the app and the website are projects by [holydevelopers.net](https://holydevelopers.net/). +## Onboarding + +For a detailed onboarding documentation go to [this file](/ONBOARDING.md) + ## Architecture * For navigation we use the Navigator with named routes ("Navigator 1.0", no routing package). See the folder [lib/routes/](lib/routes), especially the [overview of the route names](lib/routes/routes.dart). * For viewing a page, we use route names that are the same as on the [4training.net](https://www.4training.net) website: `/view/PageName/languageCode`, e.g. `/view/Dealing_with_Money/de` for viewing https://www.4training.net/Dealing_with_Money/de @@ -39,7 +43,6 @@ For formatting code, we follow the dart guidelines by using the standard [dart f Before committing, please run the following commands and make sure they don't show any issues so that our tests will pass: * `dart format .` * `dart analyze` -* `dart run custom_lint` * `flutter test` ## Contributing diff --git a/docs/architecture.md b/docs/architecture.md new file mode 100644 index 0000000..7c76785 --- /dev/null +++ b/docs/architecture.md @@ -0,0 +1,155 @@ +# Architecture + +This document explains how `app4training` is wired end-to-end: + +- process model, +- layers, +- data flow, +- and where to look for each concern. + +## Process and platform model + +`app4training` is a single-process Flutter app with **one auxiliary background isolate** (Android `workmanager`). +There is no app-owned backend; all content comes from public GitHub repos and the GitHub REST API. + +- **Main isolate**: + - UI, + - State (Riverpod `ProviderScope`), + - Reads/writes to local file system and `SharedPreferences`. +- **Background isolate**: + - Spawned by `workmanager`. Runs `backgroundTask()` in `lib/background/background_task.dart`, + builds its own `ProviderContainer`, checks each downloaded language for new GitHub commits, + persists results to `SharedPreferences`. + The main isolate later detects the activity by reloading shared prefs (see `BackgroundResultNotifier.checkForActivity`). + +> **Note:** Currently the periodic registration of the background task is **commented out** (gated behind "version 0.9"). +> The task is wired up and tested in the integration test, but is not actually being scheduled at app launch. + +## Layered overview + +``` +┌──────────────────────────────────────────────────────────────┐ +│ Presentation │ +│ lib/routes/ → screens (StartupPage, HomePage, ViewPage…) │ +│ lib/widgets/ → MainDrawer, HtmlView, dropdowns, buttons │ +│ lib/features/share/ → Share menu + ShareService │ +│ lib/design/ → ThemeData (FlexColorScheme red) │ +└────────────────────────┬─────────────────────────────────────┘ + │ reads via Riverpod (WidgetRef) + ▼ +┌──────────────────────────────────────────────────────────────┐ +│ State (Riverpod v3, no codegen) │ +│ lib/data/globals.dart → app-wide providers + constants │ +│ lib/data/app_language.dart → AppLanguage notifier │ +│ lib/data/languages.dart → languageProvider (family) │ +│ lib/data/updates.dart → languageStatusProvider (family)│ +│ lib/background/background_scheduler.dart │ +│ lib/background/background_result.dart │ +└────────────────────────┬─────────────────────────────────────┘ + │ + ┌────────────────┼─────────────────────┐ + ▼ ▼ ▼ +┌───────────────┐ ┌──────────────┐ ┌──────────────────────┐ +│ File system │ │ SharedPrefs │ │ Network │ +│ (download_ │ │ user prefs + │ │ - download_assets │ +│ assets) │ │ last-checked │ │ (zip download) │ +│ HTML + PDF │ │ timestamps │ │ - http (GitHub API) │ +│ per language │ │ │ │ commits since… │ +└───────────────┘ └──────────────┘ └──────────────────────┘ +``` + +The split is deliberately thin — there is no service/repository layer between providers and IO. +The Riverpod `Notifier` *is* the repository. +This keeps the codebase small at the cost of putting IO directly inside providers, +which is mitigated by overriding `fileSystemProvider` and `httpClientProvider` in tests. + +## Lifecycle + +1. **`main()`** (`lib/main.dart`) + + - `WidgetsFlutterBinding.ensureInitialized()` + - `_installHtmlTableSemanticsFilter()` — wraps `FlutterError.onError` to filter four known non-fatal assertions from `flutter_html_table` (see [content-rendering.md](content-rendering.md)). + - Loads `SharedPreferences` and `PackageInfo` synchronously. + - `runApp(ProviderScope(overrides: [...], child: App4Training()))`. + +1. **`App4Training`** widget + + - Watches `appLanguageProvider` for the current locale. + - Builds `MaterialApp` with `initialRoute: '/'`, `onGenerateRoute: generateRoutes`, + light/dark themes from `lib/design/theme.dart`, and a `scaffoldMessengerKey` + that lets us show snackbars from anywhere via `scaffoldMessengerProvider`. + +1. **`StartupPage`** (`/`) + + - Decides where to navigate based on persisted state in `SharedPreferences`. See [onboarding-flow.md](onboarding-flow.md). + +1. **`/view//`** + + - The "main" view. Loads HTML from disk via `pageContentProvider`, also opportunistically calls `BackgroundResultNotifier.checkForActivity()` to detect work done in the background isolate, persists `recentPage`/`recentLang` so the next launch can resume. + +## Data flow: viewing a worksheet + +``` +ViewPage(page, langCode) + → checkAndLoad() + ├─ ref.read(backgroundResultProvider.notifier).checkForActivity() + │ • reload SharedPreferences + │ • compare lastChecked- on disk vs in-memory LanguageStatus + │ • invalidate stale languageStatusProviders + │ • show snackbar if activity found + └─ ref.watch(pageContentProvider((name, langCode))).future + • watches Language(langCode) for the on-disk path + • reads / from FileSystem + • inlines images by replacing + with + • throws LanguageNotDownloadedException / PageNotFoundException / + LanguageCorruptedException for the matching cases + +→ HtmlView(content, direction) + • sanitize(content, isDarkMode) ← workarounds for flutter_html bugs + • Html.fromDom(...) with TagWrapExtension({'table'}) + + TableHtmlExtension; tables get wrapped in a horizontal scroll view + • onAnchorTap pushes /view so internal worksheet links navigate +``` + +## Data flow: downloading a language + +``` +DownloadLanguageButton(langCode).onPressed + → LanguageController(langCode).download(force?) + • _initController() → DownloadAssetsController.init(assetDir: 'assets-') + • startDownload([htmlZipUrl]) ← github.com/4training/html-/archive/main.zip + • startDownload([pdfZipUrl]) ← github.com/4training/pdf-/archive/main.zip + • _load(): + – read structure/contents.json + – build pages: Map, pageIndex: List, + images: Map, pdf paths + – compute disk usage + – read modified timestamp of contents.json (UTC) → downloadTimestamp + – emit new Language state + → snackbar, button stops spinning +``` + +The two zip downloads are issued sequentially because `download_assets` errors when both URLs share a filename (`main.zip`). See `LanguageController._download`. + +## Data flow: checking for updates + +``` +CheckNowButton or LanguageStatusNotifier.check() + → GET https://api.github.com/repos/4training/html-/commits?since= + (httpClientProvider) + → 200: count commits, persist to SharedPrefs: + lastChecked-= + updatesAvailable-= + emit new LanguageStatus(updatesAvailable, downloadTimestamp, now) + → 403: API rate-limited (60/h unauthenticated) — return apiRateLimitExceeded +``` + +The background isolate runs the same `LanguageStatusNotifier.check()`, just with a freshly-built `ProviderContainer` overriding `sharedPrefsProvider`. When the main isolate later reloads SharedPreferences, it sees the fresher `lastChecked-*` and emits `BackgroundResult(foundActivity: true)` plus a snackbar. + +## Cross-cutting concerns + +- **Localization.** `flutter_localizations` + `intl`. `.arb` files in `lib/l10n/locales/` are the source; `flutter gen-l10n` (run automatically by `flutter pub get` because `generate: true` in `pubspec.yaml`) produces `lib/l10n/generated/`. Access via `context.l10n.` (the extension lives in `lib/l10n/l10n.dart`). See [localization.md](localization.md). +- **Snackbars from anywhere.** `scaffoldMessengerKeyProvider` exposes a `GlobalKey` that `MaterialApp` uses; `scaffoldMessengerProvider` resolves it to the current state, so async callbacks without a `BuildContext` can show snackbars. +- **Errors.** Domain errors are subclasses of `App4TrainingException` (`lib/data/exceptions.dart`) and have `toLocalizedString(BuildContext)`. `ViewPage` unwraps Riverpod v3 `ProviderException` to get the original. +- **Theme.** `lib/design/theme.dart` builds light + dark themes from `FlexScheme.red`, with a custom darker primary red and a centered bold AppBar title. diff --git a/docs/background-tasks.md b/docs/background-tasks.md new file mode 100644 index 0000000..8b5a9a5 --- /dev/null +++ b/docs/background-tasks.md @@ -0,0 +1,104 @@ +# Background Tasks + +The app has a `workmanager`-based periodic background task that **checks** for content updates while the app is closed. Note: as of v0.8 the *scheduling* of this task is **disabled** (commented out, gated for v0.9). The task implementation is complete and integration-tested; only the registration is dormant. + +## What runs in the background + +### Entry point: `backgroundTask()` (`lib/background/background_task.dart`) +```dart +@pragma('vm:entry-point') +void backgroundTask() { + Workmanager().executeTask((task, inputData) async { + if (task == 'testTask') { + await backgroundTestMain(); // integration test path + IsolateNameServer.lookupPortByName('test')?.send('success'); + } else { + await backgroundMain(); + } + return Future.value(true); + }); +} +``` + +`@pragma('vm:entry-point')` is required so tree-shaking doesn't remove the function — `workmanager` reaches it through the platform channel by name. + +### `backgroundMain()` +1. Gets a fresh `SharedPreferences` instance (the background isolate has its own memory). +2. Builds a new `ProviderContainer` with `sharedPrefsProvider` overridden. +3. Calls `backgroundCheck(ref)`. +4. Currently, the task only checks for updates, never auto-downloads. + +### `backgroundCheck(ProviderContainer ref)` +For each language code in `availableLanguagesProvider`: +1. `languageProvider(code).notifier.lazyInit()` — minimal disk check, no JSON parse. +2. Skip if not downloaded. +3. `languageStatusProvider(code).check()` — same GitHub Commits API call as the foreground. +4. Bail out of the loop if the rate limit (`apiRateLimitExceeded`) is hit. + +### Debug logging +`writeLog(message)` appends to `/background.log` so the integration test (and human debuggers) can confirm the task ran. Marked `TODO: Remove later`. + +## Scheduling (`lib/background/background_scheduler.dart`) + +The current code is essentially: + +```dart +class BackgroundScheduler extends Notifier { + @override + bool build() => false; + + Future schedule() async { + /* TODO Enable this with version 0.9 + cancelByUniqueName('backgroundTask') + interval = checkFrequency.getDuration() // null if 'never' + if interval == null: state = false; return + Workmanager().registerPeriodicTask('backgroundTask', 'backgroundTask', + constraints: Constraints(networkType: NetworkType.connected), + initialDelay: interval ~/ 2) + state = true + */ + } +} +``` + +`schedule()` is called from three places (and is currently a no-op): +- `StartupPage.init()` after a successful startup, +- `SetUpdatePrefsPage` after the user submits onboarding step 3, +- `CheckFrequencyNotifier.setCheckFrequency` whenever the user changes the frequency. + +The `Workmanager().initialize(backgroundTask, isInDebugMode: false)` call in `main()` is also commented out behind the same `TODO enable in version 0.9`. + +## How the foreground learns about background work + +There is **no IPC** between isolates. They communicate via `SharedPreferences` (which is backed by a platform plugin storing data on disk). + +- The background task writes: + - `lastChecked-` (UTC, ISO-8601), + - `updatesAvailable-` (bool). + +- The foreground later: + 1. `BackgroundResultNotifier.checkForActivity()` is called from `ViewPage.checkAndLoad`. + 2. `await sharedPrefsProvider.read.reload()` — `SharedPreferences` caches values aggressively, so we have to force a re-read from disk. + 3. For each downloaded language, parse `lastChecked-` from prefs and compare with the in-memory `LanguageStatus.lastCheckedTimestamp`. If the persisted value is newer, **`ref.invalidate(languageStatusProvider())`** so it rebuilds from the fresh persisted values. + 4. If any activity was detected, return `true` and the caller shows the `foundBgActivity` snackbar. + +There's a comment in `BackgroundResultNotifier.checkForActivity`: "*languageStatusProviders must have been initialized already before, otherwise they're loading their lastChecked times from sharedPrefs now and can't detect any background activity.*" The integration test deliberately opens and closes the settings page first (which mounts `LanguagesTable` and pre-warms `languageStatusProvider` for every language) before triggering the background task. + +## Integration test — `integration_test/background_interaction_test.dart` + +Two tests, both running on a real Android emulator (CI uses `reactivecircus/android-emulator-runner@v2`, API level 29): + +1. **"Test that background task gets executed"** + - `Workmanager().initialize(backgroundTask, isInDebugMode: false)`. + - `registerOneOffTask(..., 'testTask', initialDelay: 2s)` — `task` argument is what gets passed to `executeTask` and what selects the `backgroundTestMain()` branch. + - Main isolate registers a port via `IsolateNameServer.registerPortWithName(port.sendPort, 'test')`. + - `backgroundTask` sends `'success'` via `IsolateNameServer.lookupPortByName('test')`. + - Main isolate awaits the port message with a 10-second timeout. + +2. **"Test synchronization with main isolate"** — full happy-path: mount `App4Training` with `appLanguage='de'`, open settings to warm `languageStatusProvider`, fire the background task, then open a worksheet and verify the `foundBgActivity` snackbar appears. + +The fixtures use `MemoryFileSystem` and `MockDownloadAssetsController` from `lib/background/background_test.dart`. + +## Why the test fixtures live in `lib/` + +`background_test.dart` lives in `lib/background/` rather than `test/` because the integration test imports it through the production `background_task.dart` path (`backgroundTestMain` calls `createTestFileSystem()` and `createMockDownloadAssetsController()` from inside the isolate). Test code under `test/` can't be imported by code under `lib/`, so the helpers have to be co-located with production. There's a comment to that effect at the top of the file. diff --git a/docs/content-rendering.md b/docs/content-rendering.md new file mode 100644 index 0000000..5b2c9e2 --- /dev/null +++ b/docs/content-rendering.md @@ -0,0 +1,86 @@ +# Content Rendering + +How worksheet HTML actually shows up on screen — the `flutter_html` configuration, the `sanitize()` workarounds, the global error filter, and the special-cases each addresses. + +## Pipeline + +``` +ViewPage + → pageContentProvider returns String content (with base64 images already inlined) + → HtmlView(content, direction) + Padding → SingleChildScrollView → Column → SelectionArea + → Directionality(LTR | RTL based on Globals.rtlLanguages) + → Html.fromDom( + document: sanitize(content, isDarkMode), + extensions: [TagWrapExtension({'table'}, …), TableHtmlExtension()], + style: { body, td, th, h1, h2, h3, li, p, ul }, + onAnchorTap: (url, _, __) => Navigator.pushNamed(context, '/view$url'), + ) +``` + +## `sanitize()` (`lib/widgets/html_view.dart`) + +The HTML emitted by the upstream `pywikitools` exporter has a few constructs that `flutter_html` can't render correctly. `sanitize()` runs a tree-rewrite step over the parsed DOM to fix them. Each rewrite is documented in-place, but here is the index: + +| Pattern in source HTML | What `sanitize()` does | Why | +| --- | --- | --- | +| `
` | Inlines the `
` content | flutter_html-3.0.0 with `
` inside `` makes the page unusable (issue #170) | +| `

` | Inlines the `` content | Same root cause | +| `

X

` | → `X` | flutter_html_table issue #1188 | +| `

X

` | → `X` | Same | +| `
  • x
` | → `• x
…` | Same — list items inside cells break layout | +| `` | strip `style` | confuses width computation | +| `` | strip `style` and `width` attrs | flutter_html-3.0.0 reads `100%` as `100 px` (CssBoxWidget._computeSize doesn't resolve percent units) — the table would overflow by hundreds of pixels | +| `

X

` | → `

X

` | so subtitles render at proper size | +| `

` | → `
` | flutter_html doesn't support CSS `float`; this is the workaround used by "God's Story (five fingers)" | +| Dark mode + `
` | swap colors → `#090909` + `solid white` | so the box stays readable in dark mode | + +> **Note**: There are several `// FIXME` comments for fixing these in the HTML generator +> (`pywikitools`) instead, since fixing `flutter_html` is out of scope. + +## `Html.fromDom` configuration + +- **Extensions** (order matters): `TagWrapExtension({'table'}, …)` must come **before** `TableHtmlExtension()`. The wrap extension matches `` first, delegates the inner build to the table extension via `prepareFromExtension(extensionsToIgnore: {this})`, and wraps the result in `_HorizontalTableScroll`. If the order were reversed, the table extension would win and the wrap would never apply, leaving wide tables to overflow or trigger "RenderBox was not laid out" assertions. +- **`_HorizontalTableScroll`** is a small `StatefulWidget` that gives the inner `SingleChildScrollView(scrollDirection: Axis.horizontal)` an explicit `ScrollController` (because it lives inside a `WidgetSpan` where `PrimaryScrollController` doesn't apply) and forces `Scrollbar.thumbVisibility: true` so users see when a table is wider than the screen. +- **Style overrides** for `body`, `td`, `th`, `h1`, `h2`, `h3`, `li`, `p`, `ul` to tune spacing and alignment. +- **`onAnchorTap`**: any `` link routes through `Navigator.pushNamed(context, '/view/Foo/de')`. This is what makes inline links between worksheets work. + +## `_installHtmlTableSemanticsFilter()` (`lib/main.dart`) + +A long, well-documented function that wraps `FlutterError.onError` and silently drops known-non-fatal assertions thrown by `flutter_html_table` 3.0.0. Read the docstring before changing anything here. + +The filter matches **two conditions, both required**: + +**(a) Exception message contains one of:** +- `RenderBox was not laid out` +- `computeDryBaseline` +- `renderBoxDoingDryBaseline` +- `'child!.hasSize'` + +**(b) Stack contains one of:** +- `flutter_html/` +- `flutter_layout_grid/` +- `flushSemantics` + +The four assertion variants are all inside `flutter_html_table` 3.0.0's `LayoutBuilder` / `LayoutGrid` / `WidgetSpan` composition. Variant 2 (semantics-pass) has a pure-framework stack (no package frames), so we additionally match on `flushSemantics`. + +Anything that fails either condition is forwarded to the previous handler (typically `FlutterError.presentError`), so unrelated regressions are *not* masked. + +A single breadcrumb is emitted via `debugPrint` on the first suppression per app run, so logs show the filter is active. Subsequent suppressions are silent to avoid log spam. + +**Why this is in `main.dart` rather than the widget layer:** +1. The assertions don't reproduce in `flutter_test`, so a widget-level fix can't be regression-tested. +2. The root cause is third-party (`flutter_html_table`). +3. Wrapping tables in `SelectionContainer.disabled` was tried and broke text selection without fixing the assertion. + +> **Note:** The proper fix is a newer upstream `flutter_html_table`. +> Until then this filter keeps the logs usable. + +## Selection and direction + +- The whole rendered area sits inside a single top-level `SelectionArea`, so users can copy worksheet text. (One earlier attempt to wrap tables in `SelectionContainer.disabled` was reverted because it broke selection across the page.) +- `Directionality` is set to `RTL` for languages in `Globals.rtlLanguages` (currently `['ar', 'fa']`). Adding a new RTL language requires updating that list. + +## Image handling + +Images are inlined as base64 by `pageContentProvider` *before* `HtmlView` ever sees the HTML. This avoids `flutter_html` having to load `file://`-prefixed URIs (which is fragile on Android/iOS). The downside is the rendered HTML can be large — for image-heavy worksheets like "God's Story (five fingers)" the inlined string can be hundreds of KB. So far this hasn't been a problem in practice. diff --git a/docs/conventions.md b/docs/conventions.md new file mode 100644 index 0000000..d50f10e --- /dev/null +++ b/docs/conventions.md @@ -0,0 +1,69 @@ +# Conventions + +The unwritten-but-consistent patterns you'll see across the codebase. Follow these unless you have a good reason not to. + +## Style and lints + +- **`dart format .`** — required before commit. CI doesn't yet `--set-exit-if-changed` (there's a TODO about a strange formatting issue), but `dart format` should produce no diff in normal cases. +- **`dart analyze`** — must be clean. +- **`flutter_lints` + extras** — `analysis_options.yaml` enables: `unawaited_futures`, `avoid_void_async`, `always_declare_return_types`, `join_return_with_assignment`, `unnecessary_parenthesis`, `no_literal_bool_comparisons`. A few candidates are commented out and could be enabled later (`prefer_final_in_for_each`, `directives_ordering`, `dead_code`, `use_string_buffers`). +- **Indentation**: 2 spaces. Lines wrap around 80 cols where reasonable. Trailing commas everywhere multi-arg constructors are involved (so `dart format` produces a vertical layout). + +## State + +- Anything mutable is a `Notifier` exposed via `NotifierProvider`. **Don't** introduce `ChangeNotifier`, `setState` for cross-widget state, or `package:provider`. +- Prefer `ref.watch(...)` in `build`; use `ref.read(...)` only inside callbacks or when you specifically don't want to rebuild. +- `Notifier` classes go in `lib/data/` (or the relevant feature folder). +- For new global singletons that must be set in `main()`, follow the `MustOverrideProvider()` pattern in `globals.dart` so tests can't accidentally use a real value. +- If you need an async source of truth, prefer `FutureProvider.family` over manually implementing a `FutureBuilder` in widget code. Set `retry: null` if the throw is intentional and shouldn't be auto-retried by Riverpod v3. + +## Async + UI + +- **Capture `context.l10n`** at the top of any async handler before the first `await` — see the many `final l10n = context.l10n;` lines. `BuildContext` is unsafe across async gaps; `AppLocalizations` is fine to keep. +- **Snackbars from anywhere**: `ref.watch(scaffoldMessengerProvider).showSnackBar(...)`. Don't reach for `ScaffoldMessenger.of(context)` from an async callback. +- **Loading indicators on buttons** that perform a download follow the `_isLoading` `ConsumerStatefulWidget` pattern (see `DownloadLanguageButton`, `UpdateLanguageButton`, `CheckNowButton`). Consider using one of those as a template before inventing a new pattern. + +## Routing + +- Always use `Navigator.pushNamed` (or `pushReplacementNamed`/`popAndPushNamed` as appropriate) — no anonymous `MaterialPageRoute` builders sprinkled around the codebase. New routes go through `generateRoutes()` in `lib/routes/routes.dart`. + +## Files and folders + +- **Pages → `lib/routes/`** (or `lib/routes//` if grouped, like `onboarding/`). +- **Reusable widgets → `lib/widgets/`** as flat `.dart` files. +- **Feature-specific code with multiple files → `lib/features//`** (currently only `share/`). +- **Domain models, providers, exceptions → `lib/data/`**. + +## Localization + +- Every user-facing string goes through `context.l10n.` — never hardcode strings, even in error messages. +- Any new ARB key requires a translation in **both** `app_en.arb` and `app_de.arb`. CI doesn't enforce this, but tests that assert on German strings will catch missing keys. +- `Exception.toString()` should produce a stable English message (use `AppLocalizationsEn()` directly), not a localized one — exceptions get logged. + +## Errors + +- Domain errors extend `App4TrainingException` and implement `Exception`. Override both `toString()` (English) and `toLocalizedString(BuildContext)` (localized). +- UI catches Riverpod v3 `ProviderException` and unwraps `.exception` to inspect the underlying error (see `ViewPage`). + +## Theme and constants + +- App-wide constants live in `Globals` (`lib/data/globals.dart`) or `lib/design/theme.dart`. Don't duplicate `appTitle`, snackbar durations, or smiley sizes locally. +- Don't introduce custom `Color` literals — use `Theme.of(context).colorScheme.*`. Manually-greyed widgets follow the `onSurface.withOpacity(0.12 / 0.38)` pattern from `DownloadLanguagesPage`. + +## Comments + +The repo's existing code is sparing with comments — but where comments exist, they're substantive (e.g. `_installHtmlTableSemanticsFilter` and `sanitize()` have multi-paragraph docstrings explaining the "why"). Match that bar: + +- Don't write comments that restate the code. +- **Do** write comments when the *why* is non-obvious — e.g. why two `download_assets` calls instead of one, why an internal Riverpod import is needed, why a button is intentionally clickable while looking disabled. +- `// FIXME` is used to mark workarounds that should ideally move upstream (`pywikitools`, `flutter_html`). +- `// TODO for version 0.9` marks the v0.9 milestone code. + +## Don'ts + +- **Don't** add a service/repository layer. Riverpod notifiers ARE the repository. +- **Don't** reintroduce `custom_lint` (removed for analyzer-version conflict). +- **Don't** unpin `download_assets` or `test` without first reading the comments in `pubspec.yaml`. +- **Don't** delete commented-out v0.9 code without project agreement — it's the working scaffold for the next release. +- **Don't** introduce a navigation library (`go_router`, etc.) — Navigator 1.0 is intentional and it interplays cleanly with the URL-mirroring `/view//` scheme. +- **Don't** delete the `flutter_html_table` error filter — the four assertion variants are real and well-documented. diff --git a/docs/data-layer.md b/docs/data-layer.md new file mode 100644 index 0000000..7eeb1a1 --- /dev/null +++ b/docs/data-layer.md @@ -0,0 +1,178 @@ +# Data Layer + +Everything in `lib/data/` and the file-system / GitHub interactions it owns. + +## On-device file system layout + +When a language is downloaded, `download_assets` creates a per-language assets directory (whose path is platform-defined; the Riverpod `LanguageController` doesn't care about the absolute path). Inside it: + +``` +assets-/ +├── html--main/ ← unzipped from html-.zip +│ ├── structure/ +│ │ └── contents.json ← the source of truth (see schema below) +│ ├── files/ ← images referenced by worksheets +│ │ ├── foo.png +│ │ └── ... +│ ├── .html ← one file per worksheet (translated filename) +│ ├── .html +│ └── ... +└── pdf--main/ ← unzipped from pdf-.zip + ├── .pdf + └── ... +``` + +The structure is generated by the **`ResourcesBot`** of the [`pywikitools`](https://github.com/4training/pywikitools) repo (specifically `ExportHTML` and `ExportRepository`). The app expects exactly this layout. + +### `structure/contents.json` schema (as consumed) + +```json +{ + "language_code": "de", + "english_name": "German", + "worksheets": [ + { + "page": "Forgiving_Step_by_Step", // English identifier (same on 4training.net) + "title": "Schritte der Vergebung", // translated title + "filename": "Schritte_der_Vergebung.html", // translated filename + "version": "1.3", + "pdf": "Schritte_der_Vergebung.pdf" // optional + }, + ... + ] +} +``` + +`LanguageController._load()` iterates `worksheets[]` to build `Map` keyed by the English `page` name and a `List pageIndex` recording menu order. + +## Models (`lib/data/languages.dart`) + +All models are `@immutable`. + +### `Language` +```dart +class Language { + final String languageCode; // '' if not downloaded + final Map pages; // keyed by English page name + final List pageIndex; // menu order (subset of pages.keys) + final Map images; // by filename + final String path; // local path to html--main/ + final int sizeInKB; + final DateTime downloadTimestamp; // UTC, taken from contents.json mtime + bool get downloaded => languageCode != ''; +} +``` + +### `Page` +```dart +class Page { + final String name; // English identifier + final String title; // translated title + final String fileName; // translated HTML filename + final String version; + final String? pdfPath; // full path to PDF (or null if absent) +} +``` + +### `Resource` typedef +```dart +typedef Resource = ({String name, String langCode}); +``` +The key for the family providers `pageContentProvider` and `imageContentProvider`. + +## Remote URLs (`Globals` in `lib/data/globals.dart`) + +```dart +const githubUser = '4training'; +const branch = 'main'; +const htmlPath = 'html'; +const pdfPath = 'pdf'; + +getRemoteUrlHtml(lang) → https://github.com/4training/html-/archive/refs/heads/main.zip +getRemoteUrlPdf(lang) → https://github.com/4training/pdf-/archive/refs/heads/main.zip +getResourcesDir(lang) → 'html--main' +getPdfDir(lang) → 'pdf--main' +getAssetsDir(lang) → 'assets-' +getCommitsSince(lang, t)→ https://api.github.com/repos/4training/html-/commits?since= +``` + +The branch name (`main`) is hardcoded; switching branches would require a code change. + +## Download flow + +`LanguageController.download({force=false})`: + +1. (force) `deleteResources()` → `_controller.clearAssets()` and reset state to empty. +2. `_download()`: + - `DownloadAssetsController.startDownload([htmlZipUrl])` — fetches and unzips into `assets-/`. + - Then `startDownload([pdfZipUrl])` — same. **Two separate calls** because `download_assets` errors when both URLs share the filename `main.zip`. + - On any throw, calls `_controller.clearAssets()` so the next attempt starts clean. +3. `_load()`: + - Recompute `assetsDirAlreadyExists()`. + - Sum file sizes recursively → `sizeInKB`. + - Read `contents.json` mtime → `downloadTimestamp` (UTC). + - Parse `contents.json` worksheets, build `pages`, `pageIndex`. + - Scan `pdf--main/` for `.pdf` files; match each `worksheet.pdf` → `Page.pdfPath`. + - Scan `html--main/files/` for images; build `Map`. + - `_checkConsistency()` warns about HTML files referenced but missing, or HTML files present but unreferenced. + - On any throw: log, call `_controller.clearAssets()`, reset to empty `Language`, return `false`. + +`init()` calls `_load()` only — no network. `lazyInit()` only checks for `contents.json` existence and returns a sparse `Language(languageCode, {}, [], {}, path, 0, timestamp)` without parsing — used by the background isolate which doesn't need page details. + +## `pageContentProvider` + +```dart +FutureProvider.family((ref, page) async { … }, retry: null) +``` + +Behaviour: +- Reads `/` as a string. +- Replaces `` with `` by base64-encoding the local file via `imageContentProvider`. (Inlining is necessary because `flutter_html` can't load arbitrary local file URIs.) +- Throws: + - `LanguageNotDownloadedException(langCode)` if the language is gone. + - `PageNotFoundException(name, langCode)` if the page isn't in `pages`. + - `LanguageCorruptedException(langCode, msg, [exception])` for any `FileSystemException`. + +`retry: null` is set so Riverpod v3 doesn't auto-retry these intentional throws. + +## Update-check flow (`lib/data/updates.dart`) + +`LanguageStatusNotifier.check()`: +1. `GET https://api.github.com/repos/4training/html-/commits?since=`. +2. On 200: `commits = json.decode(body).length`. Persist: + - `lastChecked-` = now (UTC, ISO 8601), + - `updatesAvailable-` = `commits > 0`. + Emit new `LanguageStatus(commits>0, downloadTimestamp, now)` and return `commits`. +3. On 403: return `apiRateLimitExceeded` (-403). +4. On any other status or throw: return `-1`. + +`LanguageStatusNotifier.build()`: +- If language is **not downloaded**: clear `updatesAvailable-` and `lastChecked-` from prefs and return a zero status. +- Otherwise: load timestamps from prefs. If `lastChecked < downloadTimestamp` (i.e. user just deleted+re-downloaded), reset both. If `lastChecked` is in the future, reset to `downloadTimestamp`. + +## Exceptions (`lib/data/exceptions.dart`) + +All extend `App4TrainingException` and have `toLocalizedString(BuildContext)` returning a localized user-facing string. + +- `LanguageNotDownloadedException(langCode)` — soft: just download. +- `PageNotFoundException(page, langCode)` — soft: probably broken link. +- `LanguageCorruptedException(langCode, msg, [Exception?])` — hard: delete + redownload. + +`toString()` uses `AppLocalizationsEn` (always English) so the exception is meaningful in logs even when no `BuildContext` is around. `toLocalizedString` uses `context.l10n` for UI surfaces. + +## Categories (`lib/data/categories.dart`) + +The drawer groups worksheets into four hardcoded categories: + +```dart +enum Category { + essentials, + essentialsForTrainers, + innerHealing, + innerHealingForTrainers; +} +``` + +`worksheetCategories` is a const `Map` mapping each English page name to its category. Adding a new worksheet that should appear in the drawer requires updating this map; otherwise it won't show up. + +There's a `// TODO excluded the following worksheet…` line for `Four_Kinds_of_Disciples` because of a rendering bug. diff --git a/docs/dependencies.md b/docs/dependencies.md new file mode 100644 index 0000000..00a8a6e --- /dev/null +++ b/docs/dependencies.md @@ -0,0 +1,84 @@ +# Dependencies + +Snapshot of the dependency tree as of `pubspec.yaml`. Group-by-purpose, with notes about why each is here and any pinned-version gotchas. + +## Runtime dependencies + +### Framework +| Package | Why | +| --- | --- | +| `flutter` (sdk) | Of course | +| `flutter_localizations` (sdk) | Provides Material/Cupertino/Widget localization delegates | +| `intl: any` | Date formatting (`DateFormat`) on the settings page; required for generated l10n | + +### State management +| Package | Why | +| --- | --- | +| `flutter_riverpod: ^3.3.1` | Riverpod widgets (`ConsumerWidget`, `ProviderScope`, `WidgetRef`) | +| `riverpod: ^3.2.1` | Core Riverpod. **Note**: a few places import `package:riverpod/src/framework.dart` for `$RefArg` to access the family argument inside a `Notifier.build()` — this is a known v3 internal escape hatch | + +### Content download / file system +| Package | Why | Notes | +| --- | --- | --- | +| `download_assets: 4.0.0` | Downloads + unzips the GitHub `archive/main.zip` per language | **Pinned to 4.0.0** — comment in `pubspec.yaml`: "the Content-Length check breaks GitHub archive URLs in 4.1.0" | +| `path_provider: ^2.0.11` | Used in `background_task.dart` (`getApplicationDocumentsDirectory()`) for the debug log file | +| `path: ^1.8.2` | `join()` for cross-platform paths | +| `file: ^7.0.0` | Abstract `FileSystem` API. Backs `fileSystemProvider`, lets tests inject `MemoryFileSystem` | +| `http: ^1.0.0` | The GitHub Commits API call (`Globals.getCommitsSince`). Backs `httpClientProvider` | +| `dio: ^5.0.0` | Transitive dependency surfaced in `pubspec.yaml`; used inside `download_assets` and referenced from tests via the `Dio` type | + +### Persistence +| Package | Why | +| --- | --- | +| `shared_preferences: ^2.2.0` | Stores app language, recent page, last-checked timestamps, updatesAvailable booleans, automaticUpdates / checkFrequency settings | + +### HTML rendering +| Package | Why | +| --- | --- | +| `flutter_html: ^3.0.0` | Renders worksheet HTML inside the app | +| `flutter_html_table: ^3.0.0` | `
` extension. Buggy in 3.0.0 — see error filter in `main.dart` | +| `html: ^0.15.4` | Parser used by `sanitize()` in `widgets/html_view.dart` to pre-process the DOM | + +### UI extras +| Package | Why | +| --- | --- | +| `flex_color_scheme: ^8.4.0` | Generates the red Material 3 themes | +| `url_launcher: ^6.3.2` | "Open in browser" + clickable links on About page | +| `flutter_linkify: ^6.0.0` | Auto-link URLs in the localized strings on the About page | +| `share_plus: ^13.0.0` | Native share dialog (PDF and link sharing) | +| `open_filex: ^4.4.0` | Open a downloaded PDF with the user's PDF app | +| `package_info_plus: ^10.0.0` | App version shown on the About page | + +### Background work +| Package | Why | +| --- | --- | +| `workmanager: ^0.9.0+3` | Periodic background isolate that calls `backgroundTask()` | + +### Test-time helpers shipped as runtime deps +These are listed under `dependencies:` rather than `dev_dependencies:` because some test fixtures live in `lib/background/background_test.dart` (which the integration test imports through the production code path). Keep them where they are. + +| Package | Why | +| --- | --- | +| `test: ^1.29.0` | **Pinned**: 1.30+ needs `test_api > 0.7.9` but `flutter_test` pins `test_api 0.7.9` | +| `mocktail: ^1.0.3` | Used by `MockDownloadAssetsController` etc. | + +## Dev dependencies + +| Package | Why | +| --- | --- | +| `flutter_test` (sdk) | Widget testing | +| `flutter_lints: ^6.0.0` | Standard recommended lints, included from `analysis_options.yaml` | +| `full_coverage: ^1.0.0` | CI step generates `test/full_coverage_test.dart` to ensure all files are imported in coverage | +| `riverpod_lint: ^3.1.3` | Riverpod-specific lints. **Note** in `pubspec.yaml`: 3.1.1+ uses `analysis_server_plugin`, replacing the previous `custom_lint`-based plugin. No `analyzer plugins:` entry is required | +| `integration_test` (sdk) | Backs `integration_test/background_interaction_test.dart` | + +## Tooling configuration + +- **Flutter version**: pinned to `3.41.6` in `.fvmrc`. Use FVM if you have it; otherwise any compatible Flutter SDK works. +- **Dart SDK**: `^3.7.0` (from `pubspec.yaml`). +- **Localization generation**: `generate: true` in `pubspec.yaml` plus `l10n.yaml` triggers `flutter gen-l10n` automatically when running `flutter pub get`. +- **Lints**: `analysis_options.yaml` includes `package:flutter_lints/flutter.yaml` plus a few extras: `unawaited_futures`, `avoid_void_async`, `always_declare_return_types`, `join_return_with_assignment`, `unnecessary_parenthesis`, `no_literal_bool_comparisons`. + +## Removed: `custom_lint` + +A note in `pubspec.yaml` records that `custom_lint` was removed because `custom_lint 0.8.1` requires `analyzer ^8` while `riverpod_lint >=3.1.1` requires `analyzer ^9`. Since `riverpod_lint 3.1.3` migrated to `analysis_server_plugin`, the separate `custom_lint` runner is no longer needed. **Don't add it back unless you understand why.** diff --git a/docs/features.md b/docs/features.md new file mode 100644 index 0000000..54eaf6e --- /dev/null +++ b/docs/features.md @@ -0,0 +1,123 @@ +# Features, Pages, and Widgets + +What's on each screen, what each widget does, and where to find it. + +## Screens (`lib/routes/`) + +### `StartupPage` +Initial loading screen. Computes `init()` (see [routing.md](routing.md)), shows a `loadingAnimation('Loading')` spinner while the future is pending, navigates with `pushReplacementNamed` once decided. The `initFunction` constructor parameter is exposed only for tests so that `StartupPage` can drive `Navigator` against a `Completer`. + +### `HomePage` (`/home`) +Minimal — an `AppBar(title: '4training')`, the `MainDrawer`, and a `TableOfContent` body with the localized "What this app is" intro at the top. + +### `ViewPage` (`/view//`) +The main reading screen. +- AppBar actions: `ShareButton` and `LanguageSelectionButton`. +- Drawer: `MainDrawer(page, langCode)` so the current page is highlighted and the right category is auto-expanded. +- Body: `FutureBuilder` over `checkAndLoad()`, which: + 1. Calls `BackgroundResultNotifier.checkForActivity()` first; if true, shows a `foundBgActivity` snackbar. + 2. Awaits `pageContentProvider((name: page, langCode: langCode)).future`. + 3. Renders `HtmlView(content, direction)`, where direction is `RTL` for `Globals.rtlLanguages` (`ar`, `fa`). +- Error handling: if the provider throws, unwraps Riverpod v3 `ProviderException`, then maps `LanguageNotDownloadedException`/`PageNotFoundException` to a warning-style `ErrorMessage`, `LanguageCorruptedException` to a red `ErrorMessage`, and anything else to a generic internal-error message. +- Side-effect: persists `recentPage`/`recentLang` on every successful render. + +### `SettingsPage` (`/settings`) +Three sections: +1. **App language** — `DropdownButtonAppLanguage`. +2. **`LanguageSettings`** — title + explanation text + `LanguagesTable` (the per-language download/update/delete table). +3. **`UpdateSettings`** — "Last check" timestamp + `CheckNowButton`. The check-frequency and automatic-updates dropdowns are commented out (for v0.9). + +### `AboutPage` (`/about`) +Static text + version. Renders localized strings via `flutter_linkify` so URLs in the strings become tappable (`url_launcher`). + +### `ErrorPage` +Internal fallback. Just shows an `ErrorMessage` with the localized "internal error" template; `MainDrawer` is still attached so the user isn't stranded. + +## Widgets (`lib/widgets/`) + +### `MainDrawer` & `TableOfContent` +The drawer plus a re-usable `TableOfContent` (used directly by `HomePage`). Top-level structure: + +``` +header (defaults to "Content" tappable, navigates to /home) +└─ for each Category: + CategoryTile + └─ for each worksheet in this language belonging to this category: + • TextButton with translated title (highlighted if it's the current page) + • optional translate icon (only if user is currently viewing a different language than the app language) + – if translation exists: opens AvailableInDialog → navigates to /view// + – if not: opens NotTranslatedDialog (greyed icon) +─ Divider +─ Settings (Icon, navigates to /settings) +─ About (Icon, navigates to /about) +``` + +When the *app language* isn't downloaded, the drawer body becomes a single error message using `LanguageNotDownloadedException`'s localized text. + +The `Tap on a translated worksheet that doesn't exist in the language you came from` flow shows a snackbar explaining the language fallback (`languageChangedBack`). + +### `HtmlView` +See [content-rendering.md](content-rendering.md) for full details. Public surface: `HtmlView(String content, TextDirection direction)`. + +### `LanguagesTable` +The settings/onboarding-step-2 table. Each row: +- ✓ if downloaded +- localized language name +- update icon (if updates available, via `UpdateLanguageButton`) or empty +- download icon (`DownloadLanguageButton`, optionally highlighted via `highlightLang`) **or** delete icon (`DeleteLanguageButton`) depending on download state + +Header row has the four "all-languages" buttons: +- `IsDownloaded(allDownloaded)` +- `UpdateAllLanguagesButton` (only renders when `updatesAvailableProvider` is true) +- `DownloadAllLanguagesButton` +- `DeleteAllLanguagesButton` + +Below the table: `diskUsage` total and a "X of Y languages" counter. + +### Language buttons +- **`DownloadLanguageButton`** (`ConsumerStatefulWidget`): icon with internal `_isLoading` flag — swaps to `CircularProgressIndicator` during `LanguageController.download()`. Optional `highlight` flag wraps it in a tinted rounded box (used during onboarding). +- **`DownloadAllLanguagesButton`**: same idea, iterates `availableLanguagesProvider`. +- **`DeleteLanguageButton`**: deleting the *current app language* is "discouraged" — the icon turns `inversePrimary` and clicking shows a `ConfirmDeletionDialog`. +- **`DeleteAllLanguagesButton`**: skips the current app language without prompting. +- **`UpdateLanguageButton`**: same loading pattern, calls `download(force: true)`. +- **`UpdateAllLanguagesButton`**: only visible when `updatesAvailableProvider` is true; iterates languages where `languageStatusProvider(code).updatesAvailable && languageProvider(code).downloaded`. + +### `CheckNowButton` +Iterates downloaded languages and calls `LanguageStatusNotifier.check()` for each. Reports via snackbar: +- success: "N updates available" (via `nUpdatesAvailable`) +- rate limit: "checkingUpdatesLimit" +- other error: "checkingUpdatesError" + +### Dropdowns +Tiny widgets that map a Riverpod provider to a `DropdownButton`. All three follow the same pattern: +- `DropdownButtonAppLanguage` ↔ `appLanguageProvider` +- `DropdownButtonCheckFrequency` ↔ `checkFrequencyProvider` +- `DropdownButtonAutomaticUpdates` ↔ `automaticUpdatesProvider` + +### `LanguageSelectionButton` +The translate icon in the AppBar of `ViewPage`. Lists all downloaded languages that have the current page translated (via `language.pages.containsKey(currentPage)`). If the list is longer than 10, splits into two columns. Bottom of the menu: "Manage languages" → `/settings`. Implemented with `MenuAnchor` + `MenuItemButton` (Material 3 idiom). + +### `ShareButton` (`lib/features/share/`) +The share icon in the AppBar of `ViewPage`. Four entries: +- **Open PDF** — `OpenFilex.open(pdfFile)`. Greyed out if `Page.pdfPath` is null. If user clicks the disabled entry, shows `PdfNotAvailableDialog`. +- **Share PDF** — `SharePlus.share(ShareParams(files: [XFile(pdfFile)]))`. Same disabled behavior. +- **Open in browser** — `url_launcher.launchUrl(https://www.4training.net//)`. +- **Share link** — `SharePlus.share(ShareParams(text: ))`. + +The wrapper `ShareService` (and `shareProvider`) exists *purely* for testability; `share_plus`/`open_filex`/`url_launcher` are static APIs that can't easily be mocked otherwise. The note in `share_service.dart` explains that dependency injection isn't easy because of `context.findAncestorWidgetOfExactType`, so wrapping into a service is the cleanest route. + +### `ErrorMessage` +A reusable card with an icon, title, and message. Used by `ErrorPage` and by `ViewPage`'s error states. + +### `loadingAnimation(msg)` +Function (not a class) returning a `Scaffold` with a centered `CircularProgressIndicator` + label. Used during startup and when fetching page content. + +## Sharing assets + +Static images live in `assets/`: +- `for_training.png` — the logo (used in `WelcomePage` `PromoBlock`). +- `file-document-outline.png` — Open PDF icon. +- `file-document-arrow-right-outline.png` — Share PDF icon. +- `link.png` — Share link icon. + +`pubspec.yaml` only registers `assets/` (without trailing wildcards), which includes everything in that folder. `share_button.dart` has a `// TODO: Use build_runner with flutter_gen instead` for generating typed asset references. diff --git a/docs/in_progress_notes/flushSemantics_assertion.md b/docs/in_progress_notes/flushSemantics_assertion.md new file mode 100644 index 0000000..b37cf86 --- /dev/null +++ b/docs/in_progress_notes/flushSemantics_assertion.md @@ -0,0 +1,86 @@ +# Known remaining issue: `flushSemantics` assertion (filtered from logs) + +### Symptom on device + +Every frame, on pages rendered by `HtmlView` that contain tables, stderr logs: + +``` +RenderBox was not laid out: RenderParagraph#... relayoutBoundary=up5 NEEDS-PAINT +'package:flutter/src/rendering/box.dart': +Failed assertion: line 2251 pos 12: 'hasSize' +``` + +Stack is 100% framework code: `PipelineOwner.flushSemantics` → `_RenderObjectSemantics.ensureGeometry` → 8 levels of `_updateChildGeometry` → `RenderBox.semanticBounds` → `.size` getter assertion. + +### Impact + +- **Non-fatal.** The app runs, tables render, users can read and scroll. The error is caught by the framework's error handler; the assertion fires during the semantics walk, not during layout. +- **Not user-visible.** Only appears in `adb logcat` / Xcode console / `flutter run` output. +- **Real devices only.** Not reproducible in `flutter_test` even with `tester.ensureSemantics()` + a phone-sized surface (`390×844`) + the real `Time_with_God.html` fixture. Confirmed during this session. + +### Root cause (hypothesized) + +The unlaid `RenderParagraph` (and friends — see the four variants below) lives inside a table cell produced by `flutter_html_table`'s `_layoutCells`: each cell is `CssBoxWidget → SizedBox.expand → Container → CssBoxWidget.withInlineSpanChildren → Text.rich → RenderParagraph`, all placed in a `LayoutGrid` inside a `LayoutBuilder` inside a `CssBoxWidget` that is a `WidgetSpan` child of the outer `RichText`. `LayoutBuilder` rebuilds children lazily when constraints change; on real devices the interaction between this rebuild and the paint / dry-layout / semantics passes produces races where one of those passes visits a render object whose current build-phase version has not yet had `performLayout` called. The page draws correctly because main layout stabilizes; only downstream passes stumble. + +### What was tried and rejected + +**`SelectionContainer.disabled` wrapper around the table** (mirroring `raw_tooltip.dart:813-815`, which Flutter itself uses to isolate tooltip overlays from `SelectionArea`). It **did not** fix the assertions and **broke text selection across the entire page** — a `SelectionContainer.disabled` inside a `WidgetSpan` appears to break the outer `SelectionArea`'s walk across sibling text, not just inside the disabled container. Reverted. There is a comment in `lib/widgets/html_view.dart` next to the `TagWrapExtension` documenting this dead end so nobody retries it. + +### The four observed assertion variants + +Diagnostic logging on the device (iOS simulator, iPhone 17, Flutter 3.41.2) captured **four** distinct signatures all originating from the same `TableHtmlExtension.build` code path: + +| # | Exception signature | Where it fires | Stack fingerprint | +|---|---|---|---| +| 1 | `RenderBox was not laid out: ...` | `PipelineOwner.flushPaint` → `RenderCSSBox.paint` → `RenderDecoratedBox.paint` → `RenderBox.size` | Contains `flutter_html/` and `flutter_layout_grid/` | +| 2 | `RenderBox was not laid out: ...` | `PipelineOwner.flushSemantics` → `_SemanticsGeometry.computeChildGeometry` → `RenderBox.semanticBounds` → `RenderBox.size` | **Pure framework** — no package frames (the semantics walk doesn't go through widget code) | +| 3 | `renderBoxDoingDryBaseline == null` / `RenderBox.size accessed in ...computeDryBaseline` | `RenderParagraph.performLayout` → `layoutInlineChildren` → child `performLayout` reading `.size` during a dry-baseline computation | Contains `flutter_html/src/css_box_widget.dart` | +| 4 | `'child!.hasSize' is not true` | `RenderAligningShiftedBox.alignChild` while laying out a `Container` constructed in `flutter_html_table.dart:261` | Contains `flutter_html/` and `flutter_layout_grid/` | + +Variant 2 is the reason early attempts at a stack-path-based filter failed: the stack is **entirely** framework code — no `flutter_html` frames appear, because `_SemanticsGeometry.computeChildGeometry` traverses the `_RenderObjectSemantics` tree directly. A filter that only requires package paths in the stack will silently miss this variant. + +### Current workaround: global `FlutterError.onError` filter + +`lib/main.dart` installs a global error filter via `_installHtmlTableSemanticsFilter()` before `runApp`. It suppresses **only** errors matching all four known signatures and forwards everything else unchanged to the previously-installed handler (typically `FlutterError.presentError`). + +Match condition (applied AND-wise): + +1. **Exception text** contains one of: + - `"RenderBox was not laid out"` — covers variants 1 and 2 + - `"computeDryBaseline"` — covers variant 3 + - `"renderBoxDoingDryBaseline"` — alternate spelling of variant 3 + - `"'child!.hasSize'"` — covers variant 4 (note the surrounding single quotes, which are part of Dart's formatted `AssertionError` output) +1. **Stack text** contains one of: + - `"flutter_html/"` — covers variants 1, 3, 4 + - `"flutter_layout_grid/"` — covers variants 1, 4 + - `"flushSemantics"` — **critical** for variant 2, whose stack has no package frames + +Any error that fails either half is forwarded unchanged, so unrelated regressions are not masked. + +First suppression per app run emits a single breadcrumb via `debugPrint`: + +> `Suppressing known flutter_html_table non-fatal framework assertions (see Task Progress.md §5 and main.dart _installHtmlTableSemanticsFilter for details). Subsequent suppressions are silent.` + +Subsequent suppressions are silent. + +### Verification + +Before the filter was installed, opening `Essentials → Time With God` on the iOS simulator produced ~2,200 lines of stack dumps per page load (see `logs.txt` committed as an artifact during the session — ~200 KB of essentially the same four errors repeating across ~50 frames). After the filter with the broadened match condition, the same page load produces **zero** full stack dumps (see `logs_2.txt`, 72 lines, ending cleanly with one breadcrumb and no error bodies). + +### Unrelated log noise that is NOT suppressed (on purpose) + +The same page load also prints ~12 lines per view of: + +> `Due to Flutter layout restrictions (see https://github.com/flutter/flutter/issues/65895), contents set to 'vertical-align: baseline' within an intrinsically-sized layout may not display as expected...` + +These are `debugPrint` messages emitted by `flutter_html` itself (not `FlutterError` exceptions), so they do **not** go through `FlutterError.onError` and are **not** affected by this filter. They point at upstream issue `flutter/flutter#65895`. They're verbose but functional — nothing is actually cut off or misrendered in the app — so they're left alone. If they ever become intolerable, the fix would be a `debugPrint` override, not an extension of this filter. Logging that as part of §7 future work. + +### Investigation trail + +The match condition went through three iterations before converging on the four-variant form above. The key insights each step produced: + +1. **v1: match on `flushSemantics` only** — worked for the first error we saw (and for variant 2 today), but missed everything the paint pass reports with `library: 'rendering library'`. Gave up because we didn't have device diagnostic output yet. +1. **v2: match on `flutter_html/` / `flutter_layout_grid/` in stack** — caught variants 1, 3, 4 but silently missed variant 2 because the semantics walk stack is pure framework. +1. **v3 (current): 4 exception signatures + package paths OR `flushSemantics`** — catches all four observed variants. Verified against a real-device log capture on iOS simulator. + +The diagnostic logging that drove this convergence (`[FILTER DIAG #N] exception type / text / library / stack-contains-flushSemantics / exception-contains-"RenderBox was not laid out"`, throttled to first 5 invocations) has since been removed from `main.dart` now that the filter is validated. If a future Flutter / flutter_html upgrade produces a new unsuppressed variant, the first step of triage should be to temporarily re-add that diagnostic block and capture one device log run. diff --git a/docs/localization.md b/docs/localization.md new file mode 100644 index 0000000..2d49701 --- /dev/null +++ b/docs/localization.md @@ -0,0 +1,91 @@ +# Localization + +The app uses Flutter's [official internationalization workflow](https://docs.flutter.dev/ui/accessibility-and-localization/internationalization) — `.arb` source files compiled into Dart by `flutter gen-l10n`. + +## Two distinct "languages" — don't confuse them + +| Concept | Stored where | Drives | Values | +| --- | --- | --- | --- | +| **App / UI language** | `appLanguageProvider`, persisted in `SharedPreferences['appLanguage']` | All UI strings (settings, drawer, dialogs, errors) | `system`, `en`, `de` (only — see `AppLanguage.availableAppLanguages`) | +| **Content language** | `languageProvider()` — per-language download state | Worksheet content shown in `ViewPage` | All 34 codes from `availableLanguagesProvider` | + +A user can be reading an Arabic worksheet (content) inside an English UI (app language), or vice versa. The drawer translate-icon flow exists specifically for that case. + +## Files + +- **Source ARB**: `lib/l10n/locales/app_en.arb`, `lib/l10n/locales/app_de.arb`. ~15-16 KB each. +- **Generated Dart**: `lib/l10n/generated/app_localizations.dart` (the abstract delegate) and `app_localizations_{en,de}.dart` (concrete classes). +- **Config**: `l10n.yaml`: + ```yaml + arb-dir: lib/l10n/locales + template-arb-file: app_en.arb + output-localization-file: app_localizations.dart + output-dir: lib/l10n/generated + nullable-getter: false + ``` + `nullable-getter: false` means `AppLocalizations.of(context)` returns `AppLocalizations` (not `AppLocalizations?`), simplifying call sites. +- **Auto-regeneration**: `pubspec.yaml` has `flutter: generate: true`, so `flutter pub get` regenerates the delegates whenever `.arb` files change. + +## Access pattern: `context.l10n` + +The extension in `lib/l10n/l10n.dart` is what you'll see everywhere: + +```dart +extension AppLocalizationsExt on BuildContext { + AppLocalizations get l10n => AppLocalizations.of(this); +} +``` + +So instead of `AppLocalizations.of(context)!.appLanguage`, the codebase writes `context.l10n.appLanguage`. + +Two typical async-callback patterns: +- **Capture before `await`**: `final l10n = context.l10n;` at the top of an async handler, since `BuildContext` is unsafe to use after an async gap. +- **`AppLocalizationsEn` directly**: `Exception.toString()` uses `AppLocalizationsEn()` to produce a stable English message in logs, while `toLocalizedString(context)` uses `context.l10n` for UI display. + +## `getLanguageName()` — dynamic key workaround + +Flutter's generated localizations don't currently support dynamic keys ([flutter#105672](https://github.com/flutter/flutter/issues/105672)), so to translate a language code (`'de'`) into its localized name (`'Deutsch (de)'`), `lib/l10n/l10n.dart` defines: + +```dart +extension GetLanguageNameExt on AppLocalizations { + String getLanguageName(String languageCode) { + final languageMap = { + 'tr': language_tr, 'zh': language_zh, ... 'de': language_de + }; + return languageMap.containsKey(languageCode) + ? "${languageMap[languageCode]!} ($languageCode)" + : languageCode.toUpperCase(); + } +} +``` + +The hardcoded `language_` strings come from the generated `AppLocalizations` (one ARB key per language). **Adding a new content language requires four edits** — see [ONBOARDING.md §7](../ONBOARDING.md#add-a-new-language). + +## Wiring in `MaterialApp` + +`lib/main.dart`: +```dart +MaterialApp( + locale: appLanguage.locale, // from appLanguageProvider + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + ... +) +``` + +`AppLanguage.locale` returns `Locale(languageCode)`. When `appLanguage.toString()` is `'system'` we resolve `languageCode` from `LocaleWrapper.languageCode` (which strips the country part of `Platform.localeName`). If that resolves to anything outside `availableAppLanguages`, we fall back to `'en'`. + +## Plurals and parameters + +The ARB files use ICU MessageFormat for parameterized strings. Examples observed in the codebase: +- `nUpdatesAvailable(count)` — used by `CheckNowButton`. +- `cantDisplayPage(page, languageName)` — used by `ViewPage`. +- `internalError(message)` — used by `ErrorPage`. +- `countLanguages(count)` — used by `LanguagesTable`. +- `updatedNLanguages(count, errors)` — used by `UpdateAllLanguagesButton`. + +When you add a parameterized string, define it once in `app_en.arb` (with `@key` metadata for the placeholders) and translate it in `app_de.arb`. `flutter pub get` regenerates the strongly-typed method. + +## Testing + +Tests typically pass `localizationsDelegates: AppLocalizations.localizationsDelegates` and a fixed `locale: const Locale('de')` (or `'en'`) to `MaterialApp`. Several tests assert against the German strings directly (e.g. `expect(find.text('Schritte der Vergebung'), …)`), so don't change `app_de.arb` casually. diff --git a/docs/onboarding-flow.md b/docs/onboarding-flow.md new file mode 100644 index 0000000..6d80b31 --- /dev/null +++ b/docs/onboarding-flow.md @@ -0,0 +1,64 @@ +# Onboarding Flow + +Three sequential screens shown the first time the app runs. Each persists its own subset of `SharedPreferences` keys; `StartupPage` uses the *presence* of those keys to decide which step (if any) to resume. + +## Step 1 — `WelcomePage` (`/onboarding/1`) + +File: `lib/routes/onboarding/welcome_page.dart`. + +- Hero "Welcome" text + the `PromoBlock` (logo, app name, four bullet points). +- A `DropdownButtonAppLanguage` lets the user pick the **app/UI language** (currently only `system`, `en`, or `de`). +- "Continue" button: + - Calls `appLanguageProvider.notifier.persistNow()` so that the chosen `appLanguage` is now saved (this is the key `StartupPage` uses to decide first-run-vs-not). + - `Navigator.pushReplacementNamed(context, '/onboarding/2')`. + +`PromoBlock` is reused on the About page. + +The page is wrapped in `LayoutBuilder + SingleChildScrollView + ConstrainedBox(minHeight: viewport) + IntrinsicHeight + Spacer` so that on tall devices it fills the screen and on small devices it scrolls. + +## Step 2 — `DownloadLanguagesPage` (`/onboarding/2`) + +File: `lib/routes/onboarding/download_languages_page.dart`. + +- Renders the same `LanguagesTable` widget used on the settings page, but with `highlightLang: appLanguage.languageCode` so the user's app-language download button is visually highlighted. +- "Continue" is disabled-looking until the app language is downloaded: + - Implementation note: we don't set `onPressed: null` (which would also disable click handling). We pass a manually-greyed `ButtonStyle` (`onSurface.withOpacity(0.12)` background, `0.38` foreground) so the button stays clickable. Clicking it while not yet downloaded shows a `MissingAppLanguageDialog` warning. + - Once the app language is downloaded, "Continue" routes to `getNextRoute(ref)`. Currently that always returns `/home` — for v0.9 this will branch to `/onboarding/3` if `checkFrequency` isn't set yet. +- "Back" returns to `/onboarding/1`. + +## Step 3 — `SetUpdatePrefsPage` (`/onboarding/3`) + +File: `lib/routes/onboarding/set_update_prefs_page.dart`. + +> Currently **bypassed during normal startup** (the corresponding branch in `StartupPage.init()` is commented out, gated for v0.9). The page works end-to-end and is exercised by tests, just not reachable from the regular onboarding chain. + +- `DropdownButtonCheckFrequency` (never / daily / weekly / monthly / 15-min test interval). +- `DropdownButtonAutomaticUpdates` (never / requireConfirmation / onlyOnWifi / yesAlways). +- "Let's go" button: + 1. `automaticUpdatesProvider.notifier.persistNow()` — writes the *current* dropdown value (which may be the default if user didn't change it) to `SharedPreferences`. Without this step, `StartupPage` would think onboarding wasn't completed. + 2. `checkFrequencyProvider.notifier.persistNow()` — same. + 3. `backgroundSchedulerProvider.notifier.schedule()` — registers the periodic task (currently a no-op since the body is commented out for v0.9). + 4. `Navigator.pushReplacementNamed(context, '/home')`. + +## Persistence map + +| Pref key | Set when | Read by | +| --- | --- | --- | +| `appLanguage` | After step 1 (`persistNow`) | `StartupPage` (first-run gate), `AppLanguageController.build` | +| `automaticUpdates` | After step 3 + every dropdown change | `AutomaticUpdatesNotifier.build` | +| `checkFrequency` | After step 3 + every dropdown change | `CheckFrequencyNotifier.build`, **(once enabled)** `StartupPage` (third-step gate) | +| `recentPage`, `recentLang` | After every `ViewPage` render | `StartupPage` for resume | +| `lastChecked-`, `updatesAvailable-` | `LanguageStatusNotifier.check()` (foreground or background) | `LanguageStatusNotifier.build`, `BackgroundResultNotifier.checkForActivity` | + +## "Continue greyed-out, but clickable" pattern + +A recurring trick. `ElevatedButton` has no clean way to look disabled but still respond to taps. The button is therefore styled greyed-out manually (matching `ElevatedButton.defaultStyleOf` opacity values) while keeping `onPressed` non-null and showing a warning dialog inside the handler. This is encapsulated in step 2's `ButtonStyle buttonStyle` ternary. + +## Tests + +- `test/welcome_page_test.dart` +- `test/download_languages_page_test.dart` +- `test/set_update_prefs_page_test.dart` +- `test/startup_page_test.dart` — the routing-decision tests for the four startup states. + +These tests use `TestLanguageController(initReturns: bool, downloadReturns: bool)` from `test/languages_test.dart` to fake out the disk state. diff --git a/docs/routing.md b/docs/routing.md new file mode 100644 index 0000000..16e2114 --- /dev/null +++ b/docs/routing.md @@ -0,0 +1,72 @@ +# Routing + +The app uses **Navigator 1.0** with **named routes** — no `go_router`, no nested navigators. The single dispatcher is `generateRoutes(RouteSettings)` in `lib/routes/routes.dart`, plugged into `MaterialApp.onGenerateRoute`. + +## Route table + +| Route | Page widget | Notes | +| --- | --- | --- | +| `/` (or null) | `StartupPage` | Loading screen + initialization. Decides where to go next | +| `/home` | `HomePage` | Table of contents, in the user's app language | +| `/view//` | `ViewPage(page, langCode)` | The "real" worksheet view. Deep-linkable. Mirrors the URL on 4training.net | +| `/settings` | `SettingsPage` | Manage languages + check-for-updates UI | +| `/about` | `AboutPage` | About text + license + version | +| `/onboarding` or `/onboarding/1` | `WelcomePage` | App language selection | +| `/onboarding/2` | `DownloadLanguagesPage` | Download required language(s) | +| `/onboarding/3` | `SetUpdatePrefsPage` | Update preferences (currently bypassed in startup flow — gated for v0.9) | +| anything else | `ErrorPage('Unknown route ...')` | Final fallback | + +`/view` malformed (missing parts) redirects to `/home`. The dispatcher logs every incoming route via `debugPrint`. + +## Why named routes + +The README spells this out: pages live at `/view//`, matching exactly the URL of the same worksheet on `4training.net` (e.g. `/view/Dealing_with_Money/de` ↔ `https://www.4training.net/Dealing_with_Money/de`). This makes deep links and link-tap handling inside the HTML body almost trivial — `HtmlView.onAnchorTap` just does `Navigator.pushNamed(context, '/view$url')`. + +## Initial navigation logic + +`StartupPage.init()` (`lib/routes/startup_page.dart`) is the one place that decides where the user lands on app launch: + +``` +StartupPage.init(): + if SharedPreferences['appLanguage'] is null: + return '/onboarding/1' # first time + + for each available language: + ref.read(languageProvider(code).notifier).init() # load disk state + + if app language is not yet downloaded: + return '/onboarding/2' # resume onboarding + + # (commented out for v0.9: third onboarding step on missing checkFrequency) + + ref.read(backgroundSchedulerProvider.notifier).schedule() + + if SharedPreferences['recentPage'] && 'recentLang' && language is downloaded: + return '/view//' # resume last worksheet + return '/home' +``` + +The first `await` in `init()` is what makes the loading spinner appear; once `init()` resolves, `Navigator.pushReplacementNamed` jumps to the chosen route, so the user never sees the home screen flash. + +## Navigation primitives + +- **`Navigator.pushNamed`** for normal in-app navigation. +- **`Navigator.pushReplacementNamed`** for the onboarding flow and the post-startup redirect, so the user can't go "back" into the loading screen. +- **`Navigator.popAndPushNamed`** when switching to a different translation of the same worksheet via the in-drawer translate icon. + +## Persisting "recent" + +Whenever `ViewPage` successfully renders a page it writes: +```dart +ref.read(sharedPrefsProvider).setString('recentPage', page); +ref.read(sharedPrefsProvider).setString('recentLang', langCode); +``` +This is the state that `StartupPage` later reads to resume. + +## Adding a route — checklist + +1. Add a new branch in `generateRoutes()`. +2. Create the page widget in `lib/routes/` (or a feature subfolder). +3. Decide whether the route should appear in the drawer (`lib/widgets/main_drawer.dart`). +4. If the route takes path parameters (like `/view/`), parse them with `settings.name!.split('/')` and validate that all parts are non-empty before dispatching; otherwise fall through to the `ErrorPage` (or redirect to `/home` if it's a deep link from outside). +5. Add tests in `test/routes_test.dart` (uses a `TestObserver` `NavigatorObserver`). diff --git a/docs/state-management.md b/docs/state-management.md new file mode 100644 index 0000000..61a207c --- /dev/null +++ b/docs/state-management.md @@ -0,0 +1,117 @@ +# State Management + +The app uses **Riverpod 2/3** style (with `flutter_riverpod 3.x`), no codegen. Every piece of cross-widget state is a provider; there is no `BLoC`, `Provider`, or `ChangeNotifier`-only code. + +This page is the index of every provider in the app — what it holds, what it depends on, and how it is overridden in tests. + +## Conventions in this codebase + +- **Notifiers, not classes-with-callbacks.** Anything mutable is a `Notifier` exposed via `NotifierProvider`. Anything pure-derived is a `Provider`. +- **Family providers for per-language state.** `languageProvider` and `languageStatusProvider` are `NotifierProvider.family<…, …, String>`, keyed by language code. +- **`MustOverrideProvider()`** (declared in `lib/data/globals.dart`) is a custom helper that creates a `Provider` whose default factory throws `ProviderNotOverriddenException`. Used for `sharedPrefsProvider` and `packageInfoProvider` so that *every* `ProviderScope` in the app or tests must explicitly override them. +- **`retry: null`** is set on a few async providers (`pageContentProvider`, `scaffoldMessengerProvider`) and on `MustOverrideProvider` to disable Riverpod v3's automatic retry, which is undesirable when the failure is intentional. +- **`ref.$arg`** (`package:riverpod/src/framework.dart`) is used inside `LanguageController.build()` and `LanguageStatusNotifier.build()` to read the family argument. This is required when overriding the family with `overrideWith()` in tests, where the argument doesn't pass through the constructor. The import is annotated `// ignore: implementation_imports, invalid_use_of_internal_member`. + +## Provider catalog + +### App infrastructure (`lib/data/globals.dart`) + +| Provider | Type | Purpose | +| --- | --- | --- | +| `sharedPrefsProvider` | `Provider` (must-override) | The single `SharedPreferences` instance, set in `main()` | +| `packageInfoProvider` | `Provider` (must-override) | App version etc. for the About page | +| `scaffoldMessengerKeyProvider` | `Provider>` | The key passed to `MaterialApp.scaffoldMessengerKey` | +| `scaffoldMessengerProvider` | `Provider` | Resolves the key to the current state. Used as `ref.watch(scaffoldMessengerProvider).showSnackBar(...)` from any callback | +| `availableLanguagesProvider` | `Provider>` | The hardcoded list of 34 supported language codes | +| `automaticUpdatesProvider` | `NotifierProvider` | Persisted user preference: never / requireConfirmation / onlyOnWifi (default) / yesAlways | + +`AutomaticUpdatesNotifier`: +- `build()` reads `automaticUpdates` from `SharedPreferences`. +- `setAutomaticUpdates(String?)` updates state + persists. +- `persistNow()` is called from the third onboarding screen so that completing onboarding writes the default value to disk explicitly. + +`Globals` (a static-only class in the same file) holds the constant URLs (`https://github.com/4training/html-/archive/refs/heads/main.zip`) and folder names (`html--main`). + +### App language (`lib/data/app_language.dart`) + +| Provider | Type | Purpose | +| --- | --- | --- | +| `appLanguageProvider` | `NotifierProvider` | The currently selected app/UI language. `AppLanguage` is `(isSystemDefault, languageCode)` | + +- `AppLanguage.fromString(str, defaultLangCode)` parses the persisted string. `'system'` resolves to `defaultLangCode` (read from `Platform.localeName` via `LocaleWrapper`). Falls back to `'en'`. +- Currently the app supports only `system | en | de` as the **app** language (`AppLanguage.availableAppLanguages`), even though there are 34 **content** languages. +- `LocaleWrapper.languageCode` is a wrapper around `Platform.localeName` for testability. + +### Per-language data (`lib/data/languages.dart`) + +| Provider | Type | Purpose | +| --- | --- | --- | +| `fileSystemProvider` | `Provider` | `LocalFileSystem()` by default; overridden with `MemoryFileSystem` in tests | +| `imageContentProvider` | `Provider.family` | Returns base64-encoded PNG bytes for a given image. Used by `pageContentProvider` | +| `pageContentProvider` | `FutureProvider.family` | The HTML body of a worksheet, with images inlined as base64. Throws `LanguageNotDownloadedException` / `PageNotFoundException` / `LanguageCorruptedException`. `retry: null` | +| `languageProvider` | `NotifierProvider.family` | Per-language state: pages, images, PDFs, disk path, size, download timestamp | +| `countDownloadedLanguagesProvider` | `Provider` | Derived count for the settings page | +| `diskUsageProvider` | `Provider` | Sum of all `Language.sizeInKB` | + +`LanguageController` is the heart of the app. Methods: +- `init()`: idempotent load from disk. Call once per language at startup. +- `lazyInit()`: like `init()` but cheap — only sets `downloaded` + `path` + timestamp without parsing JSON. Used in the background isolate. +- `download({force=false})`: clears (if forcing), downloads HTML+PDF zips, parses structure. +- `deleteResources()`: clears assets dir, resets state. +- `_load()`: the parser. Reads `structure/contents.json`, scans `pdf--main/` for PDF files, registers images in `files/`. Catches all errors and clears the assets dir on failure. + +`Language` (immutable data class): +- `languageCode`, `pages: Map`, `pageIndex: List` (menu order), `images`, `path`, `sizeInKB`, `downloadTimestamp` (always UTC). +- `downloaded` getter is `languageCode != ''`. +- `getPageTitles()` returns the menu in order: English-name → translated-title. + +### Update checking (`lib/data/updates.dart`) + +| Provider | Type | Purpose | +| --- | --- | --- | +| `httpClientProvider` | `Provider` | Default `http.Client()`; overridden in tests | +| `checkFrequencyProvider` | `NotifierProvider` | never / daily / weekly (default) / monthly / testinterval (15 min) | +| `languageStatusProvider` | `NotifierProvider.family` | Per-language status: `(updatesAvailable, downloadTimestamp, lastCheckedTimestamp)` | +| `updatesAvailableProvider` | `Provider` | Any-language updates? (drives the "update all" button visibility) | +| `lastCheckedProvider` | `Provider` | The oldest `lastCheckedTimestamp` across all downloaded languages, shown on the settings page | + +`LanguageStatusNotifier.build()` watches `languageProvider()`, so it re-runs whenever a language is downloaded or deleted (which is by design — it resets the persisted prefs when the language is gone, or refreshes timestamps when freshly downloaded). + +`LanguageStatusNotifier.check()` performs the GitHub commits-since query and returns: +- `0`: no updates, +- `> 0`: number of new commits, +- `apiRateLimitExceeded` (-403): hit the 60-req/h unauthenticated limit, +- `-1`: any other error. + +### Background isolate sync (`lib/background/background_result.dart`, `background_scheduler.dart`) + +| Provider | Type | Purpose | +| --- | --- | --- | +| `backgroundResultProvider` | `NotifierProvider` | Tracks whether the background task did something we should surface | +| `backgroundSchedulerProvider` | `NotifierProvider` | State = "task currently scheduled?" (Currently always `false` — body of `schedule()` is commented out for v0.9) | + +`BackgroundResultNotifier.checkForActivity()` is the trick that lets the foreground detect background work without IPC: it calls `prefs.reload()` and compares persisted `lastChecked-` to the in-memory `LanguageStatus.lastCheckedTimestamp`. If the persisted value is newer, it invalidates the corresponding `languageStatusProvider`. + +### Sharing (`lib/features/share/share_service.dart`) + +| Provider | Type | Purpose | +| --- | --- | --- | +| `shareProvider` | `Provider` | A thin class that wraps `share_plus`, `url_launcher`, and `open_filex` so they can be mocked in tests | + +## Override map for tests + +The standard test setup is a `ProviderContainer` (or `ProviderScope`) with overrides: + +```dart +ProviderContainer(overrides: [ + sharedPrefsProvider.overrideWithValue(prefs), // required + packageInfoProvider.overrideWithValue(packageInfo), // required if using App4Training + fileSystemProvider.overrideWith((ref) => MemoryFileSystem()), // optional + httpClientProvider.overrideWith((ref) => mockClient), // optional + languageProvider.overrideWith(() => TestLanguageController(...)), // optional + languageStatusProvider.overrideWith(() => TestLanguageStatus()), // optional + backgroundSchedulerProvider.overrideWith(() => TestBackgroundScheduler()), // for /startup tests +]) +``` + +See `test/languages_test.dart` for the concrete `Test*` controller classes. They are reused by every other widget test that needs to fake out a downloaded language. diff --git a/docs/testing.md b/docs/testing.md new file mode 100644 index 0000000..31ebe2a --- /dev/null +++ b/docs/testing.md @@ -0,0 +1,149 @@ +# Testing + +## Test layout + +``` +test/ +├── assets-en/html-en-main/ ← real HTML fixtures used by widget tests +├── assets-de/{html-de-main, pdf-de-main}/ ← bigger German fixture set +├── about_page_test.dart +├── app_language_test.dart +├── background_result_test.dart +├── background_scheduler_test.dart +├── background_task_test.dart +├── check_now_button_test.dart +├── delete_language_button_test.dart +├── download_language_button_test.dart +├── download_languages_page_test.dart +├── dropdownbutton_app_language_test.dart +├── dropdownbutton_automatic_updates_test.dart +├── dropdownbutton_check_frequency_test.dart +├── exceptions_test.dart +├── globals_test.dart +├── html_view_table_test.dart +├── html_view_test.dart +├── language_selection_button_test.dart +├── languages_table_test.dart +├── languages_test.dart ← shared helpers (TestLanguageController, FakeDownloadAssetsController, ...) +├── main_drawer_test.dart +├── routes_test.dart +├── set_update_prefs_page_test.dart +├── settings_page_test.dart +├── share_button_test.dart +├── startup_page_test.dart +├── update_language_button_test.dart +├── updates_test.dart ← shared helpers (TestLanguageStatus, mockCheckResponse) +├── view_page_test.dart +└── welcome_page_test.dart + +integration_test/ +└── background_interaction_test.dart ← real Android emulator +``` + +## Running + +```bash +# All unit + widget tests, with coverage +flutter test --coverage + +# A single file +flutter test test/view_page_test.dart + +# Integration test (needs Android emulator or device) +flutter test integration_test +``` + +CI runs both via `.github/workflows/main.yaml`. Coverage uploads to Codecov; the badge in `README.md` reflects this. + +## Coverage trick: `full_coverage` + +`flutter test --coverage` only counts files that are imported by some test. To make every file in `lib/` count, the CI step: + +```yaml +dart pub global activate full_coverage +dart pub global run full_coverage +``` + +generates `test/full_coverage_test.dart` that imports every file under `lib/`. The local equivalent is the same two commands. + +## Shared test helpers + +### `test/languages_test.dart` +- **`MockDownloadAssetsController`** (mocktail) — straight `Mock` impl. +- **`FakeDownloadAssetsController`** — minimal hand-written fake exposing `initCalled`, `clearAssetsCalled`, `startDownloadCalls` so tests can assert on call counts. +- **`ThrowingDownloadAssetsController`** — fake that throws from `startDownload` to simulate network failures. +- **`TestLanguageController`** — overrides `LanguageController` to short-circuit `init`/`download` to a configured boolean. Used pervasively by widget tests that don't care about disk state. +- **Helper functions** to build `MemoryFileSystem` instances pre-populated with the German/English fixtures from `test/assets-de/`, `test/assets-en/`. + +### `test/updates_test.dart` +- **`TestLanguageStatus`** — overrides `LanguageStatusNotifier` for tests that don't want to mock HTTP. +- **`mockCheckResponse({'de': 2, 'en': 0, ...})`** — builds an `http.Client` (via `mocktail`) that returns the right number of fake commits per language code. + +## Standard test scaffolding + +Most tests follow this template: + +```dart +testWidgets('...', (tester) async { + SharedPreferences.setMockInitialValues({...}); + final prefs = await SharedPreferences.getInstance(); + + await tester.pumpWidget(ProviderScope( + overrides: [ + sharedPrefsProvider.overrideWithValue(prefs), + packageInfoProvider.overrideWithValue(...), + languageProvider.overrideWith(() => TestLanguageController(initReturns: true)), + ], + child: const MaterialApp( + locale: Locale('de'), + localizationsDelegates: AppLocalizations.localizationsDelegates, + home: SomePage(), + ), + )); + await tester.pumpAndSettle(); + + expect(find.text('...'), findsOneWidget); +}); +``` + +Almost every test: +- Sets `SharedPreferences.setMockInitialValues` — the `shared_preferences` plugin's official test API. +- Hardcodes `locale: Locale('de')` (the German strings are the assertion targets in many tests, e.g. "Einstellungen", "Schritte der Vergebung"). +- Overrides `sharedPrefsProvider`, `packageInfoProvider`, and the relevant family providers. + +## `routes_test.dart` — routing + +Uses a `TestObserver extends NavigatorObserver` to record `didPush` and `didReplace` calls. The asserts then check that, given a starting state, the right `pushReplacementNamed` was invoked. This is the cleanest way to verify `StartupPage`'s decision matrix end-to-end. + +## Integration test + +`integration_test/background_interaction_test.dart` is the only test that runs on a real Android emulator. CI runs it via `reactivecircus/android-emulator-runner@v2` at API level 29 with `arch: x86_64`. Locally: + +```bash +flutter test integration_test +``` + +It: +1. Initializes `Workmanager` with `backgroundTask` as the dispatcher. +2. Registers a one-off task with `task: 'testTask'`, which selects the test branch in `backgroundTask` (calls `backgroundTestMain()` instead of `backgroundMain()`). +3. Uses `IsolateNameServer` to receive a "success" message from the background isolate. +4. The second test additionally verifies that opening a worksheet in the foreground after the background task ran shows the `foundBgActivity` snackbar. + +See [background-tasks.md](background-tasks.md) for the full mechanism. + +## What's NOT tested + +- **Real HTTP** to `api.github.com` or `github.com` — every test uses `httpClientProvider` overrides or `TestLanguageStatus`. +- **Real disk** — `fileSystemProvider` is overridden with `MemoryFileSystem` for download/load tests, or the `test/assets-*` directories are exposed via a `chroot`-style file system. +- **`flutter_html_table` rendering edge cases** — the assertions filtered by `_installHtmlTableSemanticsFilter` in `main.dart` don't reproduce in `flutter_test` (see the docstring there). +- **Real `workmanager`** — only the integration test exercises it. + +## Pre-commit checks + +Before pushing, run: + +```bash +dart format . +dart analyze +flutter test +``` diff --git a/lib/main.dart b/lib/main.dart index 4c267de..32e7b23 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -81,8 +81,9 @@ import 'design/theme.dart'; /// [debugPrint] so developers inspecting logs can see that the filter /// is active. Subsequent suppressions are silent to avoid log spam. /// -/// See also: `Task Progress.md` §5 for the full investigation history, -/// including the diagnostic logging session that identified all four +/// See also: `docs/in_progress_notes/flushSemantics_assertion.md` +/// for the full investigation history. +/// Including the diagnostic logging session that identified all four /// variants. void _installHtmlTableSemanticsFilter() { final FlutterExceptionHandler? previousHandler = FlutterError.onError;