refactor: reduce bundle size by moving assets to example app #452
Conversation
Reviewer's GuideRefactors the SymbolIcon widget so that symbol PNG assets are no longer bundled with the deriv_chart package but are instead expected from consuming apps, adding configuration hooks for custom asset locations while wiring the example app to host these assets and removing them from the library’s pubspec. Sequence diagram for SymbolIcon asset resolution and fallbacksequenceDiagram
participant ConsumerApp
participant SymbolIcon
participant FlutterAssetBundle
ConsumerApp->>SymbolIcon: create(symbolCode, assetPathBuilder?, placeholderPath?, package?)
activate SymbolIcon
alt custom assetPathBuilder provided
SymbolIcon->>SymbolIcon: build path via assetPathBuilder(symbolCode)
else default assetPathBuilder
SymbolIcon->>SymbolIcon: build path via getSymbolAssetPath(symbolCode)
end
SymbolIcon->>FlutterAssetBundle: load placeholder (placeholderPath or kDefaultPlaceholderPath, package)
SymbolIcon->>FlutterAssetBundle: load symbol image (resolved path, package)
alt image load succeeds
FlutterAssetBundle-->>SymbolIcon: symbol image bytes
SymbolIcon-->>ConsumerApp: render symbol image with fade
else image load fails
FlutterAssetBundle-->>SymbolIcon: error
SymbolIcon->>FlutterAssetBundle: load placeholder again (placeholderPath or kDefaultPlaceholderPath, package)
FlutterAssetBundle-->>SymbolIcon: placeholder image bytes
SymbolIcon-->>ConsumerApp: render placeholder image
end
deactivate SymbolIcon
Class diagram for updated SymbolIcon widgetclassDiagram
class SymbolIcon {
+SymbolIcon(symbolCode:String, width:double = 32, height:double = 32, fadeDuration:Duration = 50ms, assetPathBuilder:Function(String) = null, placeholderPath:String = null, package:String = null)
}
class AssetImage {
}
class FadeInImage {
}
class SymbolAssetPathHelper {
+getSymbolAssetPath(assetCode:String) String
}
class SymbolIconConstants {
+kDefaultPlaceholderPath : String = assets/icons/icon_placeholder.png
}
SymbolIcon --|> FadeInImage
SymbolIcon ..> AssetImage : uses
SymbolIcon ..> SymbolAssetPathHelper : uses
SymbolIcon ..> SymbolIconConstants : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
|
Claude finished @behnam-deriv's task —— View job PR Review: Asset Migration to Example AppI've completed my review of this PR. Overall, this is a well-executed refactoring that successfully moves assets to consumer apps while maintaining backward compatibility through sensible defaults. Here's my detailed feedback: ✅ High Priority - StrengthsExcellent Design Decisions
Code Quality
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `lib/src/widgets/market_selector/symbol_icon.dart:84-87` </location>
<code_context>
required String symbolCode,
double width = 32,
double height = 32,
Duration fadeDuration = const Duration(milliseconds: 50),
+ String Function(String)? assetPathBuilder,
+ String? placeholderPath,
+ String? package,
}) : super(
width: width,
</code_context>
<issue_to_address>
**issue:** The `fadeDuration` parameter is declared but never used in the `FadeInImage` configuration.
Since `fadeDuration` is exposed in the constructor but not passed to any `FadeInImage` property (e.g. `fadeInDuration` / `fadeOutDuration`), callers will expect it to affect the animation when it does not. Either wire it through to the relevant `FadeInImage` field(s) or remove the parameter to avoid a misleading API surface.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Claude finished @behnam-deriv's task —— View job PR Review: Asset Migration to Example AppI've completed a comprehensive review of this PR. This is a well-executed refactoring that successfully moves assets to consumer apps while maintaining backward compatibility. Here's my detailed analysis: Tasks
✅ StrengthsExcellent Design Decisions
Code Quality
Test Coverage Improvement
|
Asset Migration Guide: Moving Symbol Icons to Consumer Apps
Overview
This guide documents the migration of market selector symbol icons from the
deriv_chartpackage to consuming applications. This change reduces the bundle size of the chart library by moving consumer-specific assets out of the library.What Changed
Assets Moved
The following assets have been removed from the deriv_chart package and should now be provided by consuming applications:
assets/icons/icon_placeholder.png- Placeholder icon for symbol loadingassets/icons/symbols/*.png- All 108 trading symbol iconsAssets Retained
The following asset remains in the
deriv_chartpackage as it's used internally by the library:assets/fonts/quill_icons.ttf- Custom icon font for chart markersMigration Steps for Consumers
1. Copy Assets to Your App
Copy the following folders from the chart library to your application's assets folder:
2. Update Your
pubspec.yamlAdd the assets to your application's
pubspec.yaml:3. Update Your Code (If Needed)
The
SymbolIconwidget now defaults to loading assets from the consuming application instead of thederiv_chartpackage.Before (old usage):
After (no change needed):
Advanced Usage
Custom Asset Paths
If you want to organize your assets differently, you can customize the paths:
Assets in a Different Package
If you have a separate package for assets:
Custom Asset Builder
For more complex asset path logic:
Breaking Changes
For Library Consumers
pubspec.yamlWithout these steps, symbol icons will not display correctly.
For Library Maintainers
The
SymbolIconwidget has been updated with new optional parameters:assetPathBuilder: Custom function to build asset pathsplaceholderPath: Custom placeholder image pathpackage: Package name for assets (defaults to null for consumer app)Benefits
Reduced Bundle Size
Consumer Control
Flexibility
Troubleshooting
Icons Not Displaying
Problem: Symbol icons show as placeholder or error icon.
Solution:
assets/folderpubspec.yamlincludes the asset pathsflutter cleanandflutter pub getWrong Asset Paths
Problem: Assets are organized differently in your app.
Solution: Use the
assetPathBuilderparameter:Placeholder Not Found
Problem: Placeholder image not displaying correctly.
Solution: Specify custom placeholder path:
Example Implementation
See the
exampleapp in the repository for a complete implementation:example/assets/icons/example/pubspec.yamlexample/lib/main.dartQuestions?
If you have questions or issues with the migration, please:
SymbolIcondocumentationRelated Files
lib/src/widgets/market_selector/symbol_icon.dart- Updated widget implementationexample/pubspec.yaml- Example asset configurationpubspec.yaml- Library configuration (assets removed)Summary by Sourcery
Move symbol icon assets responsibility from the chart library to consuming apps and make the SymbolIcon widget configurable for external asset locations.
New Features:
Enhancements:
Build: