The _.each calls with an inline function expression have already been
converted to for…of loops. We could do that here, but using .forEach
when we’re just reusing an existing function seems like a good
guideline.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Now that we have the type situation of having anchor support passing a
string, this is a much more natural way to implement
use_first_unread_anchor.
We still support the old interface to avoid breaking compatibility
with legacy versions of the mobile apps.
This makes the code more readable, by just passing the anchor through
without changing its field name back and forth.
There's no reason for this parameter to involve parsing and integer --
it should be a number in all incoming code paths.
The streams:all adveritsement notice in search should only appear
after we've already received the response from the server, to avoid a
mix of problems ranging from misplaced loading indicator to scrolling
issues to the notice just being distracting while you're waiting for
the server to return results.
We need to add a pre_scroll_cont parameter to the message_fetch API,
since adding this notice would otherwise potentially throw off the
scroll positioning logic for which message to select.
Fixes#13441.
This commit was originally automatically generated using `tools/lint
--only=eslint --fix`. It was then modified by tabbott to contain only
changes to a set of files that are unlikely to result in significant
merge conflicts with any open pull request, excluding about 20 files.
His plan is to merge the remaining changes with more precise care,
potentially involving merging parts of conflicting pull requests
before running the `eslint --fix` operation.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
With webpack, variables declared in each file are already file-local
(Global variables need to be explicitly exported), so these IIFEs are
no longer needed.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
The approach taken here is basically use user IDs in operator that
support it when sending the request for fetching the messages
(see comments in code for more details).
We now have two functions:
add_new_messages
add_old_messages
This is a lot easier on the eyes, and it will also
prevent us from exceeding line length in future commits.
We also remove an unneeded stub in the narrow_activate
tests.
Apparently, we didn't have one of these, and thus had a moderate
number of generally very old violations in the codebase. Fix this and
clear the ones that exist..
Like the other similar commits, we were doing the same work in all
code paths, just with a much more error-prone approach.
We can also now remove the now-unused finish_initial_narrow function.
Like the other commits in this series, we were already doing this in
all of the callers of load_messages; this centralizes that logic in a
less ad-hoc feeling way.
We no longer use or need the start_initial_narrow function.
Previously, each individual caller of load_messages that passed
num_before > 0 would do its own manual management of fetch_status;
now, we just do it inside load_messages.
Apparently, the older side of the FetchStatus object for home_msg_list
was incorrectly not being maintained. We got away with this, because
the do_backfill code path (which runs after we're done with the
load_more cycle) will correct the error for found_oldest. But we
didn't have proper handling for history_limited here.
When we're doing the load_more frontfill, we were not correctly
declaring that we were in the process of doing a fetch. Because the
next load_more call clears this state anyway, this was generally a
short race, off-screen, but it is still a data flow bug.
See the upcoming commits for a refactor that will eliminate the
possibility of this sort of bug.
When there is some error in connecting to server(more specifically to the
tornado server) the "Unable to connect to Zulip" connection error message
gets cleared as Django server could send the response of "get" request of
old messages and hence get_old_messages_success hides the error message
even though the connection is not properly established.
Fixes: #5599.
This commit prepares the frontend code to be consumed by webpack.
It is a hack: In theory, modules should be declaring and importing the
modules they depend on and the globals they expose directly.
However, that requires significant per-module work, which we don't
really want to block moving our toolchain to webpack on.
So we expose the modules by setting window.varName = varName; as
needed in the js files.
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.
Anders Kaseorg
Tim Abbott
Priyank Patel
Steve Howell
Vishnu Ks
Shubham Dhama
Cynthia Lin
Armaan Ahluwalia