Skip to content

feat: add available platforms badge with url launcher and improve grid layouts - #291

Draft
Ziedelth wants to merge 5 commits into
v2from
feat/available-platforms-badge
Draft

feat: add available platforms badge with url launcher and improve grid layouts#291
Ziedelth wants to merge 5 commits into
v2from
feat/available-platforms-badge

Conversation

@Ziedelth

Copy link
Copy Markdown
Contributor
  • Add url field to SourceModel for platform redirection
  • Create AvailablePlatformsBadge allowing direct launch or popup selection of streaming platforms
  • Modularize platform widgets into PlatformsStack and PlatformsBadge in lib/core/widgets/platforms/
  • Switch grid layouts in CatalogView, GroupedEpisodesView, and WeeklyView to AlignedGridView.count
  • Add 'availableOn' localization string and adjust theme text style

…d layouts

- Add url field to SourceModel for platform redirection
- Create AvailablePlatformsBadge allowing direct launch or popup selection of streaming platforms
- Modularize platform widgets into PlatformsStack and PlatformsBadge in lib/core/widgets/platforms/
- Switch grid layouts in CatalogView, GroupedEpisodesView, and WeeklyView to AlignedGridView.count
- Add 'availableOn' localization string and adjust theme text style

@Ziedelth Ziedelth left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Hermes

🔴 Changes requested: 3 guideline violations confirmed.

  • available_platforms_badge.dart:36: untrusted API URL is launched without validation.
  • available_platforms_badge.dart:57: launch policy and retry behavior live in the widget.
  • catalog_view.dart:88: widget hardcodes Colors.black.

dart format --output=none --set-exit-if-changed lib and dart analyze lib pass.

⚠️ Points incertains / à clarifier
  • available_platforms_badge.dart:91: singleWhere throws if one platform has multiple source URLs. The behavior is confirmed for duplicate matches, but the API contract does not establish whether duplicates are possible. Prefer grouping/selecting sources without assuming one URL per platform.
  • available_platforms_badge.dart:80: an empty sources iterable reaches showMenu(items: []), whose implementation asserts items.isNotEmpty. The crash is reproduced, but it is unclear whether the API guarantees a non-empty list. Hide/disable the badge or guard the empty case.


for (final mode in modes) {
try {
if (await launchUrl(.parse(url), mode: mode)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 SourceModel.url comes from the API, but this passes it directly through Uri.parse to the platform launcher. Uri.parse can throw for malformed input and this code also accepts arbitrary schemes; the handler only catches PlatformException. This violates SECURITY.md's requirement to validate external input and fail safely. Parse with Uri.tryParse, allow only the expected scheme(s), and handle rejection without invoking launchUrl.

Comment thread lib/core/widgets/platforms/available_platforms_badge.dart Outdated
Comment thread lib/views/catalog/catalog_view.dart Outdated
for (final platform in platforms)
PopupMenuItem(
onTap: () {
final source = sources.singleWhere(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug potentiel confirmé : sources.singleWhere((s) => s.platform.name == platform.name) peut lever une StateError (Too many elements). _platforms est dédupliqué par plateforme (via PlatformModel.== sur name), mais sources ne l'est pas. Un épisode peut avoir plusieurs sources sur la même plateforme (ex. Crunchyroll VF + Crunchyroll VOSTFR). Quand un épisode a une plateforme dupliquée + une autre plateforme (→ le menu s'affiche), sélectionner la plateforme dupliquée fait planter singleWhere au runtime. Utiliser une source représentative (firstWhere) ou grouper explicitement au lieu de singleWhere.

Comment thread lib/views/grouped_episodes/widgets/grouped_episode_card.dart Outdated
@Ziedelth

Copy link
Copy Markdown
Contributor Author

Review consolidée Hermes — PR #291

Consolidation de 3 reviewers + validations locales. dart format lib ✅ et dart analyze lib ✅ (aucun problème).

Jalons (inline déjà posés)

  • available_platforms_badge.dart:36 — URL API lancée sans validation (Uri.parse peut lever, schémas arbitraires) → Uri.tryParse + whitelist scheme.
  • available_platforms_badge.dart:57 — logique de lancement + fallback dans le widget (violation MVVM « widgets dumb ») → déplacer dans service/ViewModel injecté.
  • catalog_view.dart:88Colors.black codé en dur (violation THEME) → token via Theme/AppThemeExtension.
  • available_platforms_badge.dart:91singleWhereStateError si une plateforme a plusieurs sources (VF+VOSTFR).
  • grouped_episode_card.dart:37GlobalKey() créé dans build (anti-pattern, positionnement du menu fragile).
⚠️ À clarifier / non bloquant
  • release_card.dart : la PR retire PlatformsBadge du Calendrier → les plateformes disparaissent des sorties hebdo. Perte d'info volontaire ou régression ?
  • available_platforms_badge.dart:80 : liste sources vide → showMenu(items: []) → assertion items.isNotEmpty. À confirmer si l'API garantit des sources non vides.
  • debugPrint (32, 40, 46) : non gardés par !kReleaseMode (log actif en release) — cohérent avec l'existant mais contraire à PERF/SECURITY.
  • URL logo shikkanime.fr codée en dur : code déplacé (pas introduit ici) — à migrer vers EnvConfig plus tard.

Approuvé / OK

  • Refactor PlatformsStack/PlatformsBadge dans lib/core/widgets/platforms/ (cohérent core/widgets).
  • url_launcher déjà dans pubspec (pas de dépendance manquante).
  • AlignedGridView.count + Align.topCenter : correct pour l'alignement des cartes.
  • Champ url ajouté proprement à SourceModel (immuable, généré).

Verdict : changements à faire avant merge — points bloquants : URL non validée, logique dans le widget, Colors.black, singleWhere (crash), GlobalKey inline.

Comment thread lib/core/widgets/platforms/platforms_stack.dart Outdated
Ziedelth and others added 4 commits August 20, 2026 13:47
- Add const PlatformLaunchService (URL validation, launch modes, fallback)
- Inject service into GroupedEpisodeViewModel and WeeklyViewModel
- Expose onSourcePress event on both ViewModels
- Make AvailablePlatformsBadge, GroupedEpisodeCard and ReleaseCard passive
  widgets receiving onSourcePress closure
- Wire PlatformLaunchService in MultiProvider
- Apply dart format
- FakeUrlLauncherPlatform and ThrowingUrlLauncherPlatform fakes
- Cover URL validation (malformed, non-https), first-mode launch,
  mode fallback loop, and PlatformException recovery
- Cover GroupedEpisodeViewModel and WeeklyViewModel delegation
  to the launch service with Given/When/Then structure
- Add url_launcher_platform_interface as dev dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant