Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] TW-1399: counter update on scroll #1619

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Te-Z
Copy link
Contributor

@Te-Z Te-Z commented Mar 21, 2024

Issue: #1399

To do:

  • when read marker is set, the last item in the list (at the bottom) is not marked as read
  • when there's more than 30 events to read and the next page load, there's still an autoscroll to the bottom

@Te-Z Te-Z marked this pull request as ready for review March 21, 2024 19:13
@Te-Z Te-Z changed the title TW-1399: counter update on scroll [WIP] TW-1399: counter update on scroll Mar 21, 2024
Copy link

This PR has been deployed to https://linagora.github.io/twake-on-matrix/1619

@@ -139,8 +140,6 @@ class ChatController extends State<Chat>

Timer? _timestampTimer;

String _markerReadLocation = '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you delete it?
It's related to display unread message

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not really deleted it: this variable was only used in _findUnreadReceivedMessages which I had to slightly modify:

String? _findUnreadReceivedMessageId(String fullyRead) {
final unreadEvents = findUnreadReceivedMessages(fullyRead);
Logs().d(
"Chat::getFirstUnreadEvent(): Last unread event ${unreadEvents.last}",
);
return unreadEvents.isEmpty ? null : unreadEvents.last.eventId;
}
List<Event> findUnreadReceivedMessages(String fullyRead) {
final events = timeline!.events;
if (fullyRead != '' && fullyRead.isNotEmpty) {
final lastIndexReadEvent = events.indexWhere(
(event) => event.eventId == fullyRead,
);
if (lastIndexReadEvent > 0) {
final afterFullyRead = events.getRange(0, lastIndexReadEvent);
final unreadEvents = afterFullyRead
.where((event) => event.senderId != client.userID)
.toList();
if (unreadEvents.isEmpty) return <Event>[];
return unreadEvents;
}
} else {
return <Event>[];
}
return <Event>[];
}

@@ -83,119 +85,145 @@ class ChatEventList extends StatelessWidget {
child: SelectionTextContainer(
chatController: controller,
focusNode: controller.selectionFocusNode,
child: InViewNotifierListCustom(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you delete it?
It's related to diplay timestamp when you scrolling

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not, I just added a widget above:

child: ValueListenableBuilder(
valueListenable: controller.lastScrollDirection,
builder: (context, lastScrollDirection, _) =>
InViewNotifierListCustom(
isInViewPortCondition: controller.isInViewPortCondition,
listViewCustom: ListView.custom(
padding: EdgeInsets.only(
top: 16,

@Te-Z
Copy link
Contributor Author

Te-Z commented Mar 25, 2024

@hoangdat @nqhhdev Here's the video of the autoscroll which mark everything as read. FYI I did not click on the right button which jumps to the first message.

Capture.video.du.25-03-2024.10.38.18.webm

@Te-Z
Copy link
Contributor Author

Te-Z commented Mar 27, 2024

@hoangdat @nqhhdev Here's the video of the autoscroll which mark everything as read. FYI I did not click on the right button which jumps to the first message.

Capture.video.du.25-03-2024.10.38.18.webm

Issue created for this: #1634

? controller.timeline!.events[currentEventIndex + 1]
: null;

return VisibilityDetector(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO: No need, InViewNotifierListCustom is enough so you can detect a message when scrolled over

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem when using InViewNotifierListCustom for our purpose is that currently it checks the top of the screen to handle the display of the time stamp. But the notifications are updated when they are visible at the bottom of the screen.
I tried to expand the InViewNotifierListCustom but it impact negatively the time stamp appearance. That's why I had to use VisibilityDetector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants