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

Implement pagination in the chat #2196

Closed
elatif2020 opened this issue Oct 12, 2023 · 11 comments · Fixed by #2307
Closed

Implement pagination in the chat #2196

elatif2020 opened this issue Oct 12, 2023 · 11 comments · Fixed by #2307
Assignees
Labels
chat Priority - A V [production] verified on production enviroment

Comments

@elatif2020
Copy link
Collaborator

Load first ~10 messages and render them (to show the first screen).
And after the screen loads request a batch of more 40 messages upward.
Request the next batch once the user starts to go up (above the first 10 messages), and if she reach the upmost loaded message show a loader (after 1 second delay) until it's rendered.
For now if a user clicks a link to message which is above the current messages loaded - I suggest to load the next 200 messages first, and if it's not there the next 800 messages (so each time *4 the previous batch)

Should be implemented with subscription (at least for the latest messages) and preferably in a way that uses Firebase's offline persistent capabilities (so use cached data if possible).

We need this both for discussions and for dm's.

@pvm-code
Copy link
Collaborator

@elatif2020 I think it's bad approach. Usually, we fetch number of messages depends on screen height, maybe 1.5 or 2 times more than screen height

@pvm-code
Copy link
Collaborator

Also telegram and other messages works the same

@pvm-code
Copy link
Collaborator

For scroll to message, I believe we need backend API for calculate this - how much pages we should fetch to this message

@elatif2020
Copy link
Collaborator Author

elatif2020 commented Oct 17, 2023

@elatif2020 I think it's bad approach. Usually, we fetch number of messages depends on screen height, maybe 1.5 or 2 times more than screen height

@MeyerPV
Not sure I got you all the way.
Maybe I will break what I wrote above into steps to make it more clear:

  1. Load first N1 messages and render them (to show the first screen)
  2. After the screen is rendered - Load a batch of more N2 messages upward.
  3. Once the user starts to scroll up (above the first N3 messages) request the next batch of N4 messages.
  4. If she reach the upmost rendered message show a loader (after 1 second delay) until it's rendered.
  • Up to here, if I understand correctly you are suggesting N1 to be 1.5-2X #messages in the screen. So maybe around 15-20 (or could be calculated dynamically?). Anyway I believe we might have to play a bit with those numbers to optimize the behavior.
  1. Scrolling to a message. So the Idea is load an exponentially growing quotas of messages according to the location the message.
  • Regarding calculating the scrolling. I'm not sure if it makes sense to do it in the BE, if I understand correctly we can do the same in the FE and the communication with BE will add more delay. Correct me if I am wrong.
    So anyway it could be good to figure out whether we can efficiently find the index location of a given message.

@pvm-code
Copy link
Collaborator

exponential growing can be reason of slow performance. I find a solution without touching BE and exponential grow

We can get message createdAt date and fetch by condition all messages which below to it

@elatif2020
Copy link
Collaborator Author

let's wait with this ticket

@elatif2020
Copy link
Collaborator Author

@MeyerPV Updating that we want to move forward with this ticket. Could be good if you can start soon, maybe tomorrow morning once you're done with current tickets?

@pvm-code
Copy link
Collaborator

pvm-code commented Nov 1, 2023

Sure

@elatif2020
Copy link
Collaborator Author

@MeyerPV Notice that we should implement it in a way that will support (afterward) preloading chat messages at the page level. See the discussion in common here

@pvm-code
Copy link
Collaborator

pvm-code commented Nov 2, 2023

@elatif2020 As I understand correctly to fetch messages at the middle of the list for looking at replies? Like a - I'm at page 1 but reply at page 10 and I show only page 10, instead of fetching 2,3,4 ...., correct?

@elatif2020
Copy link
Collaborator Author

@elatif2020 As I understand correctly to fetch messages at the middle of the list for looking at replies? Like a - I'm at page 1 but reply at page 10 and I show only page 10, instead of fetching 2,3,4 ...., correct?

yes if it possible as you wrote without BE

@pvm-code pvm-code linked a pull request Nov 12, 2023 that will close this issue
pvm-code added a commit that referenced this issue Nov 27, 2023
@NoamQA NoamQA added the V [production] verified on production enviroment label Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chat Priority - A V [production] verified on production enviroment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants