Skip to content
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

fix: Unable to handle Asset scheme #1227

Closed
wants to merge 1 commit into from
Closed

fix: Unable to handle Asset scheme #1227

wants to merge 1 commit into from

Conversation

devjiangzhou
Copy link

@devjiangzhou devjiangzhou commented Mar 17, 2022

我在本地 html 加载 asset 图片时,发现加载图片一直失败,发现 AssetImage 使用 assets://xxx 初始化失败
尝试修复,麻烦 review 下

@wssgcg1213
Copy link
Member

@devjiangzhou CI 有异常, 可以再 check 一下?

@@ -419,10 +420,14 @@ ImageProvider? defaultBlobProviderFactory(Uri uri, ImageProviderParams params) {

/// default ImageProviderFactory implementation of [ImageType.assets].
ImageProvider defaultAssetsProvider(Uri uri, ImageProviderParams params) {
String localPath = uri.toString();
Copy link
Member

Choose a reason for hiding this comment

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

这里可以把 uri 的 scheme 置空吗, 用 uri 的 replace 或者直接读 uri 的 path 属性出来

Copy link
Author

Choose a reason for hiding this comment

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

修改了一下,scheme 置空后,还需要将 // 删掉

Copy link
Member

Choose a reason for hiding this comment

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

  final String assetName = '${uri.host}${uri.path}'; // Ignore query and segment.
  return KrakenResizeImage.resizeIfNeeded(
    params.cachedWidth,
    params.cachedHeight,
    params.objectFit,
    AssetImage(assetName)
  );

字符串替换的成本相对高一点, 这里直接通过 uri 的 API 拼接就好了, query 和 segment 在本地文件里理论上没用

Copy link
Author

Choose a reason for hiding this comment

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

好的。

@answershuto answershuto self-requested a review March 21, 2022 06:08
@answershuto
Copy link
Member

加个单测吧

@wssgcg1213
Copy link
Member

wssgcg1213 commented Mar 21, 2022

加个单测吧

单测工程里面对 uri 做了处理, 所以不会进入本 PR 修改的代码 https://github.com/openkraken/kraken/blob/main/integration_tests/lib/main.dart#L35

所以看起来这里也需要处理一下

@devjiangzhou
Copy link
Author

devjiangzhou commented Mar 21, 2022

加个单测吧

单测工程里面对 uri 做了处理, 所以不会进入本 PR 修改的代码 https://github.com/openkraken/kraken/blob/main/integration_tests/lib/main.dart#L35

所以看起来这里也需要处理一下

以后提代码时注意下单测,还没详细看过 kraken 的单测 [doge]

@wssgcg1213 wssgcg1213 mentioned this pull request Mar 21, 2022
@wssgcg1213
Copy link
Member

@devjiangzhou 我在 #1232 修复了这个问题, 可以一起看一下

@devjiangzhou
Copy link
Author

devjiangzhou commented Mar 21, 2022

@devjiangzhou 我在 #1232 修复了这个问题, 可以一起看一下

好的,我先关了这个 PR 了,昨天发现的 css 的 asset 问题,发现也已经解决了。

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.

None yet

3 participants