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

Mark as read for selected message when navigating/narrowing to a new view is inconsistent #26021

Open
laurynmm opened this issue Jun 14, 2023 · 2 comments · May be fixed by #26023
Open

Mark as read for selected message when navigating/narrowing to a new view is inconsistent #26021

laurynmm opened this issue Jun 14, 2023 · 2 comments · May be fixed by #26023
Labels
area: message feed (UI) Buttons/UI directly in the message feed (not popovers, etc.)

Comments

@laurynmm
Copy link
Collaborator

As discussed in these CZO threads:

When we fixed a bug in commit e7b97e25c96cec5de4186470cace78f12b3d83ec, the behavior for marking the selected message in a new view became inconsistent, possibly due to races between scroll events and fetching messages.

The expected behavior when navigating to a new view that allows for marking messages as read (based on the type of narrow/view and the user's mark as read settings) is that the newly selected message in that view (which is normally the first unread message) should be marked as read.

@laurynmm laurynmm added area: message feed (uncategorized) area: message feed (UI) Buttons/UI directly in the message feed (not popovers, etc.) labels Jun 14, 2023
@laurynmm laurynmm self-assigned this Jun 14, 2023
laurynmm added a commit to laurynmm/zulip that referenced this issue Jun 14, 2023
Previously, the event handler for marking selected messages as read
was only triggered if there was a previously selected message ID.
This meant that user navigation (either via hotkeys or mouse clicks)
would not necessarily mark the initially selected first unread
message in the view, even if the view/user settings allowed for
the message to be marked read.

Occasionally, a document on scroll event would be called that would
create a race where the message would get marked as read in cases
where message fetch requests were received more slowly. This made
the overall mark as read experience inconsistent.

We update the event handler to allow for marking the selected
message as read when there was no previously selected message in
the message list.

There is an initial loading of messages to the home message list
where we set `mark_read: false` since we can't safely assume on
a page load/reload that the home view is the user's current or
default view. Once the home view message list is loaded, we add
a check for the current view and mark the selected message as
read if the home view is indeed the current view.

Fixes zulip#26021.
laurynmm added a commit to laurynmm/zulip that referenced this issue Jun 28, 2023
Previously, the event handler for marking selected messages as read
was only triggered if there was a previously selected message ID.
This meant that user navigation (either via hotkeys or mouse clicks)
would not necessarily mark the initially selected first unread
message in the view, even if the view/user settings allowed for
the message to be marked read.

Occasionally, a document on scroll event would be called that would
create a race where the message would get marked as read in cases
where message fetch requests were received more slowly. This made
the overall mark as read experience inconsistent.

We update the event handler to allow for marking the selected
message as read when there was no previously selected message in
the message list.

There is an initial loading of messages to the home message list
where we set `mark_read: false` since we can't safely assume on
a page load/reload that the home view is the user's current or
default view. Once the home view message list is loaded, we add
a check for the current view and mark the selected message as
read if the home view is indeed the current view.

Fixes zulip#26021.
laurynmm added a commit to laurynmm/zulip that referenced this issue Jul 1, 2023
Previously, the event handler for marking selected messages as read
was only triggered if there was a previously selected message ID.
This meant that user navigation (either via hotkeys or mouse clicks)
would not necessarily mark the initially selected first unread
message in the view, even if the view/user settings allowed for
the message to be marked read.

Occasionally, a document on scroll event would be called that would
create a race where the message would get marked as read in cases
where message fetch requests were received more slowly. This made
the overall mark as read experience inconsistent.

We update the event handler to allow for marking the selected
message as read when there was no previously selected message in
the message list.

There is an initial loading of messages to the home message list
where we set `mark_read: false` since we can't safely assume on
a page load/reload that the home view is the user's current or
default view. Once the home view message list is loaded, we add
a check for the current view and mark the selected message as
read if the home view is indeed the current view.

Fixes zulip#26021.
laurynmm added a commit to laurynmm/zulip that referenced this issue Jul 4, 2023
Previously, the event handler for marking selected messages as read
was only triggered if there was a previously selected message ID.
This meant that user navigation (either via hotkeys or mouse clicks)
would not necessarily mark the initially selected first unread
message in the view, even if the view/user settings allowed for
the message to be marked read.

Occasionally, a document on scroll event would be called that would
create a race where the message would get marked as read in cases
where message fetch requests were received more slowly. This made
the overall mark as read experience inconsistent.

We update the event handler to allow for marking the selected
message as read when there was no previously selected message in
the message list.

There is an initial loading of messages to the home message list
where we set `mark_read: false` since we can't safely assume on
a page load/reload that the home view is the user's current or
default view. Once the home view message list is loaded, we add
a check for the current view and mark the selected message as
read if the home view is indeed the current view.

Fixes zulip#26021.
@zulipbot
Copy link
Member

zulipbot commented Aug 5, 2023

@laurynmm You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!

@zulipbot
Copy link
Member

zulipbot commented Aug 29, 2023

@laurynmm You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!

laurynmm added a commit to laurynmm/zulip that referenced this issue Oct 5, 2023
Previously, the event handler for marking selected messages as read
was only triggered if there was a previously selected message ID.
This meant that user navigation (either via hotkeys or mouse clicks)
would not necessarily mark the initially selected first unread
message in the view, even if the view/user settings allowed for
the message to be marked read.

Occasionally, a document on scroll event would be called that would
create a race where the message would get marked as read in cases
where message fetch requests were received more slowly. This made
the overall mark as read experience inconsistent.

We update the event handler to allow for marking the selected
message as read when there was no previously selected message in
the message list.

There is an initial loading of messages to the home message list
where we set `mark_read: false` since we can't safely assume on
a page load/reload that the home view is the user's current or
default view. Once the home view message list is loaded, we add
a check for the current view and mark the selected message as
read if the home view is indeed the current view.

Fixes zulip#26021.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: message feed (UI) Buttons/UI directly in the message feed (not popovers, etc.)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants