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>
This feels a bit more semantically appropriate: it more clearly says
"here's some information: there is no (relevant) recipient", rather
than "no information available". (Both `null` and `undefined` in JS
can have either meaning, but `undefined` especially commonly means
the latter.)
Concretely, it ensures a bit more explicitness where the value
originates: a bare `return;` becomes `return null;`, reflecting the
fact that it is returning a quite informative value.
Also make the implementation more explicit about what's expected here,
replacing truthiness tests with `!== null`. (A bit more idiomatic
would be `!= null`, which is equivalent when the value is well-typed
and a bit more robust to ill-typing bugs. But lint complains about
that version.)
It'd already been the case for some while that calling `stop` had the
same effect as calling `update` (previously `handle_text_input`) with
a falsy recipient. With the API changes in the previous few commits,
this becomes quite natural to make explicit in the API.
This was named after when it gets called from the UI, rather than
after what it can be expected to do.
Naming it after what it's meant to do -- and giving a summary line to
expand on that -- provides a more helpful semantic idea for reasoning
about the function. Doubly so for using the function in a different
client with its own UI, like the mobile app.
The main motivation for this change is to simplify this interface
and make it easier to reason about.
The case where it affects the behavior is when
is_valid_conversation() returns false, while current_recipient
and get_recipient() agree on some truthy value.
This means the message-content textarea is empty -- in fact the
user just cleared it, because we got here from an input event on
it -- but the compose box is still open to some PM thread that we
have a typing notification still outstanding for.
The old behavior is that in this situation we would ignore the
fact that the content was empty, and go ahead and prolong the
typing notification, by updating our timer and possibly sending a
"still typing" notice.
This contrasts with the behavior (both old and new) in the case
where the content is empty and we *don't* already have an
outstanding typing notification, or we have one to some other
thread. In that case, we cancel any existing notification and
don't start a new one, exactly as if `stop` were called
(e.g. because the user closed the compose box.)
The new behavior is that we always treat clearing the input as
"stopped typing": not only in those cases where we already did,
but also in the case where we still have the same recipients.
(Which seems like probably the common case.)
That seems like the preferable behavior; indeed it's hard to see
the point of the "compose_empty" logic if restricted to the other
cases. It also makes the interface simpler.
Those two properties don't seem like a coincidence, either: the
complicated interface made it difficult to unpack exactly what
logic we actually had, which made it easy for surprising wrinkles
to hang out indefinitely.
Returning true from this function means we go on to send, or extend
the lifetime of, a typing notification; returning false means we don't.
It's hard to see why having a partially-entered name in the recipient
box should mean we're *more* inclined to send a typing notification to
the set of recipients that are already entered; if anything, it seems
like it should make us *less* inclined to do so. So we're better off
without this conditional.
The conditional was introduced in commit 72295e94b, as part of a
conversion from user emails to user IDs; there, it seems to replace a
condition that went in the opposite direction, returning *false* if
there were any invalid emails in the recipient box. So perhaps it's
just inverted.
Moreover, the (re-)inverted version would also be wrong: if the user
is typing a PM addressed to some users, and they hit send, the message
will go to those users whether or not they have any unconverted text
in the recipients box. So the typing notifications should too.
The real purpose these two callbacks serve is exactly what an ordinary
parameter is perfect for:
* Each has just one call site, at the top of the function.
* They're not done for side effects; the point is what they return.
* The function doesn't pass them any arguments of its own, or
otherwise express any internal knowledge that doesn't just as
properly belong to its caller.
So, push the calls to these callbacks up into the function's caller,
and pass in the data they return instead.
This greatly simplifies the interface of `handle_text_input` and of
`typing_status` in general.
This is intended as a pure refactor, making the data flow clearer in
preparation for further changes. In particular, this makes it
manifest that the calls to `get_recipient` and `is_valid_conversation`
don't depend on anything else that has happened during the call to
`handle_text_input`.
This is indeed a pure refactor because
* is_valid_conversation itself has no side effects, either in the
implementation in typing.js or in any reasonable implementation,
so calling it sooner doesn't affect anything else;
* if we do reach it, the only potentially-side-effecting code it's
moving before is a call to `stop_last_notification`, and that in
turn (with the existing, or any reasonable, implementation of
`notify_server_stop`) has no effect on the data consulted by
the implementation of `is_valid_conversation`.
Users generally don't expect wildcard mentions in muted streams and
topics to be treated as a mention, either for the purposes of desktop
notifications or the unread mention counts.
This fixes the unread mention counts part of the issue.
Fixes part of #13073.
When email address visibility is set to everyone, there is no change in
behavior, but when it is set to "admins-only", we don't show any email
in user profile modal (just like popovers) for everyone but admins.
When email address visibility is set to everyone, there is no change in
behavior, but when it is set to "admins-only", we don't show any email
in popovers for everyone but admins.
It should be azuread-oauth2-wrapper, as the name of the corresponding
backend is 'azuread-oauth2'. Without the correct name, the icon isn't
showing on the "Log in with AzureAD" button.
This adds the general machinery required, and sets it up for the file
`typing_status.js` as a first use case.
Co-authored-by: Anders Kaseorg <anders@zulipchat.com>
These indeed used to be strings, but were converted to arrays in
b8250fc61, and these names didn't get updated to match.
A classic example of why type-checking is a great job to get
machines to do. :-)
There are a few outstanding issues that we expect to resolve beforce
including this in a release, but this is good checkpoint to merge.
This PR is a collaboration with Tim Abbott.
Fixes#716.
When a user performs a search that might contain historical public
streams messages that the user has access to (but doesn't because
we're searching the user's own personal history), we add a notice
above the first search result to let the user know that not all
messages may have been searched.
Fixes#12036.
A somewhat recent refactoring of the left sidebar had introduced a gap
between the hover areas that looked off; this fixes this with a slight
rearrangement with where the 1px of space between elements lives.
Fixes#12508.
In 50545a3 we made an incomplete revert of some style changes from
7b8da9b, this commit reverts the "x" to "fa fa-times" and also fixes an
alignment issue for the "Discard" box in chrome.
Fixes#13233.
This fixes a glitch where the keyboard shortcuts icon, which is meant
to be a feature of the right sidebar, appears overlapping the "Reply"
button.
Fixes#13122.
When typing_status adds 10000 to this value, it would previously
obtain wacky strings like
"Fri Oct 04 2019 16:45:59 GMT-0700 (Pacific Daylight Time)10000"
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The previous code for ensuring the sort order of emoji choices was
correct relied on an OrderedDict structure, which isn't guaranteed to
be preserved when passed to the frontend via JSON (in fact, it isn't,
since we converted the way page_params is passed to use
sort_keys=True). Switch it to a list of dictionaries to correct this.
Fixes#13220.
The avatar source was misspositioned when avatar changes were disabled.
This also repositions the "X" for when avatar changes are allowed.
Fixes#12524.
The historical behavior of having `Enter` exit was optimized for the
"View source" use case; but `Esc` now handles that reasoanbly, and we
really should make it convenient to type in the user-editable text
box here.
Fixes part 1 of #11834.
This ensures that typing '```java' and pressing enter would result in
getting dropped into a java codeblock instead of javascript codeblock.
We implement this by pushing the exact match of a query to be pushed to
the top of the returned matches in `sort_languages`.
With some comments added by tabbott in the tests explaining the
current reasoning.
Fixes#13109.
Apparently, the changes in fe2adeeee1 to
fix a Firefox focus bug accidentally had the side effect of removing
the topic text box from the area being considered, resulting in the
escape key no longer working to end the message edit from within that
text box.
This is a simple and small commit which will alphabetically order the
entries of the fixtures dropdown menu in the "integrations developer
panel" devtool.
Apparently, the Zulip notifications (and resulting emails) were
correct, but the download links inside the Zulip UI were incorrectly
not including S3 prefix on the URL, making them not work.
While we're at this, we rewrite the somewhat convoluted previous
system for formatting the data export output.
This has two purposes:
1. Prevent stupid stacks of diacritical marks from overflowing into
other messages. Fixes#7843.
2. Prevent Chrome from collapsing the inside bottom margin with the
.messagebox outside (in a way that Firefox doesn’t).
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Bootstrap v2.2.0^2~40^2~6 changes this default to false, so this is a
prerequisite to upgrading Bootstrap, and it’s also safer.
This closes an HTML injection path via user full names in the emoji
reaction tooltip. It doesn’t appear to be exploitable for cross-site
scripting because we disallow `>` in full names, and the code happens
to be written such that the next `>` is in a different parser
invocation.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Anders Kaseorg
Greg Price
Rohitt Vashishtha
Vishnu KS
Tim Abbott
chgl
Pragati Agrawal
Mateusz Mandera
Vinit Singh
Ryan Rehman
YashRE42
Dinesh
Ivan Mitev
YashRE42
bartek
Hemanth V. Alluri
Joao Mauricio Carvalho