[feat] RefreshIndicator.noSpinner implementation

[opxdelwin]

New RefreshIndicator.noSpinner constructor to override and disable default spinner for custom handling.

New onModeChange function to keep a track of events performed by RefreshIndicator such as drag, armed, refresh, cancelled for custom handling by developers.
onModeChange is by default set to null for RefreshIndicator & RefreshIndicator.adaptive as UI gets updated as per.

Contributions to more (and better) tests would be equally appreciated!

Fixes #132775

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

Minimal Reproducible code

import 'package:flutter/material.dart';

/// Flutter code sample for [RefreshIndicator].

void main() => runApp(const RefreshIndicatorExampleApp());

class RefreshIndicatorExampleApp extends StatelessWidget {
  const RefreshIndicatorExampleApp({super.key});

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      home: RefreshIndicatorExample(),
    );
  }
}

class RefreshIndicatorExample extends StatelessWidget {
  const RefreshIndicatorExample({super.key});

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('RefreshIndicator Sample'),
      ),
      body: RefreshIndicator.noSpinner(
        onModeChange: (mode) => print(mode ?? ''),
        onRefresh: () async {
          print("calling on refresh");
          return Future<void>.delayed(const Duration(seconds: 3));
        },
        notificationPredicate: (ScrollNotification notification) {
          return notification.depth == 1;
        },
        child: SingleChildScrollView(
          child: Column(
            children: <Widget>[
              Container(
                height: 100,
                alignment: Alignment.center,
                color: Colors.pink[100],
                child: Column(
                  mainAxisAlignment: MainAxisAlignment.center,
                  children: <Widget>[
                    Text(
                      'Pull down here',
                      style: Theme.of(context).textTheme.headlineMedium,
                    ),
                    const Text("RefreshIndicator won't trigger"),
                  ],
                ),
              ),
              Container(
                color: Colors.green[100],
                child: ListView.builder(
                  shrinkWrap: true,
                  itemCount: 25,
                  itemBuilder: (BuildContext context, int index) {
                    return const ListTile(
                      title: Text('Pull down here'),
                      subtitle: Text('RefreshIndicator will trigger'),
                    );
                  },
                ),
              ),
            ],
          ),
        ),
      ),
    );
  }
}

[opxdelwin]

Anyone with iOS devices may test this implementation as well.

[flutter-dashboard]

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[opxdelwin]

@danagbemava-nc @HansMuller please look into this pull.

[bernaferrari]

I personally would name it none instead of noSpinner, but let's see what the reviewer says

[HansMuller]

Making the refresh indicator more flexible, so that you can change the animated "indicator" to something else seems reasonable. I'd expected to see a widget valued callback, something like buildRefreshIndicator(BuildContext, Animation), which would enable one to replace the default animation. The notification approach you've taken seems to work nicely for a situation where the indicator isn't stacked on top of the scrollable the way the default one is, but is just some unrelated part of the UI. Can you find a simple way to satisfy both use cases?

[opxdelwin]

All checks are green! 🟢

@bernaferrari pointed out that none might be a better option, so I'll be tweaking that toward the end.

Regarding @HansMuller's suggestion, yup, we can offer an alternative to the spinner. Thinking of taking in a custom animation (maybe an icon?). I'm considering adding a separate constructor, maybe RefreshIndicator.custom().

Let me know your thoughts on how to move forward. Implementation might take a bit as I'm juggling multiple projects. Thanks for understanding!

[HansMuller]

Sorry about the long review delay; I plead the holidays :-)

This is looking good. With a few small tweaks and an API doc (DartPad) example I think we could land this soon.

[opxdelwin]

Hey, sorry for the long disappearance, Had a lot of assignments and tests.

I've added an example flutter app, still unsure on how to update https://api.flutter.dev/flutter/material/RefreshIndicator-class.html. Please do guide me.

[HansMuller]

Just some comments about the example (which is headed in the right direction).

[HansMuller]

For the onStatusChange and onRefresh cases it would be best to do something that affects the UI rather than printing messages to the console. For the onRefresh case you could temporarily change the UI for 3 seconds.

[HansMuller]

We don't want to encourage developers to use shrinkWrap. This can be rewritten with CustomScrollView:

CustomScrollView(
  slivers: <Widget>[
    SliverToBoxAdapter(
      child: Container(
        height: 100,
        alignment: Alignment.center,
        color: Colors.pink[100],
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Text(
              'Pull down here',
              style: Theme.of(context).textTheme.headlineMedium,
            ),
            const Text("RefreshIndicator won't trigger"),
          ],
        ),
      ),
    ),
    DecoratedSliver(
      decoration: BoxDecoration(color: Colors.green[100]),
      sliver: SliverList(
        delegate: SliverChildBuilderDelegate(
          (BuildContext context, int index) {
            return const ListTile(
              title: Text('Pull down here'),
              subtitle: Text('RefreshIndicator will trigger'),
            );
          },
          childCount: 25,
        ),
      ),
    ),
  ],
)

[HansMuller]

This check?

Better to explain this a little more directly: To ensure that we're notified only when the nested scroll view is scrolled we check the notification's depth. [Now explain what the default is and why we don't want that, etc.]

[opxdelwin]

I copied part of the sample from another refresh indicator example, maybe I'll just remove this part?

[HansMuller]

You'll need a comment here that links to the example. Something like this (from TextButton):

/// {@tool dartpad}
/// This sample shows various ways to configure TextButtons, from the
/// simplest default appearance to versions that don't resemble
/// Material Design at all.
///
/// ** See code in examples/api/lib/material/text_button/text_button.0.dart **
/// {@end-tool}

[opxdelwin]

Is this for the doc generation part?

[HansMuller]

You'll also need a test for the example. Put it in examples/api/test/material/refresh_indicator/refresh_indicator.2_test.dart

The test just needs to exercise the example enough to know that the basics are working.

[Piinks]

Hi @opxdelwin would you like to continue working on this PR?

[Piinks]

Thank you for your contribution. I'm going to close this PR for now since there are outstanding comments, just to get this off our PR review queue. Please don't hesitate to re-open or submit a new PR if you have the time to address the review comments. Thanks!