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

setup pagination for chat room #985

Merged
merged 12 commits into from
Jul 28, 2023
Merged

Conversation

Palakkgoyal
Copy link
Contributor

@Palakkgoyal Palakkgoyal commented Jul 26, 2023

This PR closes #936
I have added pagination on the chat room and also made it scroll from bottom to top.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you Palakkgoyal! for creating this pull request and contributing to Dummygram! 💗

The maintainers will review this Pull Request and provide feedback as soon as possible! 😇
We appreciate your patience and contribution, Keep up the great work! 😀

Copy link
Owner

@narayan954 narayan954 left a comment

Choose a reason for hiding this comment

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

could this usage be due to this?? If so this could be alarming actually.
image

Also, there're many snapshots in our project which are not unsubscribed/closed. We need to lookout again for the db usage. I think before this chat feature, usage was fine...If we allow this pagination, it'd cross the limit very early tomorrow as there'd be more users.
image

As now I am thinking, maybe this usage is partly because the snapshot is not closed and chat gets updated in background of users,once he has entered the chat section.It's like opening a portal lol on opening chat page. and partly because the pagination loads the chat very aggressively.

We need to add unsubscribe function in the return() of these component lifecycle.
I was just going to read this article regarding firebase today only, maybe you can give it a read as well :)

@narayan954
Copy link
Owner

I think adding unsubscribe method should work, and this pagination is fine as well. If we get into a problem with this though, we can add pagination to be not on scroll but a button load more

@Palakkgoyal
Copy link
Contributor Author

I think adding unsubscribe method should work, and this pagination is fine as well. If we get into a problem with this though, we can add pagination to be not on scroll but a button load more

I knew that providing a cleanup function is important but due to lack of experience I was not able to grasp its importance. After watching this and reading more about it I understand why it is important. I will do the required changes in it. Should I add a button now for loading chats or not?

@narayan954
Copy link
Owner

I think adding unsubscribe method should work, and this pagination is fine as well. If we get into a problem with this though, we can add pagination to be not on scroll but a button load more

I knew that providing a cleanup function is important but due to lack of experience I was not able to grasp its importance. After watching this and reading more about it I understand why it is important. I will do the required changes in it. Should I add a button now for loading chats or not?

it's your call, if it doesn't seem like it, let it be,else you definitely can! :)

@Palakkgoyal
Copy link
Contributor Author

I have done the required changes. Can you please check.

@narayan954
Copy link
Owner

I have done the required changes. Can you please check.

aight, on it!

Copy link
Owner

@narayan954 narayan954 left a comment

Choose a reason for hiding this comment

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

looks good to me!

@narayan954 narayan954 merged commit 230d02a into narayan954:master Jul 28, 2023
2 of 4 checks passed
@narayan954
Copy link
Owner

Wait, does it work at all @Palakkgoyal ;-; I missed to check and now on production, I don't think it works ;-;

@Palakkgoyal
Copy link
Contributor Author

I checked it worked but I don't know about the production.

@narayan954
Copy link
Owner

narayan954 commented Jul 29, 2023

I checked it worked but I don't know about the production.

yes it works there, but doesn't at all in production version and there's memory leaking

@Palakkgoyal
Copy link
Contributor Author

I tried to give a good cleanup to it using AbortController but don't know why it was not working.

@narayan954
Copy link
Owner

narayan954 commented Jul 29, 2023

I tried to give a good cleanup to it using AbortController but don't know why it was not working.

now I think abortcontroller is what's causing it to abort, hence not working

@Palakkgoyal
Copy link
Contributor Author

But I didn't used it cause AbortController wasn't even working during development.

@narayan954
Copy link
Owner

But I didn't used it cause AbortController wasn't even working during development.

lol, btw in mobile devices, the feature work, but not in desktops... I'll go through the code again and try to fix as well

@Palakkgoyal
Copy link
Contributor Author

Palakkgoyal commented Jul 30, 2023

But I didn't used it cause AbortController wasn't even working during development.

lol, btw in mobile devices, the feature work, but not in desktops... I'll go through the code again and try to fix as well

Is it? That seems typical.

@narayan954
Copy link
Owner

But I didn't used it cause AbortController wasn't even working during development.

lol, btw in mobile devices, the feature work, but not in desktops... I'll go through the code again and try to fix as well

Is it? That seems typical.

I feel the same, but it worked ;-;

@Palakkgoyal
Copy link
Contributor Author

That's amazing.

@Palakkgoyal Palakkgoyal deleted the pagination branch July 31, 2023 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[FEATURE] Set up pagination for chat
2 participants