feat/ add favorite parkings feature

[tomasz-trela]

I used shared_preferences and I updated version to 2.3.2

[simon-the-shark]

Yeah i actually liked your code. I have suggestions for you, but previous version could be considered enough for this simple logic. Tell me what you think of the suggestions

About the linter warning it's actually funny case, but there's a simple fix with Future.microtask or just use riverpod instead

[simon-the-shark]

So my first note is that this can go to separate file as it's pretty much separate concept

[simon-the-shark]

So yeah, this is actually funny case here. The linter has problem with it, even though there's nothing wrong with it. Easy fix is to just use Future.microtask like this:

future: Future.microtask(_loadPreference),

and linter won't make any more problems. So this linter rule is usefull in many cases but here it actually raises warning without a greater reason I think so you can alway just shut it off with a "disable comment"

Alternative

So alternatively we can just use riverpod and make a global shared prefs singleton and we can use prefs using ref.watch instead of FutureBuilder

@Riverpod(keepAlive: true)
Future<SharedPreferences> sharedPreferencesSingleton(
  SharedPreferencesSingletonRef ref,
) {
  return SharedPreferences.getInstance();
}

[simon-the-shark]

this actually I think could reuse FavouriteIcon

[simon-the-shark]

So basically this whole statefull widget is actually well crafted I think. It's simple logic so you could say it's enough for this case. But I just wanted to show you how you can more separate logic from UI here using riverpod. Maybe in this simple case this isn't really necessary and your solution was pretty much enough, but I think it's much cleaner if you separate disk operation to "local repository" like this:

part "local_fav_parking_repository.g.dart";

@riverpod
class LocalFavParkingsRepository extends _$LocalFavParkingsRepository {
  static const _repoPrefix =
      "fav_parking_"; // actually if you use shared prefs, it's good idea to use some prefix to avoid conflicts with other features. If you've used a real DB then you would have separate collection/datatable for this feature, but shared prefs is just a bug of key-value pairs so some prefix is a good idea.

  String get _key => _repoPrefix + parkingId;

  @override
  bool? build(String parkingId) {
    final prefs = ref.watch(sharedPreferencesSingletonProvider);
    return switch (prefs) {
      AsyncError() => null,
      AsyncValue(:final value) => value?.getBool(_key) ?? false,
    };
  }

  Future<void> toggle() async { // if you really would want an "enteprise edition" of architecture here, this should only accept bool and save it and toggling logic should be handled in some controller instance (another riverpod layer). But yeah, this would be a real overkill for just one bool xD
    final currState = state ?? false;
    final newState = !currState;
    state = newState;
    final prefs = await ref.read(sharedPreferencesSingletonProvider.future);
    await prefs.setBool(_key, newState);
  }
}

then pretty much your widget gets very simplified and only defines how your UI reacts to specific state and doesn't care about handling state. Clear and nice separation of concerns imo. But yeah, I know this feature is not too heavy, but I think it's good idea to always separate those layers.

class FavouriteParkingWidget extends ConsumerWidget {
  const FavouriteParkingWidget(this.parking);
  final Parking parking;
  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final favController =
        ref.watch(localFavParkingsRepositoryProvider(parking.id).notifier);
    final isFavorite =
        ref.watch(localFavParkingsRepositoryProvider(parking.id));
    return GestureDetector(
      onTap: favController.toggle,
      child: isFavorite == null
          ? const FavouriteIcon(
              icon: Icons.error,
            )
          : FavouriteIcon(
              icon:
                  isFavorite ? Icons.favorite : Icons.favorite_border_outlined,
            ),
    );
  }
}

[simon-the-shark]

tell me what you think of it

[tomasz-trela]

I like this changes

[simon-the-shark]

And actually favourite parkings should be always on top of the list - but I guess this wasn't really specifed in the task, so we can make it a new task and merge this one, once you check everything out

[tomasz-trela]

And actually favourite parkings should be always on top of the list - but I guess this wasn't really specifed in the task, so we can make it a new task and merge this one, once you check everything out

I think it is good idea

[simon-the-shark]

@TTomaszPWR I actually run the program and quickly saw that it's better to use IconButton widget instead of Gesture Detector here, cause it gives us this nice splash when you hit the button :)

Nagranie.z.ekranu.2024-09-5.o.00.03.50.mov

[simon-the-shark]

@TTomaszPWR tell me when you think we are ready to merge so I'll squash and merge it