message-feed: Fixed narrow banner on All messages screen

[SahilSingh177]

I have made changes to the reset_ui_state() function, which now checks if the message list is empty. Based on this condition, the banner will either be shown or hidden.

Additionally, I have also added a call to the reset_ui_state() function within the stream muting functionality, to update the home page whenever a user mutes or unmutes any stream.

Furthermore, I have updated the fetch_message.js file to display narrow banners (if no messages are present) whenever a user refreshes the page. Fixes #23701

Screenshots and screen captures:

Original Behavior

New Behavior after the changes

https://user-images.githubusercontent.com/96344003/214055288-c1b29167-1592-426d-9762-bca182e4c6a5.mov

Unlike older tables, home was not acting in the same way during rerendering. So, I made changes in reset_ui_state method, so that in case of no message narrow banner appears.

Self-review checklist

• [X] Self-reviewed the changes for clarity and maintainability (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

• [X] Explains differences from previous plans (e.g., issue description).
• [X] Highlights technical choices and bugs encountered.
• [X] Calls out remaining decisions and concerns.
• [X] Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

• [X] Each commit is a coherent idea.
• [X] Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

• [X] Visual appearance of the changes.
• [X] Responsiveness and internationalization.
• [X] Strings and tooltips.
• [X] End-to-end functionality of buttons, interactions and flows.
• [X] Corner cases, error conditions, and easily imagined bugs.

[SahilSingh177]

@alya please review this PR. Thanks!

[SahilSingh177]

@N-Shar-ma Please review the PR and suggest if any change is required.

[SahilSingh177]

@N-Shar-ma I made the changes as you asked.

[alya]

Hm, I I get to an empty All messages, and then unmute a stream, I'm seeing errors popping up:

[SahilSingh177]

Hm, I I get to an empty All messages, and then unmute a stream, I'm seeing errors popping up:

It is due to this line message_lists.current.selected_row().offset() !== null in stream_muting.js. I'll change it to message_lists.current.selected_row().offset() !== undefined to solve the error.

[SahilSingh177]

@alya I fixed the error.

[SahilSingh177]

Hey @alya, I am following up on this pull request and would appreciate if you could review it as it has been pending for some time now.

[alya]

This worked for me when I first deleted the last message from All messages, but I'm not seeing the message anymore after I refresh the page.

Messages also fail to appear in the All messages view once I unmute a muted stream.

Please update the PR, and then test it carefully, trying out a variety of scenarios. Thanks!

[SahilSingh177]

@alya, I fixed the bug that you mentioned. I performed manual testing by refreshing the page after clearing all messages and the results were satisfactory. Additionally, when a muted stream is unmuted, messages display in the All messages view. I have updated the pull request description and included a video demonstration as reference.

[alya]

@laurynmm Would you be up for reviewing this one? I have not re-tested.

[SahilSingh177]

@SahilSingh177 - Thanks for working on this issue!

I made a couple of comments on your current changes, but also noticed that there's still a bug when the user is in the all messages few for all public streams (there will be messages here that are not normally in the user's message history). This is the /#narrow/streams/public view.

With your updates and with a user that has all the topics in their message history muted / no private messages, if you go to the first message in that narrowed all public messages view and mute that first message's topic, then the empty narrow message will appear at the top of the screen even though there are still messages in the view. So looking into how the mute topic works for that narrow view and how it's interacting with these changes is the next part to figure out.

Also, just noting here that you should review the guidelines for writing commit messages. Both your summary and message body currently need to be updated.

Thanks again and let me know if you have any questions!

Through examination, I discovered that the origin of the bug was due to the UI not updating upon any changes in topics. To resolve the issue, I added the reset_ui_state() function to the handle_topic_updates() in the muted_topic_ui.js file, which effectively resolved the issue, as demonstrated in the attached video.

Previous behavior

https://user-images.githubusercontent.com/96344003/215591268-3f54047d-85eb-40f9-b977-ff067ab8b329.mov

New behavior

https://user-images.githubusercontent.com/96344003/215591223-7ff3cab2-85e4-4052-8fdb-d610ca9591f2.mov

[SahilSingh177]

@laurynmm I have updated the PR with the changes you suggested. Please review this and suggest if any change is required.

[SahilSingh177]

Hey @laurynmm, I am following up on this pull request and would appreciate if you could review it as it has been pending for some time now.

[laurynmm]

@SahilSingh177 - Thanks for the ping! This PR got lost in my tracking as I was away for a bit the week before last.

Looking this over, I think that using narrow.reset_ui_state like this isn't going to work since it does a few other things besides check/set the empty narrow banner.

For example, if you have the same user logged in to two browsers (or one browser in a regular and incognito session) and have one browser unread the messages in a topic and the other browser then mark that topic or stream as muted, then we start seeing some unexpected behaviours with the mark as unread banner disappearing and reappearing.

Another example, is having a search for a word open in one browser scrolled to the top so you see the banner noting that the results are from your history. Muting or unmuting a topic from that view will remove that banner notification from the top of the feed.

Here's a quick screencast of that second example with the changes from this PR...

Muting and search view example

Screencast from 14-02-23 17:02:46.webm

So, we need to be able to update the empty narrow banner in these cases, but not these other parts of the UI, and still address the updating of the All messages view.

[SahilSingh177]

@SahilSingh177 - Thanks for the ping! This PR got lost in my tracking as I was away for a bit the week before last.

Looking this over, I think that using narrow.reset_ui_state like this isn't going to work since it does a few other things besides check/set the empty narrow banner.

For example, if you have the same user logged in to two browsers (or one browser in a regular and incognito session) and have one browser unread the messages in a topic and the other browser then mark that topic or stream as muted, then we start seeing some unexpected behaviours with the mark as unread banner disappearing and reappearing.

Another example, is having a search for a word open in one browser scrolled to the top so you see the banner noting that the results are from your history. Muting or unmuting a topic from that view will remove that banner notification from the top of the feed.

Here's a quick screencast of that second example with the changes from this PR...

Muting and search view example So, we need to be able to update the empty narrow banner in these cases, but not these other parts of the UI, and still address the updating of the All messages view.

I agree, using narrow.reset_ui_state will not work as it is calling some functions that are causing bugs.

A possible way to fix this can be to create another function that only checks whether the narrow banner should be activated or not i.e, we should move the following code to a separate function:

    if (message_lists.current.empty()) {
        narrow_banner.show_empty_narrow_message();
    } else {
        narrow_banner.hide_empty_narrow_message();
    }

We can call this function instead of calling reset_ui_state() while muting streams/topics. I tested the code for different scenarios and it seems to work perfectly.

Previous bug:

https://user-images.githubusercontent.com/96344003/219114077-8e52872b-bcfe-40d4-83b3-ef0fa28aeeb8.mov

Fix:

https://user-images.githubusercontent.com/96344003/219114135-0cbb5ad8-79dd-4a2c-94d5-f2719b92a9c7.mov

[alya]

@laurynmm would be great to get another round of review from you once @SahilSingh177 has had a chance to resolve the conflicts.

[SahilSingh177]

@laurynmm, I have updated the PR with the fix discussed above here. Please review and let me know if you require any further changes. Thank you!

[SahilSingh177]

Hey @laurynmm, I am following up on this pull request and would appreciate if you could review it as it has been pending for some time now.

[SahilSingh177]

@laurynmm, I have updated the PR with the following solution: Added a function handle_empty_narrow_message_home() inside narrow.js that checks for messages in the 'All messages' screen and displays the narrow banner appropriately.

I have tested the code for the following scenarios:

• Muting/unmuting a stream while inside it works as intended.
• Deleting the only message in "All messages" displays the narrow banner.
• Muting the only topic in "All messages" displays the narrow banner.
• Muting one stream does not affect others while inside it.
• The narrow banner only appears after the message has been fetched.

I have updated the PR description and added videos to demonstrate the changes.

[SahilSingh177]

Please review and let me know if any further changes are required. Thank you!

[SahilSingh177]

Bumping this up for another round of review. FYI @laurynmm.

[zulipbot]

Heads up @SahilSingh177, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.