We now try harder to find the first unread message in an
upcoming narrow, which has the user-visible effect that we
select the unread message before waiting for search results.
Before this change, we only applied this logic to searches
that were things like exactly stream/topic or exactly is-private.
Now we will also handle things like stream/topic/sender. For
the stream/topic piece we look up candidate unread ids using
the steam/topic buckets in unread.js, but then we still filter
those messages by stream/topic/sender as we look for the first
unread id.
I renamed get_unread_ids() to _possible_unread_message_ids().
The name is deliberately verbose, since we're about
to make it have kind of unusual semantics that only make sense
for its one caller.
The outside code will continue to call get_first_unread_info().
In the tests I wrap this function in a wrapper with the more
pleasant name of "candidate_ids", since in the test there's
less worry about unwittingly exposing a kind of janky function.
This is setting up for a subsequent commit to have a smaller
diff. The current ordering of the code blocks doesn't matter,
since only one of the conditions will be true, so this won't
change any behavior. (Later commits will make the order matter.)
We don't reference this anymore (it was only ever used by the Dropbox
integration, which was hardcoded-off for years before being removed in
e6833b6427)
We will use this to find the first id from a list of
message ids that matches a filter. (This will help us
during narrowing to determine whether we have at least
one good message locally, so that we can render something
useful before waiting for the server.)
This new API replaces some more specific functions that were
only recently introduced:
is_stream_only
is_stream_topic_only
is_pm_with_only
is_for_only
We use the deterministically sorted "term_type" values for
matching. (Notably "stream" will come before "topic".)
This uses scss mixins, this are functions you can pass parameter in it and
return css/scss. It made repeating vendored transistion, and user-select property
more easier to use with less repetation. This also includes a scss file called reuseable_components.scss
which can be used anywhere else.
The app icons (web/mobile/desktop) got all squished together when
they broke to a new line in smaller windows. They now have better
spacing and all break at the same time.
The integration widgets on the homepage had a weird-looking white
border. That border has now been updated to match the better-
looking border on the integrations page.
The "Short/Long Text" option for custom profile fields wasn't properly
capitalized (i.e. "Text" should have been all lowercase), and also
wasn't properly tagged for translation.
For the sake of consistency, the change to proper capitalization has
also been applied to the models and any tests involving this feature.
Due to a bug in Django, it complained about the models having changed
and thus not being consistent with the migrations. That isn't actually
true (since the database stores the numeric values for each key), but
the migrations have been modified to avoid this error. This does not
affect the migrations' behaviour in any way.
This works for other text boxes as well, but compose is the main one
that one would want to do a search from.
It's possible we'll find after doing this that "getting back into
compose" becomes a problem, but I guess we can handle that when the
time comes.
This is preparation for enabling an eslint indentation configuration.
90% of these changes are just fixes for indentation errors that have
snuck into the codebase over the years; the others are more
significant reformatting to make eslint happy (that are not otherwise
actually improvements).
The one area that we do not attempt to work on here is the
"switch/case" indentation.
This makes a few important cleanup changes:
* Using the more standard data-field-id name for the ID value.
* Using $(e.target).closest() rather than `.parent`, which is more
robust to future changes in markup.
Most of this was straightforward.
Most functions that were grabbed verbatim and whole from
the original class still have one-line wrappers.
Many functions are just-the-data versions of functions that
remain in MessageList: see add, append, prepend, remove as
examples. In a typical pattern the MessageList code becomes
super simple:
prepend: function MessageList_prepend(messages) {
var viewable_messages = this.data.prepend(messages);
this.view.prepend(viewable_messages);
},
Two large functions had some minor surgery:
triage_messages =
top half of add_messages +
API to pass three lists back
change_message_id =
original version +
two simple callbacks to list
For the function update_muting_and_rerender(), we continue
to early-exit if this.muting_enabled is false, and we copied
that same defensive check to the new function
named update_items_for_muting(), even though it's technically
hidden from that codepath by the caller.
For a commit that was just merged I had the "back-out" case
at the wrong nesting level. It was a pretty obscure failure
scenario that never came up in practice, but basically if you
were starting at a message that was not in your narrow, but
we did have some messages in your narrow, we would try to
go near the old message instead of talking to the server to
find the next unread message in that narrow.
Barring a few minor edge cases, when we now do a narrow
that is based on a sidebar-like search (e.g. stream/topic,
no extra conditions), we now go directly to either the
first unread message we know about locally or the last
message if we're all caught up.
We of course used to do this in master until recently; this behavior
was broken by Tim's narrowing refactor branch (ending with
26ac1d237b) which moved us to always
using the select_first_unread flag, by default (fixing issues where if
you clicked around while your pointer was behind, you'd land in the
wrong place).
We now have arguably the best of both worlds:
* The pointer is not considered when computing narrowing positioning
* We only go to the server for sidebar clicks if the data isn't
available in the browser.
We had a recent regression that had kind of a funny symptom.
If somebody else edited a topic while you were in a topic
narrow, even if wasn't your topic, then your narrow would
mysteriously go empty upon the event coming to you.
The root cause of this is that when topic names change,
we often want to rerender a lot of the world due to muting.
But we want to suppress the re-render for topic narrows that
don't support the internal data structures.
This commit restores a simple guard condition that got lost
in a recent refactoring:
see 3f736c9b06
From tabbott: This is not the correct ultimate place to end up,
because if a topic-edit moves messages in or out of a topic, the new
behavior is wrong. But the bug this fixes is a lot worse than that,
and no super local change would do the right thing here.
We now do real-time sync to update the attachments UI when new
attachments are uploaded/deleted.
While we're at it, we fix the UI for the delete option to not do a
weird local echo thing.
This completes the work of a couple issues. There's still useful
performance work to do here (see the TODO), but it's a minor issue in
a rarely-used screen.
Fixes#6731.
Fixes#3710.
Tim Abbott
Max Nussenbaum
Steve Howell
Priyank Patel
Yago González
Aastha Gupta
Yashashvi Dave
Shubham Dhama
Vishwesh Jainkuniya