-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Separate web and io implementations of network image #34112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separate web and io implementations of network image #34112
Conversation
|
also cc @goderbauer |
harryterkelsen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, as long as the use of File and Directory to get the asset directory path were just because that is a convenient way to implement it, and not because they needed platform-specific handling of path separators or something
goderbauer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with some nits
* master: retry on HttpException (flutter#34526) 308c113 Roll src/third_party/skia 057a32d9a2c8..5fe7429babe2 (1 commits) (flutter/engine#9342) (flutter#34583) Add back ability to override the local engine in Gradle (flutter#34460) Added customizable padding for the segmented controll (flutter#34555) 466a1d8 Roll src/third_party/skia 3721688f64a5..057a32d9a2c8 (1 commits) (flutter/engine#9340) (flutter#34558) Handles parsing APK manifests with additional namespaces or attributes (flutter#34535) add route information to Flutter.Navigation events (flutter#34508) Separate web and io implementations of network image (flutter#34112) pass .packages path to snapshot invocation (flutter#34517) [Material] Fix TextDirection and selected thumb for RangeSliderThumbShape and RangeSliderValueIndicatorShape (flutter#34501) redux of a change to use new engine APIs for Flutter.Frame events (flutter#34521) Roll engine b0757e6..f44b7b5 (4 commits) (flutter#34533) Roll engine 2589785..b0757e6 (6 commits) (flutter#34522) Revert "Revert "redux of a change to use new engine APIs for Flutter.Frame events (flutter#34365)" (flutter#34514)" (flutter#34530) Make sure fab semantics end up on top (flutter#34512) Revert "redux of a change to use new engine APIs for Flutter.Frame events (flutter#34365)" (flutter#34514) redux of a change to use new engine APIs for Flutter.Frame events (flutter#34365)
Description
Both NetworkImage and AssetImage use
dart:iofunctionality that cannot be supported on the web. Similar to what we've done withcompute,bitfield, anddefaultTargetPlatformwe can separate them intoioandwebimplementations, with the latter coming directly from flutter_web.To ensure the interfaces stay consistent, the original class becomes abstract and then redirects to an imported subclass.
There are also other ways these could be split up, by making certain functions public and then importing these.
#34082
Update: The usage of dart:io by AssetImage seems trivial and can be replaced, assuming that we're dealing with file URIs and not file paths.