On a high-DPI display or with a non-default zoom level, the browser
viewport may have a width strictly between md_max = 767px and md_min =
768px. Use only the *_min bounds for consistency.
This requires queries with strict inequalities to express upper
bounds (width < md_min). Fortunately, that functionality is provided
by range context queries. Unfortunately, those are not supported in
all browsers. Fortunately, we can compile them away using
postcss-media-minmax. Unfortunately, postcss-media-minmax currently
subtracts 1px for strict inequalities anyway to work around a Safari
rounding bug. Fortunately, 0.02px should be sufficient for that, so I
submitted a PR:
https://github.com/postcss/postcss-media-minmax/pull/28
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The maybe_clear_subscribers() function was an artifact of
when we used to attach subscribers to the "sub" records in
stream_data.js. I think it was basically a refactoring
shim, and due to some other recent cleanup, it was only
used in test code.
We also change how we validate stream ids.
Going forward, peer_data just looks up stream_ids with the
normal stream_data API when it's trying to warn about
rogue stream_ids coming in. As I alluded to in an earlier
commit, some of the warning code here might be overly
defensive, but at least it's pretty self-contained.
In my recent commit to introduce get_user_set() I
inadvertently skipped one place to call it.
I also remove a return statement that was made
unnecessary by the new get_user_set() helper.
Now when we want to measure how long a block
of code takes to execute, we just wrap it with
`blueslip.measure_time`, instead of the awkward
idiom from my original commit of getting a callback
function.
My rationale for the original scheme was that I
wanted to minimize diffs and avoid changing
`const` to `let` in a few cases, but I believe
now that the function wrapper is nicer.
In a few cases I just removed the blueslip timing
code, since I was able to confirm on czo that
the times were pretty minimal.
We now use the same code in all places to
get the bucket of user_ids that correspond
to a stream, and we consistently treat
a stream as having zero subscribers, not
an undefined number of subscribers, in
the hypothetical case of us asking about
a stream that we're not tracking.
The behavior for untracked streams has
always been problematic, since if a
stream is untracked, all bets are off.
So now if we don't "track" the stream,
the subscriber count is zero. None of
our callers distinguish between undefined
and zero.
And we just consider the stream to be subscribed
by a user when add_subscriber is called,
even if we haven't been told by stream_data
to track the stream. (We also stop
returning true/false from add_subscriber,
since only test code was looking at it.)
We protect against the most likely source
of internal-to-the-frontend bugs by adding
the assert_number() call.
We generally have to assume that the server
is sending us sensible data at page load
time, or all bets are off.
And we have good protections in place
for unknown ids in our dispatch code
for peer_add/peer_remove events.
For the rare case where you're doing a link to a private
stream from a larger private stream that is a superset of
the former, we have bypassed warnings that you are linking
to a private stream.
I'm not sure we need this exemption for any situation
(just let the user bypass the warning), but we definitely
don't want false positives for the exemption.
For now I am closing down this loophole specifically for
Zephyr users.
Zephyr users are special in that we might not get
subscriber info on certain streams.
The current behavior for this edge case is a little
unclear. The current implementation of
peer_data.is_subscriber_subset returns false if both
streams are untracked, but most streams are tracked if we
have a sub for them and just get treated as having an
empty set of subscribers. And an empty set is a subset of
itself. Upcoming changes to our server data are gonna
make this edge case even more annoying to maintain.
We also streamline some of the error handling code
by doing everything up front. This will prevent
scenarios where a single bad stream_id/user_id causes a
bunch of the same warnings in an inner loop.
This removes a bit of complexity. If a piece of
settings code needs to render a stream with
subscribers, it just asks for it.
We no longer have the brittle, action-at-a-distance
mechanism of mutating the subscriber count on to
the stream_data version of a sub.
Stream subs are pretty small, so making copies of
them is cheap, and the blueslip timings from the
previous commit can help confirm that.
There is some discussion of putting `subscriber_count`
on the Stream model, which may eventually get us
away from tracking it in `peer_data.js`, but we will
cross that bridge when we get there. See
https://github.com/zulip/zulip/issues/17101 for
more details.
The weekly stream traffic is a better tiebreaker
for stream typeaheads than subscriber count, as
it's more directly a measure of a stream's current
relevance.
Normally stream traffic and subscriber counts are
closely correlated, but a good example for me is
the #twitter feed on czo, which only has 80 subscribers,
but which gets more traffic than our #integrations
stream (with 16k subscribers). I would rather
see #twitter win the tiebreaker (if it even got
to the tiebreaker).
The main motivation behind this fix, though, is
to break our dependency on peer_data, which has
some upcoming changes that will introduce some
performance tradeoffs, and I want one less place
to audit.
Also, it will be easier long term to share this
code with mobile if we don't require mobile
to pull in our peer_data dependency. (The webapp
has different forces than mobile that dicate
our data structures.)
The spinner icon is not visible until the user clicks on topic_edit_save,
so the space alloted to spinner-icon looks empty for rest of the time.
To improve the design, the spinner icon is only shown when the user
clicks on topic_edit_save.
Commit e941ee4a15 (#16680) incorrectly
converted this from 775px to xl-max = 1199px instead of md-max =
767px, causing misplacement of the FRB for browser widths between
these values.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We use day_old calculated based on day instead of hours to
render last seen values. This fixes us incorrectly quoting
anything 24 - 48 hours ago as Yesterday and
incorrectly quoting `time` that are Yesterday
but < 24 hours ago in 'x hours ago' format.
We were adding `expanded` class to left-sidebar when searching
for streams even if the left-sidebar was not in the popover state.
This cased confusion with popovers.any_active returning true,
when actually it is not.
We explicitly bind `this` to MessageState class which otherwise
was defaulting to `window`.
This resulted in variables like `this.received` and `this.local_id`
being incorrectly interpreted by called function
as `window.(received | local_id)` which are `undefined`.
Hence, frontend thinks that the message was never received.
It was noticed since this was the common log message when
a double message send bug was observed. This change in no
was indicates fixing of the double send bug, but is hopefully
one step forward in that direction.
topic_generator previously included an entire lazy generator
combinator library that was used four times. These straightforward
equivalent loops might not be as fun but they are way simpler.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We use 767px for hiding left column.
The components changed here were tested to be working fine.
This change is not likely to introduce any regression as the
calculations in the components here were not dependent upon the
breakpoint being at 775px.
We use 1199px for hiding right column.
The components changed here were tested to be working fine.
This change is not likely to introduce any regression as the
calculations in the components here were not dependent upon the
breakpoint being at 1165px.
While adding custom emojis, when a user clicks on the submit
button without providing a name to the emoji, the submit button
becomes unresponsive. This commit fixes that.
Fixes#16921
After this change all peer_data functions consistently
use stream_id rather than some "sub" object whose
data type is complicated by all sort of fields that
don't really concern how we track subscribers.
The goal here is to make all our peer_data functions
basically work in id space. Passing a full `sub`
to these functions is a legacy of when subscriber
info was attached to a full stream "sub" object,
but we don't care about anything sub-related
(color, description, name, etc.) when we are
dealing with subscriptions.
When callers pass in stream_id, you can be more
confident in a quick skim of the code that we're
not mutating anything in the "sub".
This de-clutters stream_data a bit. Since our
peer data is our biggest performance concern,
I want to contain any optimizations to a fairly
well-focused module.
The name `peer_data` is a bit of a compromise,
since we already have `subs.js` and we use
`sub` as a variable name for stream records
throughout our code, but it's consistent with
our event nomenclature (peer/add, peer/remove)
and it's short while still being fairly easy
to find with grep.
This sets us up to use better system-wide data structures
for tracking subscribers.
Basically, instead of storing subscriber data on the
"sub" objects in stream_data.js, we instead have a
parallel data structure called stream_subscribers.
We also have stream_create, stream_edit, and friends
use helper functions rather than accessing
sub.subscribers directly.
We now use add_sub only in tests.
The line to defensively initialize subscribers does
not get copied from add_sub, since we know that
create_sub_from_server_data always initializes
subscribers via set_subscribers.
After exiting lightbox view by pressing the browser back button,
future requests to open images were failing. This was because the
handler called on back button press- close_for_hash_change() was not
closing the currently open overlays gracefully. This commit fixes the
problem by calling the close_handler function inside
close_for_hash_change().
Fixes#16726
I added these hooks in Zulip Desktop 5.5.0; handling these events in
the frontend will let us remove the janky desktop-side fallback code
that uses fake click events on menu items with specific indexes.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
rustc's default edition is 2015 to preserve backwards compatibility, and
the playground appears to follow this scheme. However, 2018 edition Rust
is the current standard and is the default that Cargo uses when
initializing new projects. It adds support for various features,
including async/await and a new module system. As a result, I think
Zulip should default to 2018 edition when linking to the playground.
Users can always select a different edition once in the playground if
they would like.
This refactoring should have no functional effect for any call points,
but makes the function behave more naturally. The comments explain
the situation, but specifically:
* There's the page_params.narrow hack that affects both narrows and
home_msg_list.
* There's the shared data for home_msg_list and all_msg_list that
requires we modify the query from home_msg_list.data.public_operators().
And otherwise the logic should just use the operators associated with
the message_list.data object (allowing us to remove the force_fetch
hack added in the last commit).
Hopefully in some future refactoring, we'll be able to migrate those
hacks to live in the Filter object construction and eliminate this
block of conditionals entirely.
In commit ebea17b9a6,
we added an extra fetch to get accurate data for the top
items in recent topics table.
But the `narrow` parameter wasn't passed to the endpoint,
this resulted in fetching the user's overall message
history including the muted streams/topics which aren't
required by the recent topics table.
`operators` can be replaced as we set the same value for
the `narrow_state` module and the narrowed message list's
filter, when activating the narrow.
The changes made in this commit are as follows:
* The `remove_messages` is moved to the `message_events.js`
file from `ui.js`.
* We refactor `MessageListData.change_message_id` to no
longer require an `opts` parameter as this function
just returns whether we need to rerender or not.
The blueslip error block can be removed since we made
the change to no long defer the data updates in
commit 3b5ba6b2c1,
this case can no longer occur.
The changes made in this commit are as follows:
* We remove the now unused `ui.find_message` which was added
in commit 1666403850.
* We change the function paramter to now accept message ids
instead of messages to eliminate redundant message ids to
message convertion as only the id is required.
* The remove method in MessageListData did not remove the
messages from the hash, it removed only from the items,
this fixes it.
* This commit also fixes a bug where messages are not added
to the current message list if an event is recieved where
messages are moved to this current narrow.
Only the message removal logic was present, which has been
refactored in this commit.
Currently, the Stream Name change isn't reflected in the streams
sidebar when a stream is renamed if the order of streams in the
sidebar remains unchanged, because the optimization to avoid
rerendering when nothing changes about the order prevents the
rerendering code from running.
We fix by this adding a flag in build_stream_list and
update_streams_sidebar functions to force a rerender, and pass that
when a stream is renamed.
Fixes#16026.
The comment explains the problem statement in some detail, but
basically this algorithm ensures that the top items in "Recent Topics"
on page load are always the very most recent topics the user has
received messages in (well, ignoring muted topics in this iteration).
Steve asked me to remove this, since the tictactoe game was always
intended as a proof of concept. Now that we have poll and todo
widgets, the sample code for tictactoe has much less value.
We replace the content and type in test_widgets.py to maintain
coverage.
A convenient copy-to-clipboard button was added in the Invite users to
Zulip modal, to make it slightly more convenient to share the
generated links.
The formatting is extracted to a template to make i18n and variable
substitution simpler.
Tweaked by tabbott significantly to simplify JS, HTML, and CSS.
Fixes#16442.
Color-picker overflows the screen width when an user
attempts to change color of the stream in small devices.
Fixed by making it fullscreen in narrow devices.
Fixes#16477
Floating upwards caused a weird flickering effect if the mouse floated
onto the tooltip's body, and it's still reasonable UI floating left
(and also there's guaranteed to be space).
Fixes#16438.
This fixes a bug where the autocomplete for topics
deleted all the text content, if the topic jump is used
without entering any text.
The topic typeahead is automatically set up, on entering
the ">" key for stream completions. Therefore there is a
case where the user can select a typeahead item without
entering any text.
Thus the token length will be 0 and `beginning.slice(0, -0)` returns
"" instead of the `beginning` string. The case is only relevant for
"topic_list" completion as we don't set up the typeahead for empty
strings.
Fix this by reverting a hunk of
48f5e5179a, adding a test.
Fixes#16599.
Co-authored-by: Rohitt Vashishtha <aero31aero@gmail.com>
Rename zoom_xhrs to video_call_xhrs.
Rename abort_zoom to abort_video_callbacks.
Delete callbacks from video_call_xhrs when they have been aborted.
Move generation of video_call_id in the .videolink handler into
the Jitsi video call handling block as it is the only place it is
referenced.
We now can send an implied matrix of user/stream tuples
for peer_add and peer_remove events.
The client code basically does this:
for stream_id in event['stream_ids']:
for user_id in event['user_ids']:
update_sub(stream_id, user_id)
We used to send individual events, which gets real
expensive when you are creating new streams. For
the case of copy-to-stream case, we should see
events go from U to 1, where U is the number of users
added.
Note that we don't yet fully optimize the potential
of this schema. For adding a new user with lots
of default streams, we still send S peer_add events.
And if you subscribe a bunch of users to a bunch of
private streams, we only go from U * S to S; we can't
optimize it down to one event easily.
In Firefox, other Keyboard shortcuts stop working once the users
presses escape to exit the search bar.
Fix this by explicitly focusing the main panel after we exit search.
Fixes#16394.
As per https://stackoverflow.com/questions/1865837/ location.href
should be preferred to location.replace in some places due to the
fact that location.replace violates browser history and breaks back key.
Using web_public_guest for anonymous users is confusing since
'guest' is actually a logged-in user compared to
web_public_guest which is not logged-in and has only
read access to messages. So, we rename it to
web_public_visitor.
Anders Kaseorg
Steve Howell
Ganesh Pawar
Nikhil Maske
Rohitt Vashishtha
Aman Agrawal
Anders Kaseorg
Pranav2612000
Camelid
ryanreh99
Tim Abbott
YashRE42
Suyash Vardhan Mathur
aniketsonu
aryanshridhar
Vishnu KS
Harsh Sharma
sahil839
Greg Rowe
Aryan Shridhar