Baseflow / octo_image

A multifunctional Flutter image widget
https://baseflow.com/
MIT License
156 stars 23 forks source link

Bugfix/rebuild image widget #18

Closed renefloor closed 3 years ago

renefloor commented 3 years ago

:sparkles: What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Bugfix for #17

:arrow_heading_down: What is the current behavior?

Currently OctoWidget is using the result of the frameBuilder of the Image widget as placeholder for the new image if you use gaplessPlayback. We should not keep that widget and reuse it as it might be disposed already. We should rebuild the widget.

:new: What is the new behavior (if this is a feature change)?

The new behaviour rebuilds the widget when using gaplessPlayback. To make sure it does use the old widget properties by storing them in the ImageHandler. The ImageHandler can thus rebuild the widget based on the old properties.

:boom: Does this PR introduce a breaking change?

Behaviour changes, but more like you would expect it to happen.

:bug: Recommendations for testing

I changed the example in #17 a bit with a constructed ImageBuilder function. This way the old index is visible while it fades to the new widget with the new index:

import 'dart:async';

import 'package:flutter/material.dart';
import 'package:octo_image/octo_image.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      home: MyHomePage(),
    );
  }
}

class MyHomePage extends StatefulWidget {
  @override
  _MyHomePageState createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  int index = 0;

  @override
  void initState() {
    super.initState();
    Timer.periodic(
      Duration(seconds: 10),
      (timer) {
        setState(() {
          ++index;
        });
      },
    );
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: OctoImage(
        fadeInDuration: Duration(seconds: 4),
        fadeOutDuration: Duration(seconds: 4),
        image: NetworkImage(
            'https://via.placeholder.com/300.png/09f/fff?index=$index'),
        imageBuilder: ImageBuilderFactory(index).build,
        placeholderBuilder: (_) => Container(color: Colors.red),
        gaplessPlayback: true,
      ),
    );
  }
}

class ImageBuilderFactory {
  final int index;

  ImageBuilderFactory(this.index);

  Widget build(BuildContext context, Widget child) {
    return Stack(
      alignment: Alignment.bottomCenter,
      children: [
        child,
        Text(index.toString()),
      ],
    );
  }
}

:memo: Links to relevant issues/docs

Fixes #17 Should fix https://github.com/Baseflow/flutter_cached_network_image/issues/513 as well.

:thinking: Checklist before submitting

codecov[bot] commented 3 years ago

Codecov Report

Merging #18 (1e14d4a) into develop (0fdd728) will increase coverage by 3.60%. The diff coverage is 57.77%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #18      +/-   ##
===========================================
+ Coverage    42.78%   46.38%   +3.60%     
===========================================
  Files            7        8       +1     
  Lines          187      235      +48     
===========================================
+ Hits            80      109      +29     
- Misses         107      126      +19     
Impacted Files Coverage Δ
lib/src/image/fade_widget.dart 3.44% <ø> (ø)
lib/src/image/image.dart 48.48% <48.93%> (ø)
lib/src/image/image_handler.dart 62.50% <62.50%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6733515...1e14d4a. Read the comment docs.