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

perf: only fetch limited messages (pagination) #745

Merged
merged 55 commits into from
Mar 26, 2024
Merged

Conversation

nikkothari22
Copy link
Member

@nikkothari22 nikkothari22 commented Mar 1, 2024

api: previous and new message fetching API
perf: send message reactions in websocket
perf: send deleted message in websocket
perf: send edited message in websocket
perf: send saved message event in websocket
perf: send new messages in websocket (closes #462)
fix: User avatar is showing skeleton on scroll (closes #625)
feat: highlight replied message after scroll
perf: render action palette with hover debounce

Changes only made on the web app so far. Mobile app and desk integration needs to be changed.

Pending items:

  • Scroll to message (by search, saved messages or reply) which has not been loaded in the initial page
  • API to fetch newer messages needs to be tested
  • Track channel visit if latest messages are fetched
  • Track channel visit if the user is focused and new message comes in via websockets
  • Do not show unread count if user is on the channel
  • Date separators needs to be added back. Format dates in it to avoid expensive renders
  • Remove Virtuoso library and it's files (closes Remove Virtuoso from Raven Web app #446 )
  • Show button when new messages are available
  • Add skeleton loader for messages (closes [RAV-3] Add skeleton loader for messages on web app #567)

Will keep adding to this thread as things progress.

api: previous and new message fetching API
perf: send message reactions in websocket
perf: send deleted message in websocket
perf: send edited message in websocket
perf: send saved message event in websocket
perf: send new messages in websocket
fix: User avatar is showing skeleton on scroll #625
feat: highlight replied message after scroll
perf: render action palette with hover debounce
@github-actions github-actions bot added the 🐞 bug Something isn't working label Mar 1, 2024
@nikkothari22 nikkothari22 removed 🐞 bug Something isn't working UI labels Mar 1, 2024
@github-actions github-actions bot added UI 🐞 bug Something isn't working labels Mar 1, 2024
@nikkothari22
Copy link
Member Author

image

Skeleton loader for web app messages

@nikkothari22
Copy link
Member Author

nikkothari22 commented Mar 1, 2024

Even better solution: No fetches, just rely on websockets and SWR for everything (yeah).

Instead of maintaining two arrays for messages and previous messages, we can maintain a single array
This is maintained by the useSWR hook (useFrappeGetCall).

We use the mutate method to update the messages array when messages are received, updated, deleted, etc.
Even if the user scrolls up and loads older messages, the hook is mutated by concatenating the older messages to the existing messages array

Since we are using Web Socket events, this saves multiple round-trips to the server to fetch messages (pre Raven v1.5)

When the page changes and the user comes back to the chat, the messages are fetched again and hence the messages array is updated with only new messages

This also helps when the user goes directly to a specific message from somewhere(notification, search etc.).
We just shift the base message and fire the get_messages API to get the messages around the base message.
Post that, the behaviour is the same - if the user scrolls up or down, we just mutate the messages array directly.
If a new messages comes in while the user is viewing the base message (i.e. the latest messages are not visible), we do not update the messages array.
This is tracked using the has_new_messages flag in the response from the get_messages API

Refer: https://swr.vercel.app/docs/mutation

@nikkothari22
Copy link
Member Author

nikkothari22 commented Mar 22, 2024

  • Issue with file uploads here: two websocket events are published - one without a file, the other with.

@nikkothari22
Copy link
Member Author

nikkothari22 commented Mar 22, 2024

Unread message counts:

For every channel member, we store a last_visit timestamp in the Raven Channel Member doctype. To get the number of unread messages, we can simply look at the no. of messages created after this timestamp for any given channel.

The last_visit for a member of a channel is updated when:

  1. Latest messages are fetched (usually when a channel is opened)- this is in the get_messages API under chat_stream
  2. The user fetches newer messages (let's say they were viewing a saved message (older) and scrolled down until they reached the bottom of the chat stream). This is in the get_newer_messages API under chat_stream.
  3. When the user sends a new message (handled by controller under Raven Message)
  4. When the user exits a channel after all the latest messages had been loaded

When Raven loads, we fetch the unread message counts for all channels. Post that, updates to these counts are made when:

  1. If a user opens a channel - we locally update the unread message count to 0 - no API call
  2. If a realtime event is published for unread message count change and the sender is not the user itself - we only fetch the unread count for the particular channel (instead of all channels like we used to).

The realtime event for unread message count changed is published when:

  1. A new message is sent
  2. A message is deleted
  3. The user scrolls to the bottom of the chat stream from a base message.

Further optimisations that could be done: do not fetch unread count for a channel - just increment the count by 1

@nikkothari22 nikkothari22 marked this pull request as ready for review March 26, 2024 16:41
@nikkothari22
Copy link
Member Author

nikkothari22 commented Mar 26, 2024

Closes #771
Closes #778

@nikkothari22 nikkothari22 merged commit ffaa231 into develop Mar 26, 2024
4 checks passed
@nikkothari22 nikkothari22 deleted the paginate-messages branch March 26, 2024 16:45
@nikkothari22 nikkothari22 mentioned this pull request Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📦 dependencies Pull requests that update a dependency file desk enhancement New feature or request 🏃🏻 performance 🎨 UI 💻 webapp
Projects
None yet
2 participants