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 #2307

Merged
merged 18 commits into from
Nov 27, 2023
Merged

Implement pagination in the chat #2196 #2307

merged 18 commits into from
Nov 27, 2023

Conversation

pvm-code
Copy link
Collaborator

#2196
#2196

What was changed?

  • Changed fetching logic to pagination
  • Removed parsedText async hook inside Chat message - it's slow down list a lot
  • Refactor useDiscussionMessagesById hook

Copy link

netlify bot commented Nov 12, 2023

Deploy Preview for preview-common ready!

Name Link
🔨 Latest commit a06c129
🔍 Latest deploy log https://app.netlify.com/sites/preview-common/deploys/656461b54fee5d00080a7bd0
😎 Deploy Preview https://deploy-preview-2307--preview-common.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pvm-code pvm-code linked an issue Nov 12, 2023 that may be closed by this pull request
@elatif2020
Copy link
Collaborator

@MeyerPV see my screen recording

  • there is a jump once I scroll up

  • for other items the chat messages doesn't appear at all

Common.Mozilla.Firefox.2023-11-13.13-13-49.mp4

src/services/DiscussionMessage.ts Outdated Show resolved Hide resolved
src/services/DiscussionMessage.ts Outdated Show resolved Hide resolved
src/shared/components/Chat/ChatMessage/ChatMessage.tsx Outdated Show resolved Hide resolved
src/shared/components/Chat/ChatMessage/ChatMessage.tsx Outdated Show resolved Hide resolved
src/shared/hooks/useCases/useDiscussionMessagesById.ts Outdated Show resolved Hide resolved
src/shared/hooks/useCases/useDiscussionMessagesById.ts Outdated Show resolved Hide resolved
@elatif2020
Copy link
Collaborator

@MeyerPV
The system is crashing for me with console errors referring to useDiscussionById.ts:74
for example when I visit this common
image

@pvm-code pvm-code requested a review from budnik9 November 22, 2023 09:22
@elatif2020
Copy link
Collaborator

@MeyerPV It is working for me 🙌 ⭐

@elatif2020
Copy link
Collaborator

@MeyerPV
Two comments (not sure it should be implemented with this ticket but please have a look):

  1. The "There are no messages here yet..." place holder appears while loading also when there are messages. We should not show it unless we know that there no messages, and show the spinner instead, starting only after 2 sec delay (we already made a fix for this before).
  2. If I go back to a discussion in the inbox it is fast. But when I switch to the same discussion in the space it is loading it again. Maybe we can make it also fast\ use the same storage somehow?

@pvm-code
Copy link
Collaborator Author

@MeyerPV Two comments (not sure it should be implemented with this ticket but please have a look):

  1. The "There are no messages here yet..." place holder appears while loading also when there are messages. We should not show it unless we know that there no messages, and show the spinner instead, starting only after 2 sec delay (we already made a fix for this before).
  2. If I go back to a discussion in the inbox it is fast. But when I switch to the same discussion in the space it is loading it again. Maybe we can make it also fast\ use the same storage somehow?

Fixed There are no messages

@elatif2020
Copy link
Collaborator

elatif2020 commented Nov 23, 2023

@MeyerPV
I have this issue with the chat jumping to the bottom once reaching the top of a chat page

It should be fixed now

@pvm-code
Copy link
Collaborator Author

@MeyerPV I have this issue with the chat jumping to the bottom once reaching the top of a chat page

It should be fixed now

It should be fixed now

src/services/DiscussionMessage.ts Outdated Show resolved Hide resolved
src/services/DiscussionMessage.ts Outdated Show resolved Hide resolved
src/services/DiscussionMessage.ts Show resolved Hide resolved
src/services/DiscussionMessage.ts Show resolved Hide resolved
src/shared/hooks/useCases/useDiscussionMessagesById.ts Outdated Show resolved Hide resolved
@pvm-code pvm-code merged commit 27a892b into dev Nov 27, 2023
5 checks passed
@roienatan roienatan deleted the CW-2196-pagination branch November 27, 2023 12:46
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.

Implement pagination in the chat
3 participants