If you visit a narrow that has unread messages on it that aren't part
of the home view (e.g. in a muted stream), then we were never calling
`message_util.do_unread_count_updates`, and more importantly,
`unread.process_loaded_messages` on those messages. As a result, they
would be unread, and moving the cursor over them would never mark
those messages as read (which was visible through the little green
marker never disappearing).
I can't tell whether this fixes#8042 and/or #8236; neither of them
exactly fits the description of this issue unless the PM threads in
question were muted or something, but this does feel related.
Apparently, we were incorrectly passing through something related to
opts.use_initial_narrow_pointer as the value for `use_first_anchor`.
If you read the logic in narrow.js carefully,
use_initial_narrow_pointer was unconditionally false.
The correct value for this attribute is when we're trying to narrow to
the first unread message in a given context. There are two things to
check:
* then_select_id is -1; i.e. we don't have a specific message ID we're
trying to narrow around.
* select_first_unread is True, i.e. we're trying to narrow to the
first unread message.
A bit more work should allow us to get rid of the second condition,
but I'm not quite confident enough to do that yet.
Note from tabbott: This is a somewhat surprising feature to be adding
this late in Zulip's development, but the model we've had of what
narrows are easy to access via clicking around has meant that it was
fairly difficult to get into a narrow that didn't include the very
latest messages in that narrow.
Fixes#3465.
This change mostly de-duplicates code, but it also prevents
some unnecessary backfills if you're in the home view and
happen to scroll back while the idle loop is right in the middle
of a backfill fetch, or if the idle loop has taken you back
to the oldest message already. This is a consequence of
do_backfill() calling msg_list.fetch_status functions.
Everything else should work pretty much the same, since
do_backfill() computes anchor essentially the same way we
originally doing it in the onIdle() function.
This slightly changes the order of operations of what we do in
the `cont` callback, so that we update fetch status earlier.
The order is somewhat arbitrary here, but we generally want to
update data structures first.
The "all" list and "home" list are basically kept in sync, and
the former is a superset of the latter.
Whenever we are backfilling, we want to backfill "all", so we use
it as the anchor, even though home_msg_list is the message list
we are actually rendering.
This has a small stylistic change. The load_more function
is recursive, and we now early-exit when the recursion is
finished (i.e. when we've found the newest row).
We now use the found_oldest flag from the server to determine
whether we should load older messages.
This requires us to change a few places in the code where we
pass the whole payload around now instead of just the messages.
Actually, many of the "cont" callbacks don't even look at the
data, so this change wasn't as invasive as I might have
predicted.
We now attach a fetch_status to message lists, so that they
can track their fetch status individually. When you go
back in a narrow and get all the older messages, we turn
off future fetches.
The narrow.js code no longer needs to orchestrate anything
here. The "home" message list won't have as many redundant
fetches after this commit, because we don't need to reset
flags every time we do `narrow.deactivate`.
And then actual narrows get a new message list every time
you narrow, so their fetch status gets reset implicitly
as part of constructing the MessageList object.
We are going to remove message_fetch.reset_for_narrow() soon,
but its callers probably still want to hide any scrolling
indicators, and we just let them do that directly.
This helps us consolidate the fetching constants without
having to export them. It will also remove some
responsibility for narrow.js to track fetching state.
This mostly moves code from ui.js.
We change the arguments to `message_fetch.load_more_messages()`
to be `opts` with callbacks for `show_loading` and `hide_loading`.
We also defer starting the scroll handler until `message_fetch.js`
has been initialized.
We want to call `set_message_booleans` as soon as we
get data from the server, to avoid confusion about whether
`flags` is the authoritative field.
This commit has callers to `add_message_metadata` call
`set_message_booleans`.
This also sets us up to **not** call `set_message_booleans`
in the local echo codepath, where we can just have the
markdown processor set booleans natively.
There's no advantage to doing a small batch size towards current here,
since latency isn't an issue at this point, and performance on the
server side generally favors larger batch sizes.
This also will make it significantly harder to start getting 429 rate
limiting errors when loading when far behind current.
Despite the length of this commit, it is a very straightforward
moving of code from narrow.js -> narrow_state.js, and then
everything else is just s/narrow.foo()/narrow_state.foo()/
(with a few tiny cleanups to remove some code duplication
in certain callers).
The only new functions are simple setter/getters that
encapsulate the current_filter variable:
narrow_state.reset_current_filter()
narrow_state.set_current_filter()
narrow_state.get_current_filter()
We removed narrow.predicate() as part of this, since it was dead
code.
Also, we removed the shim for narrow_state.set_compose_defaults(),
and since that was the last shim, we removed shim.js from the app.
Shubham Dhama
Tim Abbott
Steve Howell
derAnfaenger
Rishi Gupta
Cory Lynch
Rohitt Vashishtha
fionabunny
Steve Howell
Brock Whittaker