Skip to content

feat: add loading widget support for image and avatar components - #2342

Merged
usmanvrtx merged 3 commits into
android_TV_implementationfrom
feat/loading-widget-image-avatar
Aug 13, 2026
Merged

feat: add loading widget support for image and avatar components#2342
usmanvrtx merged 3 commits into
android_TV_implementationfrom
feat/loading-widget-image-avatar

Conversation

@usmanvrtx

Copy link
Copy Markdown
Contributor

Description

This PR adds a loadingWidget property to the Image and Avatar components so developers can display a custom widget while a network image is loading. Previously only a colored placeholder box could be shown during the loading state. It also introduces a networkCacheManager option that lets developers supply a custom cache manager for network image requests.

Related Issue

N/A

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)

What Has Changed

  • Added loadingWidget and networkCacheManager fields, getters, and setters to the EnsembleImage and Avatar controllers
  • When loadingWidget is set, it is built through the scope manager and shown as the placeholder while a network image loads; otherwise the existing colored placeholder is used
  • Refactored image placeholder logic into reusable _buildLoadingPlaceholder / _buildLoadingWidget helpers in widget/image.dart
  • Extended the BackgroundImage model with loadingWidget and cacheManager support
  • Threaded loadingWidget through the shared framework Image widget's placeholder builder
  • Added networkCacheManager support so a custom BaseCacheManager can be passed for network image loading

How to Test

  1. Add loadingWidget to an Image or Avatar definition and load a slow network image — the custom widget should render while loading
  2. Verify the existing placeholderColor placeholder still renders when loadingWidget is not set
  3. Optionally pass a networkCacheManager and confirm network images use it
  4. Run the existing image tests: flutter test modules/ensemble/test/widget/image_widget_test.dart

Screenshots / Videos

N/A

Checklist

  • I have run flutter analyze and addressed any new warnings
  • I have run flutter test and all tests pass
  • I have tested my changes on the relevant platform(s)
  • I have updated documentation if needed
  • My changes do not introduce new warnings or errors

@usmanvrtx
usmanvrtx requested a review from TheNoumanDev August 13, 2026 08:48
@usmanvrtx usmanvrtx self-assigned this Aug 13, 2026

@TheNoumanDev TheNoumanDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Following is Claude Review, please go though each point and see if it make sense, fix it, and if doesn't reply on it, Thanks

🔧 What we're fixing (the two that actually bite)

- #1 — Permanent spinner on empty source (widget/image.dart:132)
  - Empty/absent source now shows loadingWidget forever, because that branch is the terminal "no image" state, not a real loading state — no fetch is pending to replace it.
  - Fix: keep the empty-source branch as a neutral blank placeholder; only show loadingWidget on the real network-loading path.
- #2 — Loading widget rebuilt every frame (widget/image.dart:293)
  - The placeholder callback calls buildWidgetFromDefinition(...) on every rebuild → animated loaders reset, and any id in the loader gets re-registered in scope repeatedly.
  - Avatar already does it right (builds once, reuses). Fix: build once and reuse here too.

📋 The remaining points (lower priority, worth cleaning up)

- #3 — Redirect blocking silently dropped (image.dart:288, avatar.dart:316) — if a caller supplies networkCacheManager and allowRedirect:false, the redirect guard is bypassed. Only reachable programmatically today, but it voids a security setting.
- #4 — networkCacheManager YAML setter is a no-op (image.dart:73, avatar.dart:82) — BaseCacheManager can't be built from YAML, so any YAML value becomes null silently. Document as native-only or remove.
- #5 — Avatar lost {super.key} (avatar.dart:27) — now the only EnsembleWidget subclass that can't take a key. No caller passes one today, so latent — looks like accidental collateral; revert.
- #6 — BackgroundImage params are dead plumbing (model.dart:62) — loadingWidget/cacheManager were added to the model but getBackgroundImage never passes them, so they're always null. Wire up or drop.

@usmanvrtx
usmanvrtx force-pushed the feat/loading-widget-image-avatar branch from 394f038 to cbf9616 Compare August 13, 2026 13:09
@usmanvrtx
usmanvrtx requested a review from TheNoumanDev August 13, 2026 13:09
@usmanvrtx
usmanvrtx merged commit 3db5dab into android_TV_implementation Aug 13, 2026
3 checks passed
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.

2 participants