Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jun 8, 2019

Description

Both NetworkImage and AssetImage use dart:io functionality that cannot be supported on the web. Similar to what we've done with compute, bitfield, and defaultTargetPlatform we can separate them into io and web implementations, 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.

@jonahwilliams jonahwilliams added framework flutter/packages/flutter repository. See also f: labels. platform-web Web applications specifically labels Jun 8, 2019
@jonahwilliams
Copy link
Contributor Author

cc @yjbanov @hterkelsen

@jonahwilliams
Copy link
Contributor Author

also cc @goderbauer

@jonahwilliams jonahwilliams changed the title [Discuss] Separate web and io implementations of network and asset image [Discuss] Separate web and io implementations of network image Jun 10, 2019
@jonahwilliams jonahwilliams changed the title [Discuss] Separate web and io implementations of network image Separate web and io implementations of network image Jun 10, 2019
Copy link
Contributor

@harryterkelsen harryterkelsen left a 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

@jonahwilliams jonahwilliams requested a review from goderbauer June 11, 2019 23:45
@Piinks Piinks added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Jun 12, 2019
@jonahwilliams jonahwilliams requested a review from goderbauer June 13, 2019 06:00
Copy link
Member

@goderbauer goderbauer left a 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

@jonahwilliams jonahwilliams merged commit dfa39f3 into flutter:master Jun 15, 2019
@jonahwilliams jonahwilliams deleted the add_network_image_impl branch June 15, 2019 17:21
tango5614 added a commit to tango5614/flutter that referenced this pull request Jun 17, 2019
* 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)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels. platform-web Web applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants