Skip to content

Commit

Permalink
fix: feed scroll to top on scroll up (#601)
Browse files Browse the repository at this point in the history
## Description
This issue is related to blocking. When a feed cell is about to be
rendered, we watch the provider to check if the author is
blocked/blocking to determine if we want to show the cell. The default
value inside cell is `true`, so that when the provider is still loading,
we don’t show the cell. Later, after the provider is loaded and it
returns `false`, we know that the author is not blocked/blocking and we
show the cell.
Then, when user scrolls through the feed, some of the cells are getting
out of the viewport, they are getting disposed of and their block
checking provider is getting disposed, because nothing else is listening
to it.
Now, when user scrolls back, these cells try to render again. Provider
was disposed, so it is recreated and starts loading. When it’s loading,
we use the default `true` value. This causes the cell to just return an
empty SizedBox, so the feed listview sees it has enough space to render
the next cell. Then the next cell is not rendering because it’s waiting
for it’s provider to load so the listview tries to render the next one.
And the cycle continues for all cells up to the top. By the time they
are loaded correctly, listview assumes it reached the top of the scroll
view, because everything is rendered so it updates it’s scroll metrics.
Now after a split second when all these cells are properly rendered
because the providers are properly loaded , user stays at the top of the
scroll view and all the content is pushed down, giving an impression
that the feed was scrolled to the top.

The problem here is that the data about wether or not the cell is
blocked is disposed each time it gets out of the viewport. We should
keep it in memory. There are in my eyes two ways to do that:

1. Make the provider responsible for blocked/blocking checking
`keepAlive`, but that could mean a lot of providers alive at the same
time, if for example user scrolls through 1k of cells
2. Create a Map<String, bool> with `useState` inside the `EntitiesList`
to keep a single map of the ids of entities and their blocked/blocking
status and pass it to every cell it builds. This way, when a certain
cell builds a second time, it uses the value from the map as an initial
value and then after it’s provider is loaded, updates it if needed


## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [x] Refactoring
- [ ] Documentation
- [ ] Chore

## Screenshots (if applicable)
<!-- Include screenshots to demonstrate any UI changes. -->
<!-- <img width="180" alt="image" src="image_url_here"> -->
  • Loading branch information
ice-ajax authored Jan 24, 2025
1 parent fdd5a22 commit 2d55b3e
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 23 deletions.
20 changes: 17 additions & 3 deletions lib/app/features/components/entities_list/entities_list.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: ice License 1.0

import 'package:flutter/material.dart';
import 'package:flutter_hooks/flutter_hooks.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:ion/app/extensions/async_value_listener.dart';
import 'package:ion/app/extensions/extensions.dart';
import 'package:ion/app/features/components/entities_list/components/article_list_item.dart';
import 'package:ion/app/features/components/entities_list/components/generic_repost_list_item.dart';
Expand All @@ -15,7 +17,7 @@ import 'package:ion/app/features/ion_connect/model/ion_connect_entity.dart';
import 'package:ion/app/features/user/providers/block_list_notifier.c.dart';
import 'package:ion/app/features/user/providers/user_metadata_provider.c.dart';

class EntitiesList extends StatelessWidget {
class EntitiesList extends HookWidget {
const EntitiesList({
required this.entities,
this.showParent = false,
Expand All @@ -31,6 +33,8 @@ class EntitiesList extends StatelessWidget {

@override
Widget build(BuildContext context) {
final blockedEntitiesIds = useState(<String, bool>{});

return SliverList.builder(
itemCount: entities.length,
itemBuilder: (BuildContext context, int index) {
Expand All @@ -39,6 +43,7 @@ class EntitiesList extends StatelessWidget {
showParent: showParent,
separatorHeight: separatorHeight,
hideBlocked: hideBlocked,
blockedIds: blockedEntitiesIds,
);
},
);
Expand All @@ -51,20 +56,29 @@ class _EntityListItem extends ConsumerWidget {
required this.separatorHeight,
required this.showParent,
required this.hideBlocked,
required this.blockedIds,
});

final IonConnectEntity entity;
final double? separatorHeight;
final bool showParent;
final bool hideBlocked;
final ValueNotifier<Map<String, bool>> blockedIds;

@override
Widget build(BuildContext context, WidgetRef ref) {
final userMetadata =
ref.watch(userMetadataProvider(entity.masterPubkey, network: false)).valueOrNull;

final isBlockedOrBlocking =
ref.watch(isEntityBlockedOrBlockingProvider(entity, cacheOnly: true)).valueOrNull ?? true;
ref.listenAsyncValue(
isEntityBlockedOrBlockingProvider(entity, cacheOnly: true),
onSuccess: (blocked) {
if (blocked != null && blockedIds.value[entity.id] != blocked) {
blockedIds.value = {...blockedIds.value, entity.id: blocked};
}
},
);
final isBlockedOrBlocking = blockedIds.value[entity.id] ?? true;

if (userMetadata == null || (isBlockedOrBlocking && hideBlocked)) {
/// When we fetch lists (e.g. feed, search or data for tabs in profiles),
Expand Down
6 changes: 4 additions & 2 deletions lib/app/features/settings/views/blocked_users_modal.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ class BlockedUsersModal extends HookConsumerWidget {

@override
Widget build(BuildContext context, WidgetRef ref) {
// We fetch the data once and keep it in state so that it doesn't case users to disappear after unblocking
final isLoading = useState(true);
// We fetch the data once and keep it in state so that it doesn't cause users to disappear after unblocking
final pubkeys = useState<List<String>?>(null);
useOnInit(() async {
final blockList = await ref.read(currentUserBlockListProvider.future);
pubkeys.value = blockList?.data.pubkeys;
isLoading.value = false;
});

return SheetContent(
Expand All @@ -31,7 +33,7 @@ class BlockedUsersModal extends HookConsumerWidget {
slivers: [
const BlockedUsersAppBar(),
const BlockedUsersSearchBar(),
if (pubkeys.value != null)
if (!isLoading.value)
SliverList.separated(
separatorBuilder: (_, __) => SizedBox(height: 16.0.s),
itemCount: pubkeys.value!.length,
Expand Down
32 changes: 14 additions & 18 deletions lib/app/features/user/providers/block_list_notifier.c.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,16 @@ Future<bool> isEntityBlockedOrBlocking(
IonConnectEntity entity, {
bool cacheOnly = false,
}) async {
final isMainAuthorBlockedOrBlocking = ref
.watch(isBlockedOrBlockingProvider(entity.masterPubkey, cacheOnly: cacheOnly))
.valueOrNull ??
true;
final isMainAuthorBlockedOrBlocking = await ref
.watch(isBlockedOrBlockingProvider(entity.masterPubkey, cacheOnly: cacheOnly).future);
if (isMainAuthorBlockedOrBlocking) return true;
return switch (entity) {
PostEntity() => isPostChildBlockedOrBlocking(ref, entity, cacheOnly: cacheOnly),
RepostEntity() => isRepostChildBlockedOrBlocking(ref, entity, cacheOnly: cacheOnly),
PostEntity() =>
ref.watch(isPostChildBlockedOrBlockingProvider(entity, cacheOnly: cacheOnly).future),
RepostEntity() =>
ref.watch(isRepostChildBlockedOrBlockingProvider(entity, cacheOnly: cacheOnly).future),
GenericRepostEntity() =>
isGenericRepostChildBlockedOrBlocking(ref, entity, cacheOnly: cacheOnly),
ref.watch(isGenericRepostChildBlockedOrBlockingProvider(entity, cacheOnly: cacheOnly).future),
_ => false,
};
}
Expand All @@ -101,11 +101,9 @@ Future<bool> isPostChildBlockedOrBlocking(
final quotedPost = await ref.watch(
ionConnectEntityProvider(eventReference: quotedPostReference).future,
);
if (quotedPost == null) return true;
return ref
.watch(isEntityBlockedOrBlockingProvider(quotedPost, cacheOnly: cacheOnly))
.valueOrNull ??
true;
if (quotedPost == null) return false;

return ref.watch(isEntityBlockedOrBlockingProvider(quotedPost, cacheOnly: cacheOnly).future);
}

@riverpod
Expand All @@ -117,9 +115,8 @@ Future<bool> isRepostChildBlockedOrBlocking(
final eventReference =
ImmutableEventReference(eventId: repost.data.eventId, pubkey: repost.data.pubkey);
final entity = await ref.watch(ionConnectEntityProvider(eventReference: eventReference).future);
if (entity == null) return true;
return ref.watch(isEntityBlockedOrBlockingProvider(entity, cacheOnly: cacheOnly)).valueOrNull ??
true;
if (entity == null) return false;
return ref.watch(isEntityBlockedOrBlockingProvider(entity, cacheOnly: cacheOnly).future);
}

@riverpod
Expand All @@ -131,9 +128,8 @@ Future<bool> isGenericRepostChildBlockedOrBlocking(
final eventReference =
ImmutableEventReference(eventId: repost.data.eventId, pubkey: repost.data.pubkey);
final entity = await ref.watch(ionConnectEntityProvider(eventReference: eventReference).future);
if (entity == null) return true;
return ref.watch(isEntityBlockedOrBlockingProvider(entity, cacheOnly: cacheOnly)).valueOrNull ??
true;
if (entity == null) return false;
return ref.watch(isEntityBlockedOrBlockingProvider(entity, cacheOnly: cacheOnly).future);
}

@riverpod
Expand Down

0 comments on commit 2d55b3e

Please sign in to comment.