The selector we were passing to `condense_and_collapse`
included rows from our drafts UI, which don't have
zids and don't play nice with condense/collapse code
(which expects message ids for settings things like
`.condense` flags).
Now we just use a better selector.
If folks use an overly broad selector for message rows,
they will accidentally include drafts from the drafts
dialog, which won't have zids. More specific selectors
will be more efficient and possibly prevent strange
behaviors.
For testing convenience, we extract the message.
We now handle the esc key completely within the
keydown handler that we already have for message
editing. We allow escape to work no matter what
the focused element is within an edited message,
and we blur that element properly and end the
edit.
We remove all the strange, duplicated logic
from hotkey.js.
This should also fix a blueslip error where the
hotkey code was passing message_edit a jQuery
element with zero length.
Fixes the traceback reported in #14151, though we should still look at
the DOM cleanup discussed there.
The UI in the `#settings/notifications` page is updated similarly
to what is done in the `update_global_notifications` path present
in the `server_events_dispatch` file.
We have an alert for when the stream name is changed.
This also adds an alert when subscription settings
are updated and the widget is similar to that used in
the settings page.
This is also necessary because the stream specific
notification settings UI updation goes through this
path and it is necessary to display a confirmation
to match with other settings confirmation pattern.
Since each element containing the `.alert-notification`
class has a predefined area that wont overlap with any
other element, we make changes to the CSS so that it
just stays hidden until the text appears.
This setting is being overridden by the frontend since the last
commit, and the security model is clearer and more robust if we don't
make it appear as though the markdown processor is handling this
issue.
Co-authored-by: Tim Abbott <tabbott@zulipchat.com>
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
While we could fix this issue by changing the markdown processor,
doing so is not a robust solution, because even a momentary bug in the
markdown processor could allow cached messages that do not follow our
security policy.
This change ensures that even if our markdown processor has bugs that
result in rendered content that does not properly follow our policy of
using rel="noopener noreferrer" on links, we'll still do something
reasonable.
Co-authored-by: Tim Abbott <tabbott@zulipchat.com>
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
If we can't find data on a mentioned user to update its full_name to
the current value, we'll have to go with the value in the message
itself.
This can happen if e.g. we hard-deleted the originally mentioned user
from the database (which can sometimes happen after a "delete my
account completely" request).
The user has an option for setting global
notification settings as well as the same settings
for individual streams. Currently the user has to
keep track of each unmatched stream and then visit
each individual stream whose settings he wants to
update.
Thus this adds a dedicated UI table allowing the user
to view and update the notifications of the specific
streams which differs from the global settings.
It is located on the same page where the user defined
global notification settings can be modified.
Fixes#9228.
Currently we are updating the checkbox UI as soon as the user clicks.
This block is removed to match with the pattern of rest of the
properties in the stream edit page where `stream_events.update_property`
is responsible for updating the UI after a successful server response.
This function returns a list of objects to create a
list_render object, and each item contains the streams
whose atleast one notification setting differs from the
default set by the user.
This is done by comparing the global settings in the
`#settings/notifications` page with those settings
present in the subscribed streams.
Work towards #9228.
This flag was used to delay unread count updates while the bankruptcy
modal was visible. Now that bankrupcty is no longer a modal, we don't
need this flag at all.
Switched to top-of-page prompt to make it natural to fit in with other
notifications. As we switch to panel-based prompt, templates for the
bankruptcy modal are moved along with its usage in application's
homepage.
We include a bit of delay before reloading to make it easy for the
user to read the "Marking all messages as read" banner before it is
covered by the "Reloading..." notice in environments where the reload
is fast.
Fixes#3347.
When stream_post_policy modal is closed either after saving or using
cancel button or cross button, the pointer-events is set to none which
does not allow to close the stream settings overlay on one click.
Added overlay.close_modal on saving such that pointer-events:none is
removed.
Added line which removes pointer-events:none again on clicking cancel
button or close icon.
This is a prep commit which extracts the part of the code in open_modal
and close_modal to separate methods which adds inline style of
pointer-events to enable/disable the background mouse events.
Block comments are added for easy understanding of reader.
This commit removes "font-weight: 500;" from landing-page.scss so as
to fix a bug on landing pages that used the `markdown` class to
format content. The bug was caused by "a:hover" from landing-page.scss
overriding the font-weight (600) on links as set by the markdown
class, this caused the text to seem jumpy when one hovered over links.
Note from tabbott: The original code was added in
d7f5f31f6a, which doesn't explain it's
purpose, but it predates the more complete "markdown" CSS, was part of
an early prototype that had unfortunate hover behavior more generally,
and makes sense to remove.
Fixes: #14387.
If a non-author user clicked on view source in a poll and then close it,
the edit question icon would incorrectly get visible. This made changing
the question in local echo possible for non-author users.
Fixes: #14299
Starred messages from muted topics were not shown in the starred
messages view. Condition for muting_enabled is modified accordingly
such that the starred messages from muted topics is shown in the
starred messages narrowed view.
Node tests are updated accordingly.
Fixes#13548
The previous system for documenting arguments was very ugly if any of
the examples or descriptions were wrong. After thinking about this
for a while, I concluded the core problem was that a table was the
wrong design element to use for API parameters, and we'd be much
better off with individual card-type widgets instead.
This rewrites the API arguments documentation implementation to use a
basic sort of card-like system with some basic styling; I think the
result is a lot more readable, and it's a lot more clear how we would
add additional OpenAPI details (like parameter types) to the
documentation.
The previous logic avoided updating the setting for
non-administrators, because their value was always true, but removing
those if statements results in better test coverage and is more likely
correct if we ever try to support live-update for whether the user is
an administrator.
We've noticed that many production organizations don't set either an
organization description or profile picture, even large open source
organizations that could definitely take advantage of this feature.
This adds a top-of-page banner that bugs organization administrators
to add an organization description and profile picture, generally
starting on the second login (as we only do it on page load after
notifications are configured).
Significantly tweaked by tabbott to get the right user experience.
Fixes#14019.
The original implementation of panels.js was just for notifications,
and ended up running a bunch of notifications-specific code, including
registration click handlers and some localstorage-related
notifications logic, every time a panel was supposed to be opened.
This refactoring makes the panels library make sense -- we now
initialize all click handlers in the initialize() method, and do the
notifications check in a single, coherent place scoped to notifications.
Commit 68335d9124 removed the ability to tab
into this field, since it was a hidden field. This field is no longer
hidden, and this commit restores the ability to tab into it.
In continuation to #13250
CHANGES:
-the stream name edit button is now visible for long names too.
-ellipsis are removed when you click on edit name option.
-added border while editing name to give a text-box feel.
REASONS:
-added border while editing the name to give a textbox-esque feel.
-text overflow was changed from ellipsis to clip (while editing) as
ellipsis prevented editing the entire name (clip provides better
functionality).
The last two changes are reverted back to original (i.e. ellipsis and
no border) once you finish editing the stream name.
P.S.- clicking on anywhere else updates the new name perfectly
Here we have migrated checkboxes of all general notifications to the table.
By general notifications we mean, Mobile, Email, Desktop audio, and visual
notifications.
This is a part of a bigger migration to simply our notifications setting
changing infrastructure for all streams and individual streams. Later we
will add more row to this for different categories of notifications in
addition to the current ones ("Streams" and "PMs, mentions, alerts").
Fixes: #12182.
When you select a typeahead, it shouldn't
immediately do the action for you; you should
have to hit enter first. Even though 99% of
the time you're gonna confirm the typeahead,
it's jarring when you don't expect it.
You can still add a bunch of default streams
quickly, using only the keyboard, because
we have always had support for the enter
key saving. (and tab and enter also works)
This is a full-stack change:
- server
- JS code
- templates
It's all pretty simple--just use stream_id instead
of stream_name.
I am 99% sure we don't document this API nor use it
in mobile, so it should be a safe change.
We now only use `page_params.realm_default_streams` during
initialization, and then after that we use `stream_data`
APIs to get default stream ids and related info. (And
for the event that replace the data, we just update our
internal data structures as well.)
Long term we should have the server just send us ids here,
since we are now hydrating info from stream data in all places.
This code is a bit simpler.
The previous code was concatenating two lists
and then removing duplicates by calling filter().
Now we just have two loops that append to a single
list, and the second loop detects duplicates
before inserting into the list.
We also now use `default_stream_ids` instead of
`page_params` data, which is convenient for two
reasons:
- working with sets of ids is convenient
- we don't need to maintain `page_params`
data any more
We now use the up-to-date info from stream_data
to hydrate the default stream ids. All we need
here in the template is `invite_only` and `name`.
Since we are no longer using data from `page_params`,
we can remove `maybe_update_realm_default_stream_name`.
(If you are wondering if we still get live updates,
we get that via a more upstream call to
update_default_streams_table in the event
dispatching codepath.)
We only used get_default_stream_names() in a
test, so now it's being replaced with a function
that just gets ids.
We'll have use for get_default_streams_ids()
in an upcoming commit.
Now if a default stream gets deleted, we just
redraw the table. We always have a small number
of default streams, and the way that we were removing
rows without the actual consent of `list_render` was
really janky (and just a vestige of pre-list-render
code that never got fully ported).
This also makes us consistent with how we handle
added streams (i.e. just call
`update_default_streams_table`).
ASIDE:
Ideally we will update `list_render` at some point to
have an API for adding and removing elements. It does
allow you now to call `data()` to reset its data, but
for now we just build a new `list_render` object every
time.
Commit 03393631bd (#14142) regressed the
keyboard accessibility of the keyboard shortcuts modal. Fix it by
moving tabindex="0" to the scrolling element of the SimpleBar.
Fixes#14320.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
We stopped needing this with
0329b67048
(Dec 2016).
The function sets `bot.can_admin`,
which was only used in `bot_data.get_editable`.
We removed two tests (and then put back
some test setup that needed to leak down
to the last test).
This is code simplification motivated
by a recent bug that we fixed with some
server changes, but which was really
caused in some sense by our client code
using an overly finicky
condition to check falsiness.
For cross-realm bots, the value of
`user.bot_owner_id` may be `null`, or it
may simply be `undefined`, depending
on whether the server passes `None`
or simply omits the field.
We don't want out client code to be
coupled to that rather arbitrary
decision.
We were doing a `!== null` check instead
of checking for falsiness, which led to
blueslip errors in the past. Because a
bot owner id could be plausibly 0, a falsiness
check would be brittle in a different way.
Now we avoid that ugliness by calling
`get_bot_owner_user`, which either returns
an object or `undefined`.
And then the caller can just do a concise
check for whether `bot_owner` exists.
And we also fix up the crufty code that
was putting `bot_owner_full_name` on to
the object instead of using a local.
We have a bug report for this again, although
it might be on an old branch.
Fixes#13621.
This allows us to block use of the desktop app with insecure versions
(we simply fail to load the Zulip webapp at all, instead rendering an
error page).
For now we block only versions that are known to be both insecure and
not auto-updating, but we can easily adjust these parameters in the
future.
We know that by default the value of `enable_stream_push_notifications` is
true, so on some server if push notifications are disabled then due to
```
{{#unless show_push_notifications_tooltip}}
```
the `enable_stream_push_notifications` checkbox is rendered as unchecked
and when we update any other setting in the "Stream notifications" section,
due to `settings_notifications.update_page()` all checkboxes are changed to
current values which means above checkbox get checked.
We can deal with this bug either making changes in
`settings_notifications.update_page()` but we should show check/uncheck
state of checkboxes anyway so removing this `unless` condition fixes this
bug.
Instead of having logical expressions in templates, it's always preferred
to calculating them in javascript and pass the results as a context. It
also enhances the readability of templates and testing of such logic is
easier in js over templates.
`all_notification_settings_labels` is misleading that this variable is a
list of notifications setting labels so changed it to
`all_notification_settings`.
The reason for extracting this function is that getting the text, integer,
boolean value from the input elements (like checkboxes, dropdowns) is a
common task, and later we can use this function to get the input element
value in `settings_notifications` in the upcoming commit.
This is a bug fix where, if a list_render
object with the given name exists and it's items
have been sorted, then the filtered_list's data
does not get updated on re-rendering.
This line was present in the original commit
9576d5caef.
The use case for this are small or fixed tables, which do not need
filtering support. Thus we are able to not include the unnecessary
search input inside the html parent container.
It is not used at present, but will be required when we refactor
the settings pages.
We also split out exports.validate_filter function for
unit testing the above condition.
As a consequence of too many options in the bottom `Other permissions`
subsection, the `Save` button could end up too far up from the bottom,
such that it might appear offscreen on low-height laptops.
We fix this by reorganizing the settings in a way that is both more
intuitive and also ensures that none of the subsections are too tall.
Fixes: #14274.
Before this commit, the reactions code would
take the `message.reactions` structure from
the server and try to "collapse" all the reactions
for the same users into the same reactions,
but with each reaction having a list of user_ids.
It was a strangely denormalized structure that
was awkward to work with, and it made it really
hard to reason about whether the data was in
the original structure that the server sent or
the modified structure.
Now we use a cleaner, normalized Map to keep
each reaction (i.e. one per emoji), and we
write that to `message.clean_reactions`.
The `clean_reactions` structure is now the
authoritatize source for all reaction-related
operations. As soon as you try to do anything
with reactions, we build the `clean_reactions`
data on the fly from the server data.
In particular, when we process events, we just
directly manipulate the `clean_reactions` data,
which is much easier to work with, since it's
a Map and doesn't duplicate any data.
This rewrite should avoid some obscure bugs.
I use `r` as shorthand for the clean reaction
structures, so as not to confuse it with
data from the server's message.reactions.
It also avoids some confusion where we use
`reaction` as a var name for the reaction
elements.
We've often gotten the complaint that Zulip's emoji are a bit too big;
this should address the worst consequences of that (line-wrapping
being off with large emoji present) while still making it possible to
easily see what a given emoji is.
The right place to change this is in rendered_markdown.scss, not the
main emoji definition in zulip.scss, as the latter is also used in
places like the emoji picker where a larger size is valuable.
Closes#12731, an older PR that did this with slightly different
parameters (and without a comment).
Fixes#14254
You can test this on dev:
* do "-stream:Verona" in the search bar (the minus
sign negates the search here)
* reload the browser
You should see the same search (all streams besides Verona).
We simplify the code for deciding whether
we show a subscribe button or not, and in
doing so avoid a blueslip error where we
were passing `undefined` into `get_sub()`.
We had this API:
people.add_in_realm = full-fledged user
people.add = not necessarily in realm
Now the API is this:
people.add = full-fledged user
people._add_user = internal API for cross-realm bots
and deactivated users
I think in most of our tests the distinction between
people.add() and people.add_in_realm() was just an
accident of history and didn't reflect any real intention.
And if I had to guess the intention in 99% of the cases,
folks probably thought they were just creating ordinary,
active users in the current realm.
In places where the distinction was obviously important
(because a test failed), I deactivated the user via
`people.deactivate`.
For the 'basics' test in the people test suite, I clean
up the test setup for Isaac. Before this commit I was
adding him first as a non-realm user then as a full-fledged
user, but this was contrived and confusing, and we
didn't really need it for test coverage purposes.
Steve Howell
Hashir Sarwar
Ryan Rehman
Anders Kaseorg
Tim Abbott
majordwarf
sahil839
Siddharth Varshney
YashRE42
vaibhavrajsingh2001
Stefan Weil
Jagan
Puneeth Chaganti
Pragati Agrawal
Abhishek-Balaji
shubhamgupta2956