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

Just-opened narrow sometimes flickers "no messages" before fetch has started #5152

Open
chrisbobbe opened this issue Dec 6, 2021 · 5 comments · May be fixed by #5639
Open

Just-opened narrow sometimes flickers "no messages" before fetch has started #5152

chrisbobbe opened this issue Dec 6, 2021 · 5 comments · May be fixed by #5639
Assignees
@anshuman-8
Labels
a-data-sync Zulip's event system, event queues, staleness/liveness a-message list P1 high-priority

Comments

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Dec 6, 2021 •
edited
Loading 

const isFetching = fetching.older || fetching.newer || loading;
// [...]
const sayNoMessages = messages.length === 0 && !isFetching;

We fudge fetching.older and fetching.newer as false in fetchingReducer when this is a narrow we've just opened, one that state.fetching doesn't have info for yet.

fetchMessagesInNarrow correctly sets fetching.older and fetching.newer to true when it's called. But by the time it's called, it's too late: the false values have already made sayNoMessages true, wrongly, for at least a frame. And during that time, we've told the user there are no messages.

Right?
@gnprice gnprice added a-data-sync Zulip's event system, event queues, staleness/liveness P1 high-priority labels Dec 6, 2021
@chrisbobbe
Copy link
Contributor Author

We fudge fetching.older and fetching.newer as false in fetchingReducer when this is a narrow we've just opened; one that state.fetching doesn't have info for yet.

Should we be fudging it as true, or not fudging it at all?

@gnprice gnprice mentioned this issue Sep 15, 2022
Open
@chrisbobbe
Copy link
Contributor Author

chrisbobbe commented Jan 9, 2023 •
edited
Loading

A user report from @anshuman-8 with screenshots from #5636 (which I'm about to close as a duplicate; please let me know if you think this is wrong):

In every chat page, the "No messages" text displays for a second before showing a loading placeholder.

For a better experience, the chat page should start with a loading placeholder and then continue to show no messages or all chats.

"No messages" text before placeholder
No messages Loading placeholder

@chrisbobbe chrisbobbe mentioned this issue Jan 9, 2023
Closed
@anshuman-8
Copy link

Yes @chrisbobbe , this seems to be the same issue as #5636 , where sayNoMessage gets set as true before fetchMessagesInNarrow in chatScreen causing "No messages" to come up before loading placeholder.
May I work on this issue?

@chrisbobbe
Copy link
Contributor Author

Sure, thanks! I've just assigned it to you.

@anshuman-8 anshuman-8 linked a pull request Jan 13, 2023 that will close this issue
Open
@anshuman-8
Copy link

Hey @chrisbobbe ,

I have submitted a pull request to fix this problem.
Please review it and let me know if anything further is needed.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@anshuman-8 anshuman-8

Labels
a-data-sync Zulip's event system, event queues, staleness/liveness a-message list P1 high-priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: prevented 'no message' screen on first narrow load anshuman-8/zulip-mobile
3 participants
@gnprice @chrisbobbe @anshuman-8