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.

Sign up for GitHub

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

Jump to bottom

Make timeline pagination more flexible and smarter #1327

Merged
merged 5 commits into from
Jan 10, 2023
Merged

Make timeline pagination more flexible and smarter #1327

merged 5 commits into from
Jan 10, 2023

Conversation

jplatte
Copy link
Collaborator

@jplatte jplatte commented Jan 6, 2023

  • Add TimelineStart virtual timeline item
  • Don't actually fire off a request when the top of the timeline has already been reached
  • Allow making multiple requests without removing the loading indicator in between

@jplatte jplatte requested a review from a team January 6, 2023 12:54
@jplatte jplatte mentioned this pull request Jan 6, 2023
Closed
29 tasks
jplatte
jplatte commented Jan 6, 2023
View reviewed changes
Copy link
Collaborator Author

@jplatte jplatte left a comment

Choose a reason for hiding this comment

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

Not at my computer anymore, but just realized a mistake.

crates/matrix-sdk/src/room/timeline/pagination.rs Show resolved Hide resolved
manuroe
manuroe reviewed Jan 9, 2023
View reviewed changes
bindings/matrix-sdk-ffi/src/timeline.rs
/// The beginning of the visible timeline.
///
/// There might be earlier events the user is not allowed to see due to
/// history visibility.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for exposing this case!

@jplatte jplatte requested a review from a team January 9, 2023 10:32
poljar
poljar approved these changes Jan 9, 2023
View reviewed changes
Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

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

Looks mostly sensible, if we can get rid of the expect that would be great.

crates/matrix-sdk/src/room/timeline/mod.rs Outdated Show resolved Hide resolved
@jplatte jplatte requested a review from poljar January 10, 2023 08:23
poljar
poljar approved these changes Jan 10, 2023
View reviewed changes
crates/matrix-sdk/src/room/timeline/mod.rs Outdated Show resolved Hide resolved
@jplatte jplatte force-pushed the jplatte/timeline-pagination-2 branch from bb8f50c to 051d9f5 Compare January 10, 2023 08:51
@jplatte jplatte enabled auto-merge (rebase) January 10, 2023 08:51
@jplatte jplatte merged commit 444a82f into main Jan 10, 2023
@jplatte jplatte deleted the jplatte/timeline-pagination-2 branch January 10, 2023 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manuroe manuroe manuroe left review comments

@poljar poljar poljar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jplatte @poljar @manuroe