Should skipLoadingOnRefresh be off by default?

[AhmedLSayed9]

I understand that skipLoadingOnRefresh is on by default because the common usage is with RefreshIndicator.

But, there are quite a few other cases where it's needed to be used and I always find folks get confused of "why my loading is not showing" and we've to tell them it's a feature and you've to turn it off.

I'm rethinking that it's easier to turn it on for RefreshIndicator than turning it off for other use cases (we usually forget and catch this while testing the app).

Being off by default seems more natural, and we can make a reusable extension for RefreshIndicator:

extension AsyncValueExtension<T> on AsyncValue<T> {
  Widget refreshIndicatorWhen({
    required Widget Function(T data) data,
    required Widget Function(Object error, StackTrace stackTrace) error,
    required Widget Function() loading,
  }) {
    //Or use switch-case with `isRefreshing: true`
    return when(
      skipLoadingOnRefresh: true,
      data: data,
      error: error,
      loading: loading,
    );
  }
}

imo, this is more predictable and easier to spot than using an extension for the whole app to turn off skipLoadingOnRefresh or manually turn it off in different places.

[rrousselGit]

Why do you want to turn this off exactly?

Don't forget, when is outdated. Nowadays we use pattern matching. There's no such flag anymore.

[AhmedLSayed9]

Why do you want to turn this off exactly?

Because it produces unpredicted behavior (Loading not showing) when used with other use cases (that's not RefreshIndicator).

While the common use case is with RefreshIndicator, I still think it can be easily spotted and turned on by users for RefreshIndicator rather than fortuitously find that loading is not showing in random places (that do ref.invalidate for some reason) and discover that we forgot/have to manually turn off skipLoadingOnRefresh.

Don't forget, when is outdated. Nowadays we use pattern matching. There's no such flag anymore.

I'm not sure if this would change the argument, we still need to do the following:

switch (asyncValue) {
  case AsyncValue(:final error?): print('Error $error');
  case AsyncValue(:final value, hasData: true, isRefreshing: false): print(data);
  case _: print('loading');
}

[rrousselGit]

You're free to do:

switch (asyncValue) {
  case AsyncValue(isLoading: true): ...
  case AsyncValue(:final error?): ...,
  case AsyncValue(:final value!):
}