quire-io / scroll-to-index

scroll to index with fixed/variable row height inside Flutter scrollable widget
MIT License
511 stars 104 forks source link

setState cancels highlight #16

Closed ovidiu-anghelina closed 4 years ago

ovidiu-anghelina commented 4 years ago

Rebuilding the widget that is being highlighted cancels the highlight.

I have a use case where once the user tries to submit a form, all input fields should be validated and should display the corresponding error messages, and the first input field with an error should also be highlighted. This requires me to call setState() as well as highlight() once all input fields have been validated. This causes the highlighted widget to be rebuilt as well, which in turn cancels the highlight. Actually, because I call both of those functions at the same time, the highlight is not displayed at all.

Here is some sample code with 2 buttons, one of which calls setState() 1 second after the highlight is triggered, and one that calls them both at the same time:

import 'dart:async';

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

void main() => runApp(MyApp());

class MyApp extends StatefulWidget {
  @override
  _MyAppState createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  final AutoScrollController controller = AutoScrollController();
  final int index = 0;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        body: Center(
          child: ListView(
            controller: controller,
            shrinkWrap: true,
            children: <Widget>[
              AutoScrollTag(
                key: ValueKey(index),
                controller: controller,
                index: index,
                highlightColor: Colors.redAccent,
                child: Container(
                  height: 50,
                  decoration: BoxDecoration(
                    border: Border.all()
                  ),
                ),
              ),
              RaisedButton(
                child: const Text("Highlight (setState later)"),
                onPressed: () {
                  controller.highlight(index);

                  Future.delayed(const Duration(seconds: 1), () {
                    setState(() {});
                  });
                },
              ),
              RaisedButton(
                child: const Text("Highlight (setState now)"),
                onPressed: () {
                  controller.highlight(index);

                  setState(() {});
                },
              ),
            ],
          ),
        ),
      ),
    );
  }
}
jerrywell commented 4 years ago

i believe you should maintain a validation set for the the background color by yourself. it's because the highlight function is used for just highlight and done. if you move any of list view item outside viewport, the widget state will be destroyed and the highlight state will be gone. you should keep the validation state in your own set and pass the information to your item widget. if the content in the validation set is changed, you need to call setState to rebuild the whole list view.

ovidiu-anghelina commented 4 years ago

I don't think I understood your explanation. As you can see from the example above, this is not about the item going outside of the viewport - I wouldn't expect the highlight to be maintained in that scenario as the listview would have recycled the highlighted widget and its state. This is about simply rebuilding the widget while it's highlighted.

I copied your code and simply commented out _cancelController(); on line 494 within didUpdateWidget, and it fixed the issue. You are already conditionally calling unregister within didUpdateWidget if the index is changed, which makes sense. But why would you unconditionally cancel the controller on each rebuild? If you have specific use cases where that is needed, can you maybe make it conditional? As you can see in the example above I am not changing any of the AutoScrollTag properties across rebuilds, and I can't see any valid reason to cancel the animation controller.

jerrywell commented 4 years ago

got your point :) that make sense. I have fixed this issue. thanks for your clarifying.