Feature: friends popup #337
Conversation
- FriendsButton as a PlatformIconButton navigate to FriendsScreen
handled socket topics: - following_onlines - following_enters - following_leaves
| ); | ||
| } | ||
|
|
||
| @riverpod |
There was a problem hiding this comment.
we should cache this with @Riverpod(keepAlive: true)
There was a problem hiding this comment.
with @Riverpod(keepAlive: true) I have this behavior, steps to reproduce:
- go to following screen
- unfollow user
- go back to friends screen
- go to following screen
it serves old response, not updated with unfollow request at step 2
because cached with @Riverpod(keepAlive: true)
There was a problem hiding this comment.
yes I see... the unfollow action should also invalidate the cache with ref.invalidate
There was a problem hiding this comment.
Now I've added @Riverpod(keepAlive: true) and ref.invalidate :)
List<dynamic> => IList<LightUser>
|
@papo1011 I'm going to review the PR. Could you fix the conflict please? |
| ); | ||
| } | ||
|
|
||
| void _invalidateFollowingCache() { |
There was a problem hiding this comment.
looks like a bit complex logic here: we should probably not cache the data at all to avoid such code
There was a problem hiding this comment.
the added benefit of caching is not worth the complexity imo
| }); | ||
| } else { | ||
| ref | ||
| .read( |
There was a problem hiding this comment.
Same as other comment, the whole caching logic seems too complex here. Normally just invalidating the followingProvider should be enough to trigger a screen refresh.
Perhaps you could take the account preferences notifier as an example:
It works the same way: the initial data is fetched async from the server, and methods that update the state trigger an invalidateSelf to force fetching new data:
There was a problem hiding this comment.
Yes, I understand.
But if we add await Future.delayed(const Duration(seconds: 1)); before line 89 in the account_preferences, simulating a slow POST request, the delay will affect the toggle component, causing it to wait for 1 second before switching. The same behavior would occur when removing a user tile on the following screen.
There was a problem hiding this comment.
Yes it's not perfect, I agree, but in the vast majority of cases it will work smoothly.
UX can be improved I guess: set an opacity, or any other way to indicate something is loading. It will have the big advantage to be simple, compared to complex caching.
(I should do the same for the switches)
…11/mobile into papo1011-feature/friends-popup-242 Conflicts: lib/src/navigation.dart lib/src/view/home/home_tab_screen.dart pubspec.yaml
|
Too bad the diff includes already merged commits... dunno how to fix that. |
Here main...papo1011:mobile:feature/friends-popup-242 the three dots diff shows the difference between the two branches, displaying the correct commits. |
Conflicts:
lib/src/view/home/home_tab_screen.dart
Friends popup #242