From 9b80fff0ad438727a6c977ace3e521a619bc21f0 Mon Sep 17 00:00:00 2001 From: Ziedelth Date: Wed, 19 Aug 2026 09:05:17 +0000 Subject: [PATCH] Add Flutter guidelines for MVVM rework (v2) --- AGENTS.md | 67 ++++++++++++++++++++++ guidelines/API_CONVENTIONS.md | 102 +++++++++++++++++++++++++++++++++ guidelines/ARCHITECTURE.md | 72 +++++++++++++++++++++++ guidelines/CODE_STYLE.md | 87 ++++++++++++++++++++++++++++ guidelines/LOCALIZATION.md | 68 ++++++++++++++++++++++ guidelines/MODELS.md | 71 +++++++++++++++++++++++ guidelines/PERFORMANCE.md | 25 ++++++++ guidelines/SECURITY.md | 26 +++++++++ guidelines/STATE_MANAGEMENT.md | 70 ++++++++++++++++++++++ guidelines/TESTING.md | 83 +++++++++++++++++++++++++++ guidelines/THEME.md | 84 +++++++++++++++++++++++++++ 11 files changed, 755 insertions(+) create mode 100644 AGENTS.md create mode 100644 guidelines/API_CONVENTIONS.md create mode 100644 guidelines/ARCHITECTURE.md create mode 100644 guidelines/CODE_STYLE.md create mode 100644 guidelines/LOCALIZATION.md create mode 100644 guidelines/MODELS.md create mode 100644 guidelines/PERFORMANCE.md create mode 100644 guidelines/SECURITY.md create mode 100644 guidelines/STATE_MANAGEMENT.md create mode 100644 guidelines/TESTING.md create mode 100644 guidelines/THEME.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..18a36f1 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,67 @@ +# LLM Agent Instructions - Shikkanime Application (Flutter) + +As an AI agent, your primary directive is to adhere to the established patterns and architectural principles of this project. Your goal is to write clean, maintainable Flutter code that aligns with the existing codebase. This application is built on a **Model-View-ViewModel (MVVM)** architecture and is currently being reworked on the `v2` branch. + +This file contains your core, non-negotiable rules. For detailed implementation guidance, refer to the linked documents in the `guidelines` directory. + +## 1. The Prime Directive: Respect the MVVM Architecture + +The project follows a strict MVVM layered architecture. **Do not violate this structure.** + +``` +View (widgets) -> ViewModel (ChangeNotifier) -> Repository -> HttpClient -> API +``` + +- **Views** (`lib/views/`) are "dumb" widgets. They display state and forward user events. They must NOT contain business logic. +- **ViewModels** (`lib/viewmodels/`) hold UI state and logic, extend `ChangeNotifier`, and expose the state the view reads. +- **Repositories** (`lib/repositories/`) are the single source of truth for data and orchestrate HTTP calls. They are `const` classes with constructor injection. +- **Models** (`lib/models/`) are immutable DTOs, mostly generated with `json_serializable`. +- Data flows in **one direction**: UI reads state, sends events to the ViewModel, the ViewModel calls the Repository, then notifies the UI. + +For a deeper understanding, read the [Architecture Guide](guidelines/ARCHITECTURE.md). + +The UI layer MUST use `context.read()` to trigger events and `context.watch()` to read reactive state. **No global singletons.** + +## 2. State Management: Provider + ChangeNotifier + +- Dependency injection is handled with the `provider` package. Wire everything in `main.dart` through `MultiProvider`. +- ViewModels extend `ChangeNotifier`; call `notifyListeners()` after mutating state that the UI observes. +- **Do not put logic in widgets.** Widgets only bind to the ViewModel. +- Read the [State Management Guide](guidelines/STATE_MANAGEMENT.md) for details. + +## 3. Write Code for Humans (and other AIs) + +- Write all code, comments, and documentation in **English**. +- Your code must be readable and self-explanatory. Add comments only when the logic is non-obvious. +- **Reuse existing patterns and conventions.** Before writing new code, understand how similar features are implemented in the project. +- Keep one responsibility per class and function, keep functions short, and do not add premature abstraction. + +Refer to the style and convention guides for specifics: +- [Code Style Guide](guidelines/CODE_STYLE.md) +- [Models Guide](guidelines/MODELS.md) +- [API Conventions](guidelines/API_CONVENTIONS.md) +- [Theme Guide](guidelines/THEME.md) + +## 4. Security is Not Optional + +- Treat **all** external input (responses, query params, user input) as untrusted. +- Validate and sanitize incoming data before use. +- **Never** log sensitive information such as tokens, cookies, or personal identifiers. +- Consult the [Security Guide](guidelines/SECURITY.md) for detailed instructions. + +## 5. Testing and Localization + +- Tests are written with `Given / When / Then` structure using fakes (subclassing concrete classes). +- UI text goes through `AppLocalizations`; source language is French (`app_fr.arb`). +- Consult the [Testing Guide](guidelines/TESTING.md) and [Localization Guide](guidelines/LOCALIZATION.md). + +## 6. Before Submitting Changes + +Before concluding your task, ensure: +- The change fits the correct architectural layer (View/ViewModel/Repository/Model). +- Existing patterns and conventions are reused. +- `dart format lib` and `dart analyze lib` pass cleanly. +- No sensitive data is logged. +- Reusable widgets go to `lib/core/widgets/`, not ad-hoc. + +Consult the [Performance Guide](guidelines/PERFORMANCE.md) for rules as needed. diff --git a/guidelines/API_CONVENTIONS.md b/guidelines/API_CONVENTIONS.md new file mode 100644 index 0000000..7e7cd12 --- /dev/null +++ b/guidelines/API_CONVENTIONS.md @@ -0,0 +1,102 @@ +# API Conventions + +Networking is centralized in the `core/network/` layer: a single `HttpClient` and a `sealed class ApiResult`. Repositories build endpoints and map responses. + +## ApiResult + +Every request returns an `ApiResult`: + +- `ApiSuccess(data)` — HTTP 2xx and payload parsed. +- `ApiFailure([error, statusCode])` — non-2xx status or a parse/network error. + +```dart +sealed class ApiResult { + const ApiResult(); +} + +class ApiSuccess extends ApiResult { + const ApiSuccess(this.data); + final T data; +} + +class ApiFailure extends ApiResult { + const ApiFailure([this.error, this.statusCode]); + final String? error; + final int? statusCode; +} +``` + +Because `ApiResult` is `sealed`, consumers handle it with an exhaustive `switch`. + +## HttpClient + +A single `const HttpClient` wraps `package:http`, builds the URI from `EnvConfig.apiBaseUrl`, enforces a `timeout`, checks the status, and parses JSON. + +- Call it as `get(endpoint, queryParameters: {...})`. +- `T` is the **raw parsed type** (`Map` or `List`), not the model. JSON is decoded to `Map`/`List`, and `_parseJson` returns `ApiFailure` when the decoded value is not of type `T`. + +```dart +final response = await _httpClient.get>( + 'v1/animes', + queryParameters: { + 'country': 'FR', + if (query != null && query.isNotEmpty) 'name': query, + 'page': page, + 'limit': limit, + }, +); +``` + +## Repository mapping pattern + +Repositories are `const` classes holding an `HttpClient` via constructor injection. They: + +1. call `_httpClient.get(...)` with an appropriate raw type (`Map` for one object, `List` for a list); +2. `switch` over the returned `ApiResult`; +3. map the raw payload (or the raw list) to typed models through `Model.fromJson(...)`. + +```dart +class WeeklyRepository { + const WeeklyRepository(this._httpClient); + + final HttpClient _httpClient; + + Future>> getWeekly({ + List? langTypes, + }) async { + final response = await _httpClient.get>( + 'v1/animes/weekly', + queryParameters: { + 'country': 'FR', + if (langTypes != null && langTypes.isNotEmpty) + 'searchTypes': langTypes.map((e) => e.name.toUpperCase()).join(','), + }, + ); + + return switch (response) { + ApiSuccess>(:final data) => + ApiSuccess>( + data.map((e) => WeeklyDayModel.fromJson(e as Map)) + .toList(), + ), + ApiFailure> failure => ApiFailure>( + failure.error, + failure.statusCode, + ), + }; + } +} +``` + +## Query parameters + +- Add query parameters conditionally with collection-`if` so empty/null values are omitted. +- Enums in query params are serialized via `e.name.toUpperCase()` (matching the API contract). +- Always include required API params (e.g. `'country': 'FR'`). + +## Guidelines + +- Repositories are the **only** layer that talks to `HttpClient`. ViewModels never call `HttpClient` directly. +- Keep methods compact and map results at the repository boundary. +- Endpoints are versioned (`v1/...`); keep them aligned with the API. +- 🌱 To consider for the rework: Flutter officially recommends abstract repository classes for per-environment implementations and separate API vs domain models in large apps. Not required now; concrete `const` repositories are the documented norm. \ No newline at end of file diff --git a/guidelines/ARCHITECTURE.md b/guidelines/ARCHITECTURE.md new file mode 100644 index 0000000..9367305 --- /dev/null +++ b/guidelines/ARCHITECTURE.md @@ -0,0 +1,72 @@ +# Architecture Guide + +The application follows a **Model-View-ViewModel (MVVM)** architecture. Respect the project layers and their dependency direction: + +``` +View (widgets) -> ViewModel (ChangeNotifier) -> Repository -> HttpClient -> API +``` + +## Layer responsibilities + +### Views (`lib/views/`) +- Widgets are **"dumb"**: they render state and forward user events to the ViewModel. +- The only logic a view may contain is: + - simple `if` conditions that show/hide a widget based on a flag or nullable field on the ViewModel; + - layout logic based on device information (screen size, orientation); + - simple routing logic. +- Views read state with `context.watch()` and trigger events with `context.read()`. + +### ViewModels (lib/viewmodels/) +- Hold UI state and the logic that operates on it. +- Extend `ChangeNotifier` (via `package:flutter/foundation.dart`) and call `notifyListeners()` after any mutation the UI observes. +- Orchestrate calls to Repositories; they never perform HTTP or layout work directly. +- Compose other ViewModels by constructor injection (see `AnimeViewModel` which depends on `SimulcastViewModel`). + +### Repositories (lib/repositories/) +- **Single source of truth for data.** Repositories isolate the app from the API. +- `const` classes whose constructor receives an `HttpClient`. +- They build endpoints, attach query parameters, execute the HTTP call, and map the raw response to typed models (see the [API Conventions](API_CONVENTIONS.md) for the exact mapping pattern). + +### Models (lib/models/) +- Immutable DTOs. Use `final` fields and constructor injection (`AnimeModel(this.uuid, ...)`). +- Marked with `@JsonSerializable`; generated factory logic lives in the sibling `.g.dart` file (`part 'anime_model.g.dart'`). +- See the [Models Guide](MODELS.md). + +### Core (lib/core/) +- Shared, reusable code across the app: configuration (`config/`), networking (`network/`), theming (`theme/`), and shared widgets (`widgets/`). +- Reusable widgets live in `lib/core/widgets/`, not ad-hoc inside a screen folder. + +## Navigation + +The app uses a single `PageView` + `BottomNavigationBar` driven by `NavigationViewModel`. There is no URL router. Use the existing pattern for the app. + +- 🌱 To consider for the rework: Flutter officially recommends `go_router` for navigation in most apps. Not required yet; document the existing `PageView` pattern as the norm. + +## Repository and models: to consider + +- 🌱 Flutter strongly recommends abstract repository classes so different environments (dev/staging) can swap implementations, and generated immutable models (`freezed` / `built_value`). These are **not** required now; the existing concrete repositories (`const` + constructor injection) and `json_serializable` models are the documented norm. + +## Project structure (v2) + +``` +lib/ + main.dart # Composition root: MultiProvider + runApp + MaterialApp + core/ + config/ # EnvConfig (env-driven values) + network/ # HttpClient, ApiResult + theme/ # AppTheme, AppThemeExtension, app_theme_colors + widgets/ # Shared, reusable widgets (app_*, lang_types/*) + models/ # Immutable DTOs + generated .g.dart + repositories/ # Concrete repositories (source of truth for data) + viewmodels/ # ChangeNotifier view models + views/ # Screen widgets + per-screen widgets/ + l10n/ # Generated localizations (app_localizations, *_fr) +``` + +`lib/data/` (repositories + services) exists but is **empty**; it is a leftover of the rework in progress. Do not add code there; `lib/repositories/` is the active location. + +## Architecture principles (transversal) + +- **Single responsibility**: one concern per class/function. +- **No dead or commented-out code.** +- **Unidirectional data flow**: interact from UI to data layer; updates flow back through `ChangeNotifier`. \ No newline at end of file diff --git a/guidelines/CODE_STYLE.md b/guidelines/CODE_STYLE.md new file mode 100644 index 0000000..c59c91f --- /dev/null +++ b/guidelines/CODE_STYLE.md @@ -0,0 +1,87 @@ +# Code Style Guide + +## General style + +- **Write all code, examples, and comments in English.** +- **Prefer short, readable, direct code** with explicit, descriptive names. +- **One responsibility per class and function.** Keep functions short and focused. +- **One statement per line.** Do not chain multiple statements on a single line. +- **No visual separators** in comments (no ASCII banner lines, no decorative `#`, `-`, `*` runs). +- **Reuse existing project patterns** instead of introducing new ones without need. +- **No premature abstraction and no duplication (DRY).** Do not copy-paste logic; extract it when reused. + +## Classes, constructors and initializers + +- **Use `const` constructors** (and `const` class instances) whenever possible. The `flutter_lints` analyzer enforces this. +- **Prefer `const` declarations** for local values and class members that never change. +- **Prefer constructors over static factory methods** unless the factory genuinely shines (this is enforced by the analyzer). +- **Prefer constructor injection** over positional parameter lists that are too long; use named optional parameters with defaults where needed. + +```dart +class HttpClient { + const HttpClient({this.timeout = const Duration(seconds: 10)}); + + final Duration timeout; +} +``` + +For immutable data classes, use compact generative constructors: + +```dart +class AnimeModel { + final String uuid; + final String shortName; + final List langTypes; + final List platformIds; + + AnimeModel(this.uuid, this.shortName, this.langTypes, this.platformIds); +} +``` + +## Imports + +- **Import symbols directly** (`import 'package:application/models/lang_type.dart';`); never reference a type by its fully-qualified name inline. +- Imports are a single statement per line, following the existing ordering (project imports first, then third-party). + +## Expression bodies + +- Prefer **expression bodies** (`=>`) for simple getters and short functions, matching the existing code. + +```dart +int get length => _animes.length + (_loading ? 8 : 0); + +AnimeModel? getOrNull(int index) => + index >= 0 && index < _animes.length ? _animes[index] : null; +``` + +## No singletons + +- **No global singletons or `Static` global state.** Dependencies are provided through the `provider` package (`MultiProvider`) in `main.dart` and injected via constructors. +- In widgets, read state with `context.watch()` and send events with `context.read()`. + +## Naming + +- Classes and interfaces: `PascalCase` (`AnimeViewModel`, `AppHorizontalListView`). +- Files and directories: `snake_case.dart` (`anime_view_model.dart`, `lang_type.dart`). +- Private instance fields: leading underscore (`_animes`, `_repository`). +- Local variables, parameters, methods, getters: `camelCase`. +- Constants: private-ish `camelCase` prefixed with `_` for instance values; `const` for true constants. +- Public domain interfaces (shared across layers) use `VerbNounViewModel` / `NounViewModel` naming (e.g. `LangTypeFilterViewModel`). + +## Flutter / Dart conventions + +- Widgets that hold state define a private `_State` companion class (`CatalogView` -> `_CatalogViewState`). +- Use `const` widget subtrees (`const Icon(...)`) whenever the subtree is static. +- Use `switch` expressions and modern Dart constructs (`is!`, `runtimeType`, spread) over legacy imperative equivalents where they read more cleanly. + +## Lints (enforced) + +The repository's `analysis_options.yaml` enables `flutter_lints` (`flutter_lints: ^6.0.0`) with these rules. Code MUST satisfy the analyzer at review time. The most relevant enforced rules: + +- `prefer_const_constructors` +- `prefer_const_constructors_in_immutables` +- `prefer_const_declarations` +- `prefer_const_literals_to_create_immutables` +- `prefer_constructors_over_static_methods` + +Always run `dart format lib` and `dart analyze lib` before submitting. `lib/models/*.g.dart` is excluded from analysis (generated code). \ No newline at end of file diff --git a/guidelines/LOCALIZATION.md b/guidelines/LOCALIZATION.md new file mode 100644 index 0000000..8265f9a --- /dev/null +++ b/guidelines/LOCALIZATION.md @@ -0,0 +1,68 @@ +# Localization Guide + +Localization uses Flutter's built-in **gen-l10n** (`flutter gen-l10n`). UI strings are resolved through `AppLocalizations`. + +> **Source language note:** the source/template localization is **French** (`app_fr.arb`). This is intentional even though all code is otherwise in English. Add/change UI strings in French; only add other `.arb` locale files when a translation is actually needed. + +## Configuration + +`l10n.yaml` at the repository root drives code generation: + +```yaml +arb-dir: lib/l10n +template-arb-file: app_fr.arb +output-localization-file: app_localizations.dart +format: true +``` + +- `arb-dir: lib/l10n` — where `.arb` files live. +- `template-arb-file: app_fr.arb` — the source French template. +- `output-localization-file: app_localizations.dart` — generated entry point in `lib/l10n/`. +- `format: true` — generated files are formatted. + +## Adding / changing a string + +1. Add or update the key in `lib/l10n/app_fr.arb`: + +```json +{ + "catalog": "Catalogue", + "search": "Rechercher..." +} +``` + +2. Regenerate the localizations with: + +```bash +flutter gen-l10n +``` + +3. Use the generated getter in widgets via `AppLocalizations.of(context)!`: + +```dart +SearchBar( + hintText: AppLocalizations.of(context)!.search, +) +``` + +## Wiring + +- `AppLocalizations` is wired once in `main.dart` (`MaterialApp.localizationsDelegates` + `supportedLocales`) and in `pubspec.yaml` (`flutter: generate: true`). Do not re-add delegates per widget. + +```dart +MaterialApp( + localizationsDelegates: const [ + ...GlobalMaterialLocalizations.delegates, + ...AppLocalizations.localizationsDelegates, + ], + supportedLocales: AppLocalizations.supportedLocales, + /* ... */ +) +``` + +## Rules + +- **Never hardcode user-facing strings** in widgets; always use `AppLocalizations.of(context)!`. +- Keep strings in `.arb`, not inline. +- Source-language strings are French; do not translate existing strings unless the feature changes. +- Do not hand-edit generated `app_localizations*.dart` files. \ No newline at end of file diff --git a/guidelines/MODELS.md b/guidelines/MODELS.md new file mode 100644 index 0000000..8cdcba5 --- /dev/null +++ b/guidelines/MODELS.md @@ -0,0 +1,71 @@ +# Models Guide + +Models are **immutable data-transfer objects (DTOs)** that mirror the API payloads. They live in `lib/models/` and are primarily generated with `json_serializable`. + +## Immutability + +- All fields are `final` and assigned through a generative constructor. +- Provide a `@JsonSerializable` factory `fromJson` so the model can be rebuilt from an API response. Do not use setters; changes mean a new instance (through the data layer). + +```dart +import 'package:application/models/anime_platform_model.dart'; +import 'package:application/models/lang_type.dart'; +import 'package:json_annotation/json_annotation.dart'; + +part 'anime_model.g.dart'; + +@JsonSerializable(createToJson: false) +class AnimeModel { + final String uuid; + final String shortName; + final List langTypes; + final List platformIds; + + AnimeModel(this.uuid, this.shortName, this.langTypes, this.platformIds); + + factory AnimeModel.fromJson(Map json) => + _$AnimeModelFromJson(json); +} +``` + +## Generated code + +- Each model has a sibling `.g.dart` file referenced via `part 'anime_model.g.dart';`. +- Generated files are produced by running `dart run build_runner build`. +- `analysis_options.yaml` excludes `lib/models/*.g.dart` from the analyzer. Do not hand-edit generated files. + +## Generic models + +- For generic models (e.g. pagination wrappers), enable `genericArgumentFactories: true` and pass the item factory through `fromJson`: + +```dart +@JsonSerializable(genericArgumentFactories: true, createToJson: false) +class PageableModel { + final List data; + final int page; + final int limit; + final int total; + + PageableModel(this.data, this.page, this.limit, this.total); + + factory PageableModel.fromJson( + Map json, + T Function(Object?) fromJsonT, + ) => _$PageableModelFromJson(json, fromJsonT); +} +``` + +## Enums + +- Use `@JsonEnum` with an explicit field renaming strategy to keep wire-format stable: + +```dart +@JsonEnum(fieldRename: FieldRename.screamingSnake) +enum LangType { subtitles, voice } +``` + +## API result wrapper + +- The API returns a `sealed class ApiResult` with `ApiSuccess` and `ApiFailure` subtypes. Models are returned inside `ApiSuccess`. Consumers exhaustively `switch` over the sealed class (see the [API Conventions](API_CONVENTIONS.md)). + +- 🌱 To consider for the rework: Flutter officially recommends generated immutable models with `freezed` or `built_value` (deep equality, copyWith, JSON). Not required now; `json_serializable` is the documented norm. \ No newline at end of file diff --git a/guidelines/PERFORMANCE.md b/guidelines/PERFORMANCE.md new file mode 100644 index 0000000..29d1cc5 --- /dev/null +++ b/guidelines/PERFORMANCE.md @@ -0,0 +1,25 @@ +# Performance Guide + +## Widgets + +- **Use `const` constructors** for any widget subtree that does not depend on mutable state. This lets Flutter skip rebuilds entirely. +- **Avoid work in `build`**: do not perform HTTP calls, heavy computation, or object allocation in `build()`. All data work belongs in ViewModels/Repositories. +- **Keep builds cheap and focused**: prefer small, single-purpose widgets; avoid rebuilding an entire page when only one region changed. +- **Use `context.watch()` at the narrowest scope**: watch where the value is read so only that widget rebuilds. + +## Data & state + +- **Avoid re-fetching when valid data is cached**: ViewModels use a `bypass` flag so `init(bypass: false)` returns early if data is already loaded. Do not unconditionally reload on every rebuild. +- **Bound pagination**: paginate long lists (`_limit = 15`) and stop when `_canLoadMore` is false or the end of data is reached. Do not load everything eagerly. +- **Deduplicate input**: guard against redundant concurrent fetches (e.g. skip while `_loading`). +- **Avoid janky scroll work**: keep scroll listeners light and guarded (`hasClients`, `maxScrollExtent` checks); schedule UI updates with `addPostFrameCallback` when needed. + +## Rendering + +- Use **skeleton placeholders** while loading (e.g. `AnimeSkeletonCard`) instead of spinners where it improves perceived performance and layout stability. +- **Reuse built-in widgets** (`MasonryGridView.builder` with `itemBuilder`) for virtualized, lazy lists instead of building all children eagerly. +- **Keep layout logical**: make responsive layout decisions in hit-tested leaf widgets, and avoid expensive layout passes in hot paths. + +## Logging + +- Use `debugPrint` for local debugging only. It is **not** stripped from release builds by default — guard any sensitive or verbose production logging behind `kReleaseMode`, and do **not** build production logging around requests unless explicitly required. Do not log sensitive data (see [Security Guide](SECURITY.md)). \ No newline at end of file diff --git a/guidelines/SECURITY.md b/guidelines/SECURITY.md new file mode 100644 index 0000000..84b0d9b --- /dev/null +++ b/guidelines/SECURITY.md @@ -0,0 +1,26 @@ +# Security Guide + +## Treat external data as untrusted + +- **All external input is untrusted**: API responses, query parameters, and user-entered data. +- Validate and sanitize incoming data before use. Do not blindly trust field types, lengths, or content from the API. +- Guard against malformed responses (wrong types, nulls, missing fields) so a bad payload fails safely instead of crashing or being rendered raw. + +## Do not expose or log sensitive information + +- **Never log secrets**, tokens, cookies, or personal identifiers. +- `debugPrint` is acceptable for local debugging, but must never print credentials, auth tokens, or user-private data (see the [Performance Guide](PERFORMANCE.md) for logging status). +- Do not include API keys or secrets in application source. Environment-driven values (e.g. `API_BASE_URL`) are injected via `--dart-define` / `String.fromEnvironment` (`EnvConfig`), never hardcoded. + +## Configuration security + +- Keep configuration in `lib/core/config/` (e.g. `EnvConfig`) and inject secrets at build time. Never commit real credentials. + +## Error handling + +- Fail safely on `ApiFailure`: handle success and failure branches on the sealed `ApiResult` exhaustively; do not silently ignore errors or leak raw stack traces to the UI. +- Keep user-facing errors generic; surface the actionable message without internal implementation details. + +## Think about impact + +- Any change that processes external data, renders remote content, or interacts with platform channels must be reviewed with these rules in mind. \ No newline at end of file diff --git a/guidelines/STATE_MANAGEMENT.md b/guidelines/STATE_MANAGEMENT.md new file mode 100644 index 0000000..c0adf82 --- /dev/null +++ b/guidelines/STATE_MANAGEMENT.md @@ -0,0 +1,70 @@ +# State Management Guide + +State management is handled by the **MVVM pattern** combined with the **provider** package for dependency injection and `ChangeNotifier`/`Listenable` for reactive updates. + +## Dependency injection with provider + +- Wire all application dependencies in `lib/main.dart` using `MultiProvider`. +- Dependencies are injected through constructors; never through global singletons or `Static` state. + +```dart +MultiProvider( + providers: [ + Provider(create: (_) => const HttpClient()), + Provider( + create: (context) => AnimeRepository(context.read()), + ), + ChangeNotifierProvider( + create: (context) => AnimeViewModel( + context.read(), + context.read(), + ), + ), + ], + child: const MyApp(), +) +``` + +- **ViewModels** (classes that widgets observe) use `ChangeNotifierProvider` so Dispose is handled by the provider framework. +- **Repositories / services / clients** use plain `Provider`. + +## Reading state and triggering events in widgets + +- Widgets read reactive state with `context.watch()`. This subscribes the widget to the current value and rebuilds it on `notifyListeners()`. +- Widgets trigger a one-shot event with `context.read()` (do not subscribe). + +```dart +Widget build(BuildContext context) { + final viewModel = context.watch(); + return Column(/* bind to viewModel */); +} +``` + +## Notifying the UI + +- A ViewModel calls `notifyListeners()` after mutating any state the UI observes (lists, flags, selections, loading state, page index). + +```dart +void _setLoading(bool loading) { + _loading = loading; + notifyListeners(); +} +``` + +## No logic in widgets + +- Widgets must NOT contain business logic. Only: + - simple conditionals that show/hide based on a ViewModel flag/field; + - layout logic (screen size / orientation); + - simple routing. +- All other logic belongs on the ViewModel (`init(bypass:)`, `onChanged(...)`, `setSelectedSimulcast(...)`, etc.). + +## Sharing state across ViewModels + +- Compose ViewModels: a ViewModel may depend on other ViewModels via constructor injection (`AnimeViewModel` depends on `SimulcastViewModel`). +- Factor shared behavior behind a `Listenable` interface (`LangTypeFilterViewModel`) implemented by multiple ViewModels. + +## Controller lifecycle + +- Widgets that own a controller (e.g. `ScrollController`) dispose it in their `dispose()` lifecycle, and swap it in `didUpdateWidget` when the injected controller changes. See `app_horizontal_list_view.dart`. +- 🌱 Good practice for the rework (not currently applied across all ViewModels): ViewModels that create controllers (`ScrollController`, `TextEditingController`, `PageController`) may need to own and `dispose()` them when the ViewModel is disposed. This is currently left to the rework; existing code does not yet dispose all ViewModel-held controllers. \ No newline at end of file diff --git a/guidelines/TESTING.md b/guidelines/TESTING.md new file mode 100644 index 0000000..eef803a --- /dev/null +++ b/guidelines/TESTING.md @@ -0,0 +1,83 @@ +# Testing Guide + +This guide defines the conventions for writing tests in this application. All test code, names, and comments are written in **English**. + +> **Status note:** there is currently no `test/` directory in `v2`. This guide is **aspirational**: it sets the conventions to apply as tests are introduced (during the rework). Follow it when adding tests. + +## Core stack + +- **`flutter_test`** (SDK) — the primary framework, alongside `package:test`. +- **Fakes by subclassing** — because repositories and `HttpClient` are concrete `const` classes (by design, no abstraction imposed), tests build **fakes that override the methods** of the real concrete classes. There is no mock framework requirement implied by the architecture. + +## How to inject fakes + +- **Unit tests for ViewModels**: inject a fake Repository. Since repositories are concrete classes, create a subclass that overrides the data methods. + +```dart +class FakeAnimeRepository extends AnimeRepository { + FakeAnimeRepository() : super(const HttpClient()); + + @override + Future>> getAnimes( + int page, + int limit, { + String? query, + String? simulcast, + List? langTypes, + }) async { + // return canned ApiSuccess / ApiFailure data + } +} +``` + +- **Unit tests for Repositories**: inject a fake `HttpClient` that overrides `get(...)` to return canned maps/lists, then assert the repository mapping. + +## Test structure + +- Use the **`Given / When / Then`** pattern, with comments delineating the three sections. +- Structure each test as: + - `// Given` — setup: build the fake, configure the state under test; + - `// When` — act: invoke the method / trigger the event; + - `// Then` — assert: verify the resulting state / calls. + +```dart +test('reload keeps current data when called with bypass', () async { + // Given + final viewModel = WeeklyViewModel(FakeWeeklyRepository()); + + // When + await viewModel.init(bypass: true); + + // Then + expect(viewModel.length, greaterThan(0)); +}); +``` + +## Unit tests + +- **Test every ViewModel method** in isolation (init, selection updates, pagination, error handling). +- **Test every Repository method**: with a fake `HttpClient`, verify the correct endpoint is requested, the right query parameters are attached, and the raw payload maps to the typed models. + +## Widget tests + +- Build views inside a **Provider harness** (`ChangeNotifierProvider.value` / `MultiProvider`) that provides a fake-injected ViewModel. +- Verify rendering, correct state binding, and that events trigger the expected ViewModel calls. + +```dart +testWidgets('shows the search bar', (tester) async { + await tester.pumpWidget( + ChangeNotifierProvider.value( + value: AnimeViewModel(FakeAnimeRepository(), FakeSimulcastViewModel()), + child: const CatalogView(), + ), + ); + // Then: find the SearchBar +}); +``` + +## What to cover + +- ViewModels: every public method, success and failure branches, loading state transitions. +- Repositories: endpoint, query params, mapping for success and failure. +- Widgets: key rendering states (data, empty, loading/skeleton) and interactions. +- Always exercise the **failure branches** (`ApiFailure`) to confirm error paths do not crash. \ No newline at end of file diff --git a/guidelines/THEME.md b/guidelines/THEME.md new file mode 100644 index 0000000..f11a0e1 --- /dev/null +++ b/guidelines/THEME.md @@ -0,0 +1,84 @@ +# Theme Guide + +Theming is centralized in `lib/core/theme/` and driven by Material 3 (`ThemeData`). The app supports **light** and **dark** modes. + +## Structure + +- `app_theme.dart` — `AppTheme` (a `sealed` class) exposing `light` and `dark` `ThemeData` getters, built by a private `_build(...)` helper. +- `app_theme_colors.dart` — `AppThemeExtension`, a `ThemeExtension` carrying custom color/image tokens not covered by `ThemeData`. +- Both are consumed through `Theme.of(context)`. + +## AppTheme + +`AppTheme.light` and `AppTheme.dark` each return a fully-built `ThemeData` (brightness, seed `colorScheme`, text theme, app bar / navigation bar / FAB / button / snack bar / icon / dialog / bottom sheet / divider themes, and the `AppThemeExtension`). + +```dart +sealed class AppTheme { + static ThemeData get light => _build( + brightness: Brightness.light, + iconImage: const AssetImage('assets/dark_icon.png'), + /* ... */ + ); + + static ThemeData get dark => _build( + brightness: Brightness.dark, + /* ... */ + ); + + static ThemeData _build({...}) { /* assemble ThemeData */ } +} +``` + +- Register both themes in `MaterialApp` via `theme:` and `darkTheme:`, and enable mode switching in `main.dart`: + +```dart +MaterialApp( + theme: AppTheme.light, + darkTheme: AppTheme.dark, + /* ... */ +) +``` + +## AppThemeExtension + +- Custom tokens that are not standard `ThemeData` go in a `ThemeExtension`. +- It MUST implement `copyWith` and `lerp` (required by `ThemeExtension`). +- Register it inside each theme's `extensions` list. + +```dart +@immutable +class AppThemeExtension extends ThemeExtension { + const AppThemeExtension({ + required this.inverseTextColor, + required this.iconImage, + }); + + final Color inverseTextColor; + final ImageProvider iconImage; + + @override + AppThemeExtension copyWith({Color? inverseTextColor, ImageProvider? iconImage}) { ... } + + @override + AppThemeExtension lerp(covariant AppThemeExtension? other, double t) { ... } +} +``` + +## Reading custom theme values in widgets + +- Access `ThemeData` values with `Theme.of(context)`. +- Access `AppThemeExtension` values via: + +```dart +final inverseTextColor = Theme.of(context) + .extension() + ?.inverseTextColor; +``` + +- To fall back on the system brightness when extension values are absent, use the null-safe accessor shown above. + +## Rules + +- **Do not hardcode colors or fonts in widgets.** Always read them from `Theme.of(context)` or `AppThemeExtension`. +- Keep all theme material (colors, images, fonts) inside `lib/core/theme/`; widgets consume it. +- The app font is **Satoshi** (declared in `pubspec.yaml` and set via `fontFamily` in `AppTheme._build`). Keep single source of truth for font constants in `_build`. \ No newline at end of file