Mobile 14

[gry-mar]

#26

Implemented Research Group Tab according to UI v1.0.0 with redirection to Study Circles detail view.

[simon-the-shark]

Nice implementation. I've left few notes and proposed bunch of changes. I guess some may be questionable, so if you disagree we can always discuss. Hopefully, I'm not forcing my coding style here too much on all of you guys.
🦈

[gry-mar]

I applied changes according to your CR and also changed the search query in scientific_circles_tab_repository. But I used force unwrapping which is not elegant but I didn't have an idea how to implement it differently in that case

@riverpod
Future<Iterable<ScientificCircle?>?> _sciCirclesFilteredByTextQuery(
    _SciCirclesFilteredByTextQueryRef ref) async {
  final originalList =
      await ref.watch(scientificCirclesRepositoryProvider.future);
  final query = ref.watch(searchScientificCirclesControllerProvider);
  return originalList?.where((element) =>
      element == null ||
      element.name.toLowerCase().contains(query.toLowerCase()) ||
      element.department == null ||
      element.department!.name.toLowerCase().contains(query.toLowerCase()));
}

Also I noticed that now there's a bug that if you open the tab there is "Wszystkie" selected but there aren't any circles shown. If you click on other tag and come back to "Wszystkie" they are all present. Like the view is not waiting for data while it renders. I have this issue after applying the changes and for now I don't know where is the problem as the ScientificCirclesDataView should render only if the circles list is present.

[simon-the-shark]

It's ok to use force unwrapping in this case, cause you manually check it before and there's no option this will cause an error:

element.department == null ||
      element.department!.name.toLowerCase().contains(query.toLowerCase())

if element.department is null, then the rest of the expression won't be evaluated, cause || works lazily.

However, if you want, then you can do it this way too:

element.department?.name.toLowerCase().contains(query.toLowerCase()) !=
          false

OR

element.department?.name.toLowerCase().contains(query.toLowerCase()) !=
          true

the behaviour will be different, you should use either use != true or != false, depending if you want treat null as false or as true

But as I said, force checks are ok if we manually check if the value is null or not with if, || or &&, so do what you want here

[simon-the-shark]

So here's a fix for the bug you've mentioned. Also few really simple notes. Good work here!

[simon-the-shark]

After above changes you can merge