This is the first part of a larger migration to convert Zulip's
reactions storage to something based on the codepoint, not the emoji
name that the user typed in, so that we don't need to worry about
changes in the names we're using breaking the emoji storage.
Here are the functions in top_left_corner:
get_global_filter_li: pure code move
update_count_in_dom: simplifed copy of similar function in stream_list.js
update_dom_with_unread_counts: pure code move, split out from function
of same name in stream_list.js
delselect_top_left_corner_items: pure code move
handle_narrow_activated: pure code move + rename
handle_narrow_deactivated: pure code move, split out from from function
of smae name in stream_list.js
Until we have an easy way to consistently determine whether a
stream has more topics than have been loaded already, we err
on the side of showing a "more topics" link. This in some ways
leads to a more consistent experience where you can zoom in on
any stream, even one that's really new.
This fix simplifies how we re-render topic lists when we
re-narrow or zoom out from a topic list.
* The topic_list.zoom_out() no longer gets called as
part of re-narrowing, and we eliminate the clear_topics
option.
* For all situations where we narrow to a filter that does
not have a topic, we simply call the new function
clear_topics().
* The stream_list code no longer calls remove_expanded_topics()
in cases where the new narrow has a topic. This allows us
to optimize away scroll/flicker churn a little more easily.
As part of this, we rename maybe_activate_stream_item() to
update_stream_sidebar_for_narrow(), since the function clears
stuff as well as turning stuff on.
We have code that can automatically scroll an element into "view"
in its container. We use this for stream sidebar rows inside the
stream list.
Generally the stream sidebar rows are small enough to fit into
the container, and the prior algorithm worked correctly for that
scenario.
If you have lots of topics, however, and a short screen, the
algorithm was being too aggressive. For example, if the top
wasn't showing, it would scroll the top into view, but at the
cost of scrolling the bottom out of view.
This fix makes the general scrolling algorithm more tame.
Part of the user-facing problem is that the element we pass
into the scrolling code for the stream sidebar rows is bigger
than the part of the row that actually should be shown on
screen. Nevertheless, it makes sense here to make the general
algorithm more robust.
If you read a message, then got a topic edit for it, we were
adding the message to our data structure of unread stream/topic
messages.
Now we guard against this in unread.update_unread_topics. I
no longer expose an update() method in unread_topic_counter,
since we really want to do the unread check at a higher level
to keep other data structures consistent.
We no longer use real jQuery to test topic_list. This changes
the nature of the tests to be higher level checks on how the DOM
is constructed. The actual details of how templates get
rendered should be in templates.js.
We are phasing out the following in tests:
add_dependencies - this is just kind of a clunky UI
require - normal JS requires cause test leaks
In order to plug require leaks, we are effectively doing what
we always have done inside of add_dependencies, which is to
keep track of which modules we have done `require` on, and
these get cleared between tests.
Now we just use `zrequire` every time we want to pull in real
code to our global namespace.
This code adds 'read' to message.flags and sets message.unread
to false.
It's not clear that the boolean message.unread is used in any
meaningful way, but we set it to false to avoid confusion. The
bankruptcy code was not doing this before.
Another quirk that existed before was that you could get two
'read' flags in a message when you declared bankruptcy. It's
also plausible that this could happen if you marked a message
as read via two different ways. It probably did not cause
user-facing bugs, but it would be confusing for troubleshooting.
Fixes#5032.
The new method borrows some code from the event loop
and unread_ops.mark_messages_as_read, and it is now
flexible about message_ids being marked as unread
even when there is no corresponding message in the
message store. For that scenario we still want to
update our data structures, which wasn't happening
before this change. (Generally, this was a non-issue
up until now, but it will become a bigger issue when
we start loading unread message ids from the server.)
This function allows us to see whether unread.js thinks a message
id is unread (as opposed to looking at the message itself). This
method is useful when we get notifications from the server that a
message has been read. In the future, we may not actually have
a local copy of an unread message, but we'll still know that it is
unread based on page_params. We'll want to update the data in that
case.
Going forward, we'll want to deprecate message.flags for most use
cases and just use the unread.js data structures to track unread
messages.
The prior implementation was needlessly complex. Both del() and
add() are cheap and idempotent.
With this change we no longer bother to delete a topic from a
dictionary when its last message is mark as read, since it doesn't
really help performance. We add a line to the tests to maintain
100% line coverage.
It's not always clear whether user_ids are strings or integers, so
we explicitly convert them to integers for sorting when creating
keys for PMs.
To keep the tests passing, this commit removes some unneeded
defensive code in message_store.js that only applies to contrived
test input.
This commit extract send_messages.js to clean up code related
to the following things:
* sending data to /json/report_send_time
* restarting the event loop if events don't arrive on time
The code related to /json/report changes the following ways:
* We track the state almost completely in the new
send_messages.js module, with other modules just
making one-line calls.
* We no longer send "displayed" times to the servers, since
we were kind of lying about them anyway.
* We now explicitly track the state of each single sent
message in its own object.
* We now look up data related to the messages by local_id,
instead of message_id. The problem with message_id was
that is was mutable. Now we use local_id, and we extend
the local_id concept to messages that don't get rendered
client side. We no longer need to react to the
'message_id_changed' event to change our hash key.
* The code used to live in many places:
* various big chunks were scattered among compose.js,
and those were all moved or reduced to one-line
calls into the new module
* echo.js continues to make basically one-line calls,
but it no longer calls compose.report_as_received(),
nor does it set the "start" time.
* message_util.js used to report received events, but
only when they finally got drawn in the home view;
this code is gone now
The code related to restarting the event loop if events don't arrive
changes as follows:
* The timer now gets set up from within
send_messages.message_state.report_server_ack,
where we can easily inspect the current state of the
possibly-still-in-flight message.
* The code to confirm that an event was received happens now
in server_events.js, rather than later, so that we don't
falsely blame the event loop for a downstream bug. (Plus
it's easier to just do it one place.)
This change removes a fair amount of code from our node tests. Some
of the removal is good stuff related to us completing killing off
unnecessary code. Other removals are more expediency-driven, and
we should make another sweep at ramping up our coverage on compose.js,
with possibly a little more mocking of the new `send_messages` code
layer, since it's now abstracted better.
There is also some minor cleanup to echo.resend_message() in this
commit.
See #5968 for a detailed breakdown of the changes.
Prior to this we were also performing highlighting inside HTML tags
which was wrong and causing weird behavior. Like, for example, if
someone added `emoji` as an alert word then any message containing
both emoji and alert word was rendered with a jumbo emoji.
Fixes: #4357.
A realm filter should match only after the start of a line, whitespace
or opening delimiters. But markdown was not configured to respect those
rules which was causing some weird rendering behavior. This commit fixes
the regex used for matching realm filters. On the backend we are using
regex with negative lookbehind to perform matches but since javascript
regex don't support lookbehind we are using a workaround on the frontend
using `contains_backend_only_syntax()` function which detects if a realm
filter can be rendered correctly by backend only and if so it stops the
message from getting echoed locally.
Fixes: #5154.
While we do have some known cases where syntax diverges intentionally,
this change should make it a lot easier to maintain
markdown.contains_backend_only_syntax over time.
It appears that a regression introduced in
3f60074c33 caused undefined to be passed
as the subject to the recent_senders library much more often; this
fixing that, and makes the library handle such cases reasonably
without an exception regardless.
This was causing a huge number of "Tried to call a Dict method with an
undefined key." exceptions.
This mostly moves code, and it also removes some unnecessary
coupling to stream_data.js. The topic_data code purely
works in the stream_id space, so there's no need to set up
actual stream data for it.
We now call topic_data.add_message() and
topic_data.remove_message() when we get info about
incoming messages. The old way of passing in a boolean
made the calling code hard to read and added unncessary
conditional logic to the codepath.
We also have vague plans to change how we handle
removing topics, since increment/decrement logic is now
kind of fragile, so making the "remove" path more explicit
prepares us to something smarter in the future, like just
figure out when the last topic has been removed by calling
a filter function or something outside of topic_data.js.
Another thing to note here is that the code changed here
in echo.js is dead code, since we've disabled
message editing for locally edited messages. I considered
removing this code in a preparatory commit, but there's
other PR activity related to local echo that I don't want
to conflict with.
One nice aspect of removing process_message() is that
the new topic_data.js module does not refer to the legacy
field "subject" any more, nor do its node tests.
This commit introduces a per-stream topic_history class
inside of topic_data.js to better encapsulate how we store topic
history.
To the callers, nothing changes here. (Some of our non-black-box
node tests change their way of setting up data, though, since the
internal data structures are different.)
The new class has the following improvements:
* We use message_id instead of timestamp as our sorting key.
(We could have done this in a prep commit, but it wouldn't
have made the diff much cleaner here.)
* We use a dictionary instead of a sorted list to store the
data, so that writes are O(1) instead of O(NlogN). Reads
now do sorts, so they're O(NlogN) instead of O(N), but reads
are fairly infrequent. (The main goal here isn't actually
performance, but instead it just simplifies the
implementation.)
* We isolate `topic_history` from the format of the messages.
This prepares us for upcoming changes where updates to the
data structure may come from topic history queries as well
as messages.
* We split out the message-add path from the message-remove
path. This prepares us to eventually get rid of the "count"
mechanism that is kind of fragile and which has to be
bypassed for historical topics.
This new module tracks the recent topic names for any given
stream.
The code was pulled over almost verbatim from stream_data.js,
with minor renames to the function names.
We introduced a minor one-line function called stream_has_topics.
We now have all of our callers into recent_topics code just
receive a list of topic names from get_recent_topic_names().
This is more encapsulated than handing off tiny little
structures to the three callers, two of whom immediately
mapped the objects to names, and one of whom needlessly
used the now defunct name canon_subject field.
The consolidation here removes some "subject" references, and
now all lookup are by stream id, not stream name.
The diff here is a bit daunting, but it's mostly simplification
of tests and calling code. Two of the callers now need to look
up stream ids, but they are otherwise streamlined.
The main change here is to stream_data.js, and we replace the
`canon_subject` and `subject` fields with `name`.
We use multiple casings of "lunch" as a topic in our tests, so
that we verify that unread counts respect that topics are
not case sensitive.
We also eliminate an obsolete stub.
Modified timerender.js absolute_time() to include the year
in the returned string when the supplied timestamp is in
an older year. This included adding an optional second
argument to specify the current date to facilitate unit
tests.
Fixes#5737.
This commit specifically addresses the issue when in preview mode,
while "enter sends" is enabled. Previously the messages were just
sent, now they must pass validation.
Fixes#5574.
This allows us to reliably parse the error in code, rather than
attempt to parse the error text. Because the error text gets
translated into the user's language, this error-handling path
wasn't functioning at all for users using Zulip in any of the
seven non-English languages for which we had a translation for
this string.
Together with 709c3b50f which fixed a similar issue in a
different error-handling path, this fixes#5598.
This function no longer sets properties to false, so the supported
way of doing this is to instead use prop(foo, false). Some tests
had to be fixed to accommodate this.
In case the user was not allowed to upload an emoji, we were displaying
two different but sematically same tips. This commit merges them and
also updates `update_custom_emoji_ui()` function in settings_emoji.js
to live update tooltip.
Our logic for editing failed messages is broken in various ways,
so we are removing the codepath for editing for now. We will
try to restore these features as part of #5841.
Because of local echo, message ids can change in message rows.
Having reactions use markup to indicate their message id just
creates more moving parts, since we would need to handle
message_id_changed events.
Now our handlers just call row.get_message_id() as needed.
We no longer do the message_store piece of reifying ids
via a trigger. We now make an explicit call to an
ordinary function.
This has several benefits:
- no more initialize() function
- no more scary comments about garbage collection
- the function has a real name now
- the function is less indented
- we can easily see when the message_store step happens
- simpler node tests
- simpler tracebacks (no jQuery cruft)
This commit removes all code related to headers because
(1) we don't need the code and (2) it splits #**stream**
as a paragraph, which we don't want. This commit also
fixes the inconsistency when #**stream** is on a new line.
Fixes#4678.
This allow the webbpack dev server to properly reload JavaScript modules
while running in dev without restarting the server. We need to connect
to webpack-dev-server directly because SockJS doesn't support more than
one connection on the same host/port.
We eliminate `.get(0)` calls in buld_stream_list.
The easy case is that we stop building jQuery objects
for the splitters only to pull out the DOM immediately.
The more subtle case is that we also don't do `.get(0)` calls
to get DOM out of our individual list items. By passing
in full jQuery objects to `append()`, we should prevent ourself
from orphaning the old objects, which may in the future have
things like tooltip logic attached to them.
The user mention regex was checking for multiple lines,
so it broke when the user mention was on a new line.
This changes the regex AND adds a couple tests to
test inline markdown regexes.
Specifically, this checks to make sure that if you
surround an operand with quotes, having spaces inside
is permitted. Also, an extra space after the operator is
also permitted.
I deleted a test case that involved a highlighted stream, but
the query was empty. This produces kind of a weird result with
typeahead_helper.highlight_with_escaping, but this function already
has coverage in node_tests/typeahead_helper.js, so the check here
was essentially redundant anyway. Specifically, the highlighter
wraps every character individually with <strong>, and looks really
messy in html.
I pushed a bunch of commits that attempted to introduce
the concept of `client_message_id` into our server, as
part of cleaning up our codepaths related to messages you
sent (both for the locally echoed case and for the host
case).
When we deployed this, we had some strange failures involving
double-echoed messages and issues advancing the pointer that appeared
related to #5779. We didn't get to the bottom of exactly why the PR
caused havoc, but I decided there was a cleaner approach, anyway.
This change has us tracking messages as soon as we start
sending the message to the server. The next step is to
reconfigure the timeouts a bit to deal with the server not
responding.
We now use a client-side message id to track the state of our
sent messages. This sets up future commits to start tracking
state earlier in the message's life cycle.
It also avoids ugly reify logic where we capture an event to
update our data structure to key on the server's message id
instead of the local id. That eliminates the node test as well.
Another node test gets deleted here, just because it's not
worth the trouble with upcoming refactorings.
This mostly sets the stage for a subsequent commit to start
using client_message_id as the key into sent_messages.
It has the nice side effect of making it more explicit that
certain things should always happen when transmit_message()
succeeds.
This commit does regress our node test coverage a bit.
This commit starts to decouple client_message_id from local_id.
We don't really take advantage of the decoupling in this
commit--in fact, it's a bit of a pain at first. But this should
be a fully working checkpoint commit.
This is mostly straightforward moving of code out of compose.js.
The code that was moved currently supports sending time
reports for sent messages, but we intend to grow out the new
module to track more state about sent messages.
The following function names in this commit are new, but their
code was basically pulled over verbatim:
process_success (was process_send_time)
set_timer_for_restarting_event_loop
clear
initialize
All the code in the new module is covered by previous tests that
had been written for compose.js. This commit only modifies
a few things to keep those tests.
The new module has 100% node coverage, so we updated `enforce_fully_covered`.
We are deprecating local_id/local_message_id on the Python server.
Instead of the server knowing about the client's implementation of
local id, with the message id = 9999.01 scheme, we just send the
server an opaque id to send back to us.
This commit changes the name from local_id -> client_message_id,
but it doesn't change the actual values passed yet.
The goal for client_key in future commits will be to:
* Have it for all messages, not just locally rendered messages
* Not have it overlap with server-side message ids.
The history behind local_id having numbers like 9999.01 is that
they are actually interim message ids and the numerical value is
used for rendering the message list when we do client-side rendering.
This was previously failing because we didn't wait for a particular
state after logging in, and the title changes at least once during the
process after the URL changes to the Zulip homepage.
In typeahead_helper.js, added a compare function to first sort by
subscription, then by pm partners and lastly based on recency in the
current topic. Altered function sort_for_at_mention to take topic data
and sort using the above function. Also altered node tests for
typeahead_helper.js to test for the above added functionality.
Fixes: #4249
There is no reason to render the template for compose mention
warnings if the user is already in the widget.
This commit also restructures the unit test significantly to more
carefully exercise each case, particularly in regard to when
templates get rendered.
This commit add $.create(), which allows you to create a
jQuery object that just has a name to identify it, as opposed
to some selector or HTML fragment. It's useful for things that
are really used as stubs.
This also fixes a bunch of the existing tests to use $.create().
Before this fix, you could actually just do $('some-stub'), but
now we enforce that the input to $() looks like a valid selector
or HTML fragment, and we make some exceptions for things like
window-stub and document-stub.
Hopefully this will make it more explicit that zjquery does
not truly simulate DOM, but it instead allows you to dynamically
set what you want the results of $('foo').find(some_selector)
to be.
Before this commit, we were erroneously setting up parents
as part of add_child() calls, but it's not necessarily the
case that those children are immediate children, and therefore
the first object is not necessarily the immediate parent.
Our current workflow for creating a new stream allows the user to
invite as many other users as they like but since there can be
mistakes in doing so, we now open a modal with a warning if the
number of invites are more than 100 just to confirm that user indeed
wanted to do this.
Fixes: #1663.
This system hasn't been in active use for several years, and had some
problems with it's design. So it makes sense to just remove it to declutter
the codebase.
Fixes#5655.
Fixes#5612. What this specifically does is that if you are
typing a group PM, this logic iterates through the possible
search suggestions for the next autocomplete. If that suggestion
contains a group PM that already exists, then prioritize it with
the most recent one on top.
This test verifies the following line of code:
$(document).trigger($.Event('subscription_remove_done.zulip', {sub: sub}));
Before this change, the mocking mechanism used `$(document).on(...)`
to set the function that gets called on trigger, but it didn't clear
any of the other handlers.
Since all we care about is making sure that the event gets triggered,
we now just override `$(document).trigger`.
Tim Abbott
Steve Howell
Harshit Bansal
Jack Zhang
Steve Howell
Rishi Gupta
Joshua Pan
digi0ps
David Coleman
hollywoodno
Greg Price
Sarah
Cory Lynch
Cynthia Lin
Pweaver (Paul Weaver)
Brock Whittaker
Vaida Plankyte
Aditya Bansal
Durga Akhil Mundroy
Vishnu Ks