[Regression] BoxFit cover for placeholder and error builders

[davidmartos96]

🔙 Regression

Old (and correct) behavior

When using Image.asset with BoxFit cover as placeholder or error builders, the fit is applied correctly. (On cached_network_image v2.2.0)

Current behavior

The specified BoxFit for the placeholder and error builders is not applied.

Reproduction steps

Simple example. In a real scenario the placeholder would be an Image.asset widget, but this is enough to see the regression.

Container(
    color: Colors.red,
    child: OctoImage(
      image: NetworkImage('...'),
      placeholderBuilder: (context) {
        return Image.network(
          "https://blurha.sh/assets/images/img1.jpg",
          fit: BoxFit.cover,
        );
      },
      errorBuilder: (context, e, s) {
        return Image.network(
          "https://blurha.sh/assets/images/img1.jpg",
          fit: BoxFit.cover,
        );
      },
      fit: BoxFit.cover,
      height: 100,
      width: 100,
    ),
  ),

Configuration

Version: 0.2.1

[renefloor]

@davidmartos96 I fixed it in 0.3.0. Will update CachedNetworkImage this week.

[davidmartos96]

@renefloor Thanks! I took a look at the source code and example project and this fix would be a breaking change for octo image users. For instance, in the example, with the fix, the CircularProgressIndicator will need to be wrapped around a Center, as usual.

For the time being I was able to achieve the expected behavior wrapping my placeholders with a SizedBox.expand

[renefloor]

Yes I know, I also put BREAKING in the changelog for that: https://pub.dev/packages/octo_image/changelog I indeed centered the progressIndicator, the CircularProgressIndicator as Placeholder was already centered: https://github.com/Baseflow/octo_image/commit/321306990a711e6ac78a7a5e35da14c43816d34f#diff-f8aabd1e3179bee091b84bf05b2b3fcf

I still think it is better to not center in the widget as that also doesn't happen for example in a container.

[davidmartos96]

I indeed centered the progressIndicator

Oh I didn't see that commit.

I still think it is better to not center in the widget as that also doesn't happen for example in a container.

Yes, I agree. It is also the behaviour of the Image widget in Flutter.

[renefloor]

Oh I didn't see that commit.

I can imagine, that wasn't part of the PR. I found a bit later that that one didn't center.