We received a complaint about the generation of multiple duplicate
drafts for a single message. It was discovered that the likely cause
of this was how we were handling clients that were frequently
suspending/unsuspending, we would initiate a reload when we discovered
this, and expect the `beforeunload` handler to save the draft. This
behaved correctly, however, we would also save the compose state and
fill it in via `preserve_state` in reload.js. The important detail
here is that `preserve_state` would not encode and preserve the
`draft_id` for the current message, partly because it had no way of
knowing the `draft_id` of the draft... since we have not saved it yet,
the `beforeunload` event happens after `preserve_state`. As such,
performing any action that would trigger a draft to be saved, eg
pressing Esc to close the compose box, would save a duplicate draft of
the same message.
To resolve the above bug, we (1) ensure that we call
`drafts.update_draft()` in `preserve_state`, this returns a draft_id
to us, which we (2) ensure that we encode as part of the url and (3)
set on the `#composebox-textarea` as a `draft-id` data attribute,
which we check the next time we try to save the draft, post reload.
Note that this causes us to save the draft twice, once from
preserve_state and then again from the `beforeunload` handler, but we
do not add two drafts since the second update_draft call just edits
the timestamp because it finds the `draft-id` data attribute on the
`#composebox-textarea` set by the first call.
Previously, opening a draft and closing it without changing the
content would cause us to show the "saved as draft" tooltip. This was
annoying and would cause annoying UX after fixing a bug related to
saving drafts when reloading, as such, this commit removes the above
behaviour by introducing a simple check on whether the draft contents
are edited.
In message header search bar, users didn't use to get any typeahead
suggestions if a normal filter follows search filter.
E.g.: query => foo bar stream:D
In the above case, users didn't use to get any typeahead suggestions.
This was because we had set that the callers of 'parse' function can
assume that the 'search' operator is present in the last in the query.
Because of which `get_search_result` function (in search_suggestion.js)
didn't use to show any typeahead suggestions as it used to assume that
the latest typed query is for search filters.
Fixes part of #19435.
We directly pass operators to remove dependency on narrow_state
module. This avoids a circular dependency of `filter` module
which is evident on the `/devtools/integrations/` page.
This commit extends dialog_widget class by adding a new
optional paramter validate_input which will be a function
to validate the inputs in the dialog and will be called
before showing the spinner and calling the on_click function.
Currently, the password change modal uses this paramter to
validate that the old and new password inputs must not be
empty. Since the spinner will not be initiated in the case
where form is invalid, we need not hide the spinner after
showing the error and thus we can simplify the code to use
ui_report.error to show the error messages of empty fields.
This fixes unexpected cursor repositioning behavior when the cursor was
positioned before or inside the "Quoting..." element.
The comments document the new logic, but roughly we aim to just
preserve the logical position of your cursor after replacing the
placeholder.
We also factor out a shared variable for the "Quoting..." string which
will allow us to tag it for translation in a future commit.
If you used "Quote and reply" to start composing a message, and
started typing before receiving the original message body from the
server, we ended up resetting your cursor to the start of the line
after the quote for two reasons:
* We were incorrectly fetching the pre_cursor for our replacement
operation before doing the server fetch, which meant we ignored any
editing done while waiting for the server to respond.
* Worse, we actually fetched the original cursor position before
inserting the "[Quoting...]" placeholder text. So we were guaranteed
to have at least some amount of error in the cursor position.
Fixes#20379.
Continuing the efforts to reduce dom trashing from the previous
commits, here we remove the third forced reflow by reordering the call
to $(".top-messages-logo").show() via narrow.reset_ui_state(), such
that it happens before the other DOM writes in
recent_topics_ui.hide().
Tweaked by tabbott to avoid adding an unnecessary if/else statement
around recent_topics_ui.hide.
This is a prep commit towards pushing the reset_ui_state calls upwards
into recent_topics_ui, in order to prevent a forced reflow. One side
effect of this change is that we add a call to
`narrow_banner.hide_empty_narrow_message()` from `narrow.activate()`.
This likely has no visible effect, in that the message list rendering
process would end up setting the narrow_banner state correctly, but
logically it could in the future avoid a banner from a stale banner
incorrectly appearing before we've rendered the current view.
This is a prep commit towards extracting a reset_ui_state function
which we can call from here, narrow.activate(), recent_topics.show()
and recent_topics.hide().
We want that function because it will enables us to prevent a forced
reflow when navigating from recent_topics to stream: xyz.
Going through the description of commit
a150b9b0ae is highly recommended since
this is a related issue.
We had received a complaint on chat.zulip.org about navigation with
the keyboard `n` key being significantly slow (~5 seconds), the first
time `n` was pressed when starting from the recent topics view.
It was difficult to reproduce the amount of lag that was reported, but
running chrome with the profile tab set to 4x slowdown helped get
close to it.
Based on profiling from the original report, as well as locally with
chrome set to 4x slowdown, led to the realisation that recent topics
to stream navigation involved a lot of dom thrashing, and so this
series of commits aims to prevent this path from causing forced
reflows.
In this commit, we reorder the calls to $(...).show() in
recent_topics_ui.hide(), this prevents the first reflow in this path,
most likely because displaying message_feed_container before
message_view_header_underpadding was guaranteed to cause style
recalculations since the underpadding is visually above the message
container.
This causes a net 60 ms decrease in the first renarrow, an ~ 70 ms
increase in the second renarrow and an ~ 5 ms increase in the third
renarrow but, more importantly, it eliminates one reflow and sets on a
path where we can achieve strong gains in subsequent commits.
We show "Please enter your password" error inside the modal
if the "Old password" input is empty and "Please choose a new
password" error if the "New password" input is empty and do
not send a request to server.
Fixes#19901.
The stream creation form currently does not setup its own handler for
displaying the "N:" input when ".stream_message_retention_setting" is
changed.
Prior to e793ef7f62d280300afeeab2f4a086e99858a5a9, this form would
sometimes work as intended because stream_edit would set the handler
on this dropdown, when it was opened. However, after that commit, this
would simply never work.
Hence, in this commit, we make changes so that stream_create correctly
sets the handler on its dropdown. This causes us to repeat ourselves a
little and as such is not the cleanest solution, but this might be the
best we can do due to the complications of stream_edit opening a
modal.
The stream creation form also uses the same stream_types template as
the stream privacy modal, however, it currently does not setup its own
handler for displaying the "N:" input when
".stream_message_retention_setting" is changed.
Previously, if one opened the stream creation form, then opened the
stream edit modal, and then went back to the stream creation form, the
drop down would correctly also .show() the input, because the handler
here would also target that selector. This is incorrect since we can't
always expect the stream_edit modal to be opened first, stream_create
should set up its own handlers.
Hence, as a prep commit to fixing stream_creation, and to ensure we
don't add duplicate handlers, in this commit we change all selectors
that targeted ".stream_message_retention_setting" to
"#stream_privacy_modal .stream_message_retention_setting" in this
file.
The public and private stream choices in stream creation form are disabled
according to create_public_stream_policy and create_private_stream_policy
settings. It is not needed to disable them in stream privacy modal since
only admins can change the privacy of stream and they are allowed to
create public and private streams always irrespective of the setting.
This commit has the following changes -
- Adds dropdown for changing create_web_public_stream_policy and this
dropdown is visible only if settings.WEB_PUBLIC_STREAMS_ENABLED and
enable_spectator_access is set to True. This dropdown is live-udpated
on changing enable_spectator_access setting.
- The web-public stream option in stream creation form and stream privacy
modal is hidden if one of settings.WEB_PUBLIC_STREAMS_ENABLED or
enable_spectator_access is set to False except in stream privacy modal
when the stream is already web-public so that the user is not confused by
none of the options being selected.
- We disable the web-public stream option in stream creation form and
in stream-privacy modals of stream which are not already web-public
when the user is not allowed to create web-public streams as per
create_web_public_stream_policy setting.
- We use on_show parameter to hide or disable the options in stream-privacy
modal because we use the visible property of element to remove the bottom
border from last element in the stream-privacy choices and thus we have
to wait for the modal to be visible.
Fixes#20287. Fixes#20296.
This commit adds user_can_create_web_public_streams function
in settings_data.js which will be used in further commits
to disable or hide the UI elements for creating web-public
streams.
We do not have 'realm_' prefix to the settings used as keys
in realm_settings object, we directly use the setting name.
This commit removes the 'realm_' prefix from enable_spectator_access
setting.
This is a very frequently requested feature for organizations that are
new to Markdown, that brings Zulip's UI more in line with that of
competing projects and other markdown editors like the GitHub UI.
This works surprisingly unlike my previous attempts to do so.
WARN: This is a pseudo commit and should only be merged with upcoming
compose box bottom refactoring commit since the css changes required
for this change are missing here and are not required after that
commit.
Since we have the drafts button in top left corner and we need space
to insert formatting buttons in the bottom of compose box; removing
drafts link makes sense.
This provides a convenient interface to hide all drafts.
Fixes#19360.
However, we may want to continue to implement a button in the drafts
overlay as well for doing this operation.
In commit 3d86267041 we add logic to
`/shared/emoji.js` which duplicated some of the logic in this
function. Since this isn't desirable, we remove the duplicate logic
here and instead just call `emoji.get_emoji_details_for_rendering`.
In commit 3d86267041 we add logic to
`/shared/emoji.js` which duplicated some of the logic in this
function. Since this isn't desirable, we remove the duplicate logic
here and instead just call `emoji.get_emoji_details_for_rendering`.
Previously, if a user had a realm emoji set as their status emoji and
someone deleted the realm emoji, the app would fail to initialize,
because of the error we throw from `./shared/js/emoji.js`.
This commit fixes this by just displaying the deactivated emoji,
similar to how we do when realm_emoji used as reactions are deleted.
As part of the fix, we add a function get_emoji_details_for_rendering,
which duplicates some of the logic used in `reactions.js`, we can
refactor to remove the duplication in `reactions.js` in future
commits.
Note that the following behaviour is a part of our design:
If a user sets their emoji to a particular realm emoji, say for
example "octo-ninja", and "octo-ninja" was then deleted, and a new
emoji was added with the name "octo-ninja", the user's status emoji
would change to show the new emoji instead of the deleted emoji.
Also note that in the `user_status.js` node test, we were able to
change the name for the 991 realm_emoji because it had not been
previously used anywhere in the test (possibly added as just a copy
paste artifact?).
Fixes: #20274.
emoji: Use reaction_type parameter to analyze emoji.
This commit renames "Automatic" option in color scheme setting
dropdown to "Sync with compute". We do not change any variables
used in code just the text in the dropdown visible to user.
Fixes part of #20228.
We restrict access of messages from web public streams if
anonymous login is disabled via `enable_spectator_access`.
Display of `Anonymous login` button is now controlled by
the value of `enable_spectator_access`.
Admins can toggle `enable_spectator_access` via org settings in UI.
Changes `update_page` to only update the modified user setting instead
of updating all of the user settings on the page. This is modeled on
the behavior for updates to the realm user default settings.
This is a follow up on feedback in #20070.
Now that it's further away from the composebox, we probably want it to
be visible for longer. Doubling it from 1.5 seconds to 3 seconds seems
reasonable to start with, although we should tune it based on feedback.
Now that this is in the left sidebar, we can remove the now-redundant
compose area button for it. This also changes where the "Saved as
draft" tooltip appears.
This currently shows the drafts as a popup. Eventually, we'll want to
migrate it to be a view in the center pane, as we did with Recent
Topics.
This uses the same style as starred messages in order to show the number
of drafts.
See CZO for more context:
https://chat.zulip.org/#narrow/stream/101-design/topic/drafts.20in.20sidebar
Since the position of topic in recent topics can change, we
focus the last selected topic using the `topic_key` instead
of relying `row_focus` value which is incorrect.
When user is scrolling, we simply keep the center element in
focus.
When user is using hotkeys, we keep the focused element in
center.
When user is using keyboard, we need to always keep the
"focused" topic in visible scrolling area.
We determine if the topic row is above or below the visible
area and scroll half_height_of_visible_area so that the selected
topic is visible.
This gives a nice navigation experience for both the views.
Reduced height of recent topics table to account for
compose box so that focused element is not below compose box.
Previously, opening multiple message_edits and then drag-dropping a
file into any one of them would cause all of them to upload ie you'd
get one uploaded file in each message_edit.
This bug was caused by returning multiple elements from
upload.get_item("drag_drop_container", config) when config.mode =
"edit".
This commit changes the selector to use the row provided (config.row),
and so ensures that the above bug doesn't happen.
`calendarContainer` is defined for flatpickr instance if it is
open.
This also fixes a bug where the flatpickr doesn't open when
user tries to toggle it using the global time icon in compose.
We need to handle live-update of twenty_four_hour_time setting
separately in update_page because the database value of this
setting is boolean but we use dropdown in the frontend for this
setting with option values as "true" and "false" strings.
There was no heading for "Time format" setting in the
"Default user settings" section and thus no save-discard
widget to update the setting. This commit fixes the bug
and changes the heading to be only "Time" since there is
no realm-level default of language setting.
This bug was introduced in adb612a0b4.
Flatpickr tries to show a different picker for mobile which
is not visible for some reason. We display the same picker
on mobile which we know works for our use case.
Docs: https://flatpickr.js.org/options/
```
Set disableMobile to true to always use the non-native picker.
By default, flatpickr utilizes native datetime widgets unless
certain options (e.g. disable) are used.
```
Previously, navigating from any stream to the recent topics view would
cause a forced reflow every time we checked `is_visible()` because it
would call `$("#recent_topics_view").is(":visible")`.
The reason for this is related to how browsers ship frames, the
process follows these steps:
JavaScript > style calculations > layout > paint > composite.
(The layout step is called Reflow in firefox.)
Typically, the browser will handle these steps in the most optimal
manner possible, delaying expensive operations until they're needed.
However, it is possible to cause the browser to perform a layout
earlier than necessary. An example of this is what we previously did:
When we call `top_left_corner.narrow_to_recent_topics()`, we ask to
add a class via `.addClass()`, this schedules a Style Recalculation,
then, when we call `message_view_header.make_message_view_header()` it
calls `recent_topics_util.is_visible()` which calls
`$("#recent_topics_view").is(":visible")`.
Before the browser can get this value, it realizes that our dom was
invalidated by `.addClass()` and so it must execute the scheduled
Style Recalculation and cause a layout.
This is called a forced synchronous layout.
This commit adds a JavaScript variable representing the visible state,
in order to prevent the above behavior.
This commit reduces the main thread run time of
`build_message_view_header` from 131.81 ms to 5.20 ms.
Unfortunately we still have the case where
`recent_topics_ui.revive_current_focus()` calls
`recent_topics_ui.set_table_focus()` which causes a reflow.
However, by eliminating this reflow we still save ~100ms.
(It's important to note that we only save this sometimes, as other
things can still cost us a reflow.)
Further reading: https://developers.google.com/web/fundamentals/
performance/rendering/avoid-large-complex-layouts-and-layout-thrashing
PR #19576 added a settings option for selecting a notification batching
period. We want to extend that UI option with the ability to select
a custom period.
Tweaked by tabbott to have the natural model that picking a value
present in the dropdown live-updates to remove the custom input,
rather than only having the custom input disappear on reload.
Fixes#19713.
It's always better to use the user ID than the email for fetching data
about an object whose unique ID we have, which should be all of them.
And it's also cleaner code to use the standard people.js method; tabbott
checked that indeed all callers get their `user` objects from `people.js`.
Since we restrict spectators from having access to avatars using
email to avoid someone brute forcing a user's email, this removes
a 401 response from the server in spectator view when trying
to open user info popover.
Additionally, this fixes the cached-fetching behavior documented in
the comments we add about the way we construct URLs.
This commit updates the error message returned when the maximum
invite limit for the day. We update the error returned by API to
only mention that the limit is reached and add the suggestion
to use multi-use link or contact support in the message shown
in webapp.
Add `escape_navigates_to_default_view` as a bool setting in
UserBaseSettings model and implement it as a checkbox that toggles
the hotkey implementation of escape to the default view in the
advanced user display settings.
With /help/ documentation edits from Alya Abbott.
Fixes#20043.
A user wouldn't differentiate between a "normal" modal and a "settings"
modal. If one shows up instantly, one would expect all the others to do
the same. The difference between Bootstrap fade and non-fade is pretty
noticeable (300 ms for fading).
This is a prep commit for the Micromodal migration which will have 120ms
as the animation time which wouldn't feel slow.
We attach the DOM for the modal to the body element
to avoid style interference from other elements and having to choose
a separate parent element for every single dialog_widget.
Previously, there existed a bug where clicking an open message edit
box when the compose box was open would not focus the message edit
contents, but would instead:
- focus the message edit content,
- close the compose box,
- and unfocus the message edit content.
That first bug was fixed in 4e1525d7c4
which eliminated the unfocus part of that problem, but closing the
compose box is undesirable here anyway, since often it can be useful
to have compose open while interacting with the sidebar filter widgets
or working on editing a message.
Click_handler.js was created in commit
e5467d3268 by moving code from ui.js.
While making this change, some handlers were rearranged, but the part
of this comment saying "... code above" was never changed. This commit
changes that part of the comment to refer to the correct handler
(which is labeled by a comment as MAIN CLICK HANDLER).
The `make_compose_box_original_size` function is designed in such a
way that it should only be called when it actually needs to resize the
compose box. In 3 of the 4 places that called it, we checked whether
the compose was already the original size; this fixes the 4th to do
the same.
Since it's a button, it doesn't need the "keydown" event. So,
removed it. This fixes the bug where pressing any key while the
avatar's delete_button was in focus would pop up the modal.
It was introduced in e5d0448505.
This provides a cleaner organization for our display settings, to make
browsing them more intuitive for new users.
We still need to update the /help/ documentation following this migration.
Fixes#19960.
Removes the `/day` and `/night` options from the typeahead menu while
still allowing the commands to be used. Typing `/day` and `/night`
will now suggest `/light` and `/dark`, respectively. Also changes the
`Dark mode` and `Light mode` popups that appear after using the
corresponding command.
Fixes#18318.
* Remove unnecessary json_validator for full_name parameter.
* Update frontend to pass the right parameter.
* Update documentation and note the change.
Fixes#18409.
This fixes an issue where the auto-complete dropdown doesn't reflect
the changed stream in the message edit UI.
We add an unlisten method to the typeahead library to support this
reinitialization cleanly and in a way that can be readily reused in
the future.
Fixes#19874.
It can be pretty annoying to lose your place when replying to an old
message, even though every other chat application does this. And it
doesn't really buy us much; the user can always scroll down if they
want to, we have a helpful notification about where their message is
(which could be improved), and then we don't need to add some sort of
new complicated logic to avoid marking messages as read unexpectedly,
which the existing logic for this block badly needed.
(It had existing logic of that form dating from the pre-unread counts
pointer era).
Fixes#11462.
The from_scroll=true setting has been present since essentially the
beginning of time (6ae117ea5f), and has
moved around a number of times since. It's possible that it was
correct with the UI model as originally implemented, but the behavior
it creates now is that sending a message in the home view does not
move the cursor, and sending in a narrow does, without any intent
behind that behavior.
Further, the logic for controlling whether to display a "Scroll down"
notification clearly expects that this code path will actually trigger
a scroll to the current message, which would be the case without the
from_scroll setting.
This commit refactors the display settings code to use
a single handler similar to what we do in notification
settings. We still keep default language and emojiset
setting as they are handled differently and they do
not call 'change_display_setting' directly on changing
input.
We refactor the 'change_display_setting' to directly
accept status element as parameter and not class as
a string. We also add a common class to the subsection
div such that we can get status element for each of
them easily.
We do not need to check for_realm_settings variable for
left_side_userlist handler since realm-level defaults
now uses save-discard widget and we return early in
that case and thus for_realm_settings will always be
false at this point.
There is a bug when a newly created stream appears in the notification
settings table if user changes any setting in 'Streams' row of the
table even though user has not changed the notification setting of
the stream and thus the stream should follow the global-level values.
The stream is correctly not added to the table if user changes the
setting after reloading once after creating the stream.
The bug is due to the notification settings of the new-stream being
set to the user's global settings at time of stream creation which
are not overridden since the stream-creation event contains only
stream fields and not subscription fields. And the newly created
stream is not present in the table after reload because during
initialization of the client-level data structures the notification
setting values are overridden by the values stored in server.
This commit fixes create_sub_from_server_data code to initially have
the notification settings set as null in the sub object which are
overridden by the correct value during initialization. This keeps
the notification setting values as null just after stream creation.
Fixes#19933.
This will be useful to let users enable/disable
sharing read receipts once we add that feature.
Note: Added "I've" to IGNORED_PHRASES in
tools/lib/capitalization.py to avoid capitalization
errors for the label text of this setting.
Note: These are not functional in enabling/disabling sending of
typing notifications with this commit.
Refactored the privacy settings update to keep the code less
duplicated along with making the addition of new settings easier.
This isn't necessary as `settings_checkbox.hbs` template used
for presence enabled setting in `account_settings.hbs` takes
care of checking/unchecking this checkbox.
I believe we intended to show 'several users are typing...' only
if number of typists are more than 3 but not 2.
The variable name MAX_USERS_TO_DISPLAY_NAME and commit message of
dba21d201c which added this also
suggests the same.
We remove patch_url from settings_notifications.user_settings_panel
since realm-level defaults section uses save-discard widget and its
code is separate from user notification settings and we can directly
use "/json/settings" as url in user settings code.
We do not need patch_url field in realm_default_settings_panel
because we use save-discard widget in realm-level defaults section
which is handled separately from user display settings.
We remove patch_url from settings_display.user_settings_panel
since realm-level defaults section uses save-discard widget
and its code is separate from user display settings and we
can directly use "/json/settings" as url in user settings code.
We don't need container element and patch_url in change_display_setting
since this function is only used for user-display settings and not for
realm-level settings which now uses save-discard widget. So we do not
pass settings_panel as parameter to change_display_setting.
We added language_modal_elem to settings_display.user_settings_panel
object considering there would be realm-level default_language
setting and we would need to access the modal element using a
variable to avoid code duplication. But now we have decided that
we do not want the setting and we would instead use browser's
language to set the language for new user, so now we do not need
this variable and we can instead use id of the modal directly.
This handler was broken during refactor
78d511fd03, as we can see from the
original implementation in 30065b4ee8,
the intent is that hovering over any link within the
narrow_description should not cause the search_icon to change color ie
the hover effect should not be used. This is so because it aligns
with the fact that clicking the links would not open the search bar.
However, during the refactor this was incorrectly switched to forcing
the effect to be applied when we hover over links in the
narrow_description.
This commit reverts to the original and intended behaviour, and also
switches to using opacity rather than color, in accordance with the
changes from the previous commit
(316d499ac74c2caddb57c98a43d9b776b1b32d98).
This commit removes unnecessary if condtions which are checking
for_realm_settings value which are basically present to handle
the code which is only for user-level settings, but since we
now return early for the realm-level defaults code we do not
need these conditions.
This is done in a separate commit just to make the original commit
adding save/discard widget easy to review.
Previously, on receiving udpate event of realm-level default setting,
we updated the whole page, but this might be problematic now in case
where user has edited settings in two subsections with save/discard
button still present and if user clicks on save button of one
subsection then the setting in other subsection also resets to its
original value as whole page is updated.
So, this commit changes the behavior to only update the changed
setting and not affecting other settings similar to what we do
in 'Organization settings' and 'Organization permissions' sections.
We also do not call 'settings_display.report_emojiset_change' when
realm-level default of emojiset setting is updated because we now
uses save/discard widget.
This commit adds save-discard widget in the realm-level defaults
section. We use most of the functions used in settings_org.js
by passing for_realm_default_settings and add conditionals
according to it.
Some of the major changes wrt to the organzation settings code
are -
- We use element name attribute here to get the setting name from
element instead of id. We can add id for the elements here but
there is a problem doing so for the emojiset setting as for the
radio buttons we use four different input elements and all being
for the same setting.
- Added separate cases in discard_property_element_changes and
get_input_element_value to handle the radio buttons.
- We do not need get_complete_data_for_subsection here because
all settings are controlled by single field in DB and single
element in UI and thus we can simply get changed setting values
from populate_data_for_request.
- Added org-subsection-parent to the subsection container and
prop-element to the input and select elements so that we can
use the existing code.
- Modified get_subsection_property_elements to just return the
input element which is selected for emoji-settings subsection and
not all the input elements because we only need the selected value
of emojiset. We need other elements also when discarding the changes
but we handle it separately.
This commit moves all the click handlers used for controlling the
behavior of save/discard widgets to a separate function such that
it can be used for realm-level default section also. This function
has container and patch_url as parameters.
This commit reorders the click handlers in notification settings,
such that we can easily move the click handlers for realm-default
settings to settings_realm_user_settings_defaults.js by returning
early.
We use 'admin-realm-form' class as selector in save-discard click
handlers instead of 'organization' which includes all the organization
settings sections, but save/discard widget is used only for some
sections and all of them are inside the form with class 'admin-realm-form'.
This will also help us to avoid code duplication when changing the realm
level defaults section to use save-discard widget.
This commit adds patch_url parameter to save_organization_settings
function such that we can use the same function for realm-level
defaults section also as those settings are updated using different
endpoint.
We directly declare the save_organization_settings function instead
of first declaring a variable and then setting it in the build_page
function. This is a prep commit for using save/discard widget in
realm-level defaults section.
Previously, the "resolve/unresolve topic" checkmark option was displayed in recipient bars
was presented to users regardless of whether they had permission to resolve topics in that
stream, which was confusing.
Fixes#19880.
Move the password tracking variables from setup to channel module.
Track password_change requests in channel.
Directly use xhr object to store `password_changes` information.
Tests modified to accomodote this change by converting strings to
objects.
The previous logic, introduced in
fa928d5cd1, crashed when visiting
"manage streams" in the case that the user didn't have permission to
create any streams at all.
We remove the more complex logic.
This isn't the right long term fix, because it means users who can
only create public streams just get a vague "Insufficient permission"
error when trying to create a private stream (and vice versa), but is
sufficient to fix the critical bug of "manage streams" failing to load.
Fixes#19895.
For text that already has the formatting that the user is
trying to apply, we undo the formatting. This gives a nice
experience of applying and removing the formatting from text
on the same button press.
Users wanted a feature where they could specify
which users can create public streams and which users can
create private streams.
This splits stream creation code into two parts,
public and private stream creation.
Fixes#17009.
Having tooltip `appendTo` to parent causes it to be trimmed by
the size of parent container if the parent doesn't have enough
size to include the tooltip. To fix this, we append tooltip
to `document.body`.
This change doesn't have proper test coverage and also doesn't work as
intended, since due to the structure of the function, we'll only
complain about invalid policy values for members in unlikely situations.
This reverts commit c69a968ffe.
This commit replaces 'allow_message_deleting' boolean setting
with an integer setting 'delete_own_message_policy'. We have a
separate dropdown now for deciding which user-roles can delete
messages sent by themselves and the time-limit setting droddown
is different.
This new setting has two options - everyone and admins only. Other
options including moderators will be added further.
We also remove the "Never" option from the original time-limit
dropdown, as admins are always allowed to delete message. This
never option resembled the case of only admins being allowed to
delete but this state is now resembled by setting the dropdown
to "admins only" and we also disable the time-limit dropdown in
this case as admins are allowed to delete irrespective of limit.
Note, this setting is only for deleting messages sent by the
deleting user themselves, and only admins are allowed to delete
messages sent by others as before.
We make zero invalid value for message_content_delete_limit_seconds and
for handling the case of "Allow to delete message any time", the API-level
value of message_content_delete_limit_seconds is "anytime" and "None"
as the DB-level value. We also use these values for message retention
setting, so it helps maintain consistency.
We should also hide the 'Other emails' heading in realm-level
defaults section when digest emails organization setting is
disabled because there is no other setting in this subsection
after we removed the enable_login_emails setting in d1732fb.
This commit does not remove the 'enable_login_emails' field from
RealmUserDefault table but it is just not used and cannot be
changed from UI or API similar to 'enable_marketing_emails' setting.
We incorrectly show message to reload in the savings indicator
for realm-level default of left_side_userlist setting. It should
be shown only for user-level setting since no change will take
place for the user changing realm-level default.
Previously, after sending a message from the full-sized compose-box,
the compose-box remained in expanded state covering the entire middle
part. Instead, it should return to the original state after the
message is sent.
There's a subtle race that would cause the "Scroll down to see your
message" compose notification to appear incorrectly, because the
full-size compose box occluded the entire message feed at the time the
message was locally echoed, even though it would no longer do so after
collapsing.
We address that by shrinking the compose box immediately before doing
a local echo, in addition to the primary code path in
`clear_compose_box`. Care is taken to ensure that we avoid shrinking
the compose box when sending a message that cannot be locally echoed
and gets an error from the server.
Tested on my Ubuntu development environment, by sending empty message,
valid message and slash commands. The compose-box only shrunk on
sending valid messages.
Fixes part of #19353.
This commit removes the existing default_twenty_four_hour_time field in
Realm table which was used to set the twenty_four_hour_time setting of
new user on joining and instead we now use the twenty_four_hour_time
field of RealmUserDefault table for the same.
With some tweaks by tabbott to clarify the documentation.
We refactor the code for user notification settings and realm-level
defaults of notification settings to pass a single object consisting
of container element, settings object, url and for_realm_settings
bool variable, to the functions, instead of passing them as separate
variables.
We refactor the code for user display settings and realm-level
defaults of display settings to pass a single object consisting
of container element, settings object, url and for_realm_settings
bool variable, to the functions, instead of passing them as
separate variables.
This commit changes the code to use container id in the
selector of the status element of presence_enabled setting
such that the correct element is selected because we will
add another element with same class in the realm-level
presence_enabled setting.
This is a prep-commit which simplifies changes when we add
web public streams condition here. Since our linter forces us
use switch then, we use it beforehand to simplify changes.
We pass user_settings object to all_notifications and
get_notifications_table_row_data such that we can use these
functions for realm-level settings also.
The container element and settings object are passed as
parameters to update_notification_sound_source. This change
is done so that we can use the same function for updating
the realm-level notification sound source.
This commit changes the click handler for playing
notification sound to use a variable instead of
directly using the element id such that we can
use the same code for realm-level settings also
by just setting the variable accordingly.
This commit creates a new module user_notification_settings.js
for user-level notification settings and the settings_notification.js
will be used as a common module for both user-level and realm-level
settings.
We also add parameters to the functions in settings_notifications.js
to pass container elem and settings object for the same purpose.
Notification settings uses '.on("change", "input, select")' to
handle changes to all types of input elements. But for realm-level
settings page, both display and notification settings have the
same container unlike the user-level settings, so we use inner
form element ".notification-settings-form" such that this handler
is for changing input of notification settings and not display
settings.
We set settings_object variable to user_settings object and then
use settings_object everywhere. This change will help us in using
the same code for realm-level settings also by setting the
appropriate value for settings_object.
We refactor the change_notification_setting function to have a url
parameter instead of directly using "/json/settings" directly.
This change will help us in using the same code for realm-level
settings also.
We add code for live updating notification batching period
setting. This commit also adds 'continue' statement in the
else-if condition of update_page function such that we do
not execute the code of checkbox settings for notification
sound and email batching period settings.
We also update the other_notification_settings list in
settings_config.ts to remove email_notifications_batching_period
and instead have email_notifications_batching_period_seconds
which is the correct name of setting used in backend and in
the events sent to the clients such that the live-update is
done correctly.
This commit removes the id from email notification batching period
setting dropdown and adds class such that we can use same template
for the realm-level settings as well. We also add 'setting_'
prefix as we do for other settings and modify the JS code accordingly.
We do not have any system user groups as of
now, but this commit is just a prep commit
to prevent any change in user-facing pages
to avoid confusion till this feature is
completed.
This change was initially made in 6117c38,
but it was reverted in 1543775a due to merge
conflicts with the typescript migration of
user_groups.js.
This commit adds code for live-updating the realm-level default
settings page which contains only display settings as of now.
This commit also adds realm_user_settings_defaults object to
zpage_params so that we can write the tests.
This commit adds a new module settings_defaults.js which calls
the functions in settings_display passing appropriate container
element and settings object as parameters.
We also add one more parameter for_realm_settings to some of the
functions in settings_dislay to differentiate between the user
and realm-level settings.
This commit refactors the code in settings_display.js
by modifying the functions to receive the container
element and settings object as parameter such that
we can use the same functions for realm-level
settings by passing appropriate container element
and settings object.
This change is needed as settings_display will be
used as a common module for user settings and realm
level settings.
We also rename the default_language_name variable in
settings_display to user_default_language_name as we
would add a separate variable for realm-level setting
in future.
We want the saving indicator to be sticky in the cases
where we ask user to reload after changing settings.
This used to work correctly before 9e08c6db93, as
'if(remove_after)' returned false if remove_after was
null, but the condition was changed in 9e08c6db93
to 'if(remove_after !== undefined)' and thus the
condition returned true when remove_after was null.
This commit change the remove_after value to undefined
for sticky cases.
The previous phrasing used incorrect terminology (E.g. "stream
members", not "stream subscribers", which is really confusing given
that we have a "member" role which is also relevant in this text).
This commit changes the settings_display.js code to reference the
user_setings object using settings_object variable. This will help
to easily use the same code for realm-level defaults where instead
of user_settings realm_user_settings_defaults object will be used.
Instead of passing user settings directly as user_settings
variable to the settings templates, we pass them using
settings_object variable.
This change is important so that we can use the exisiting
display_settings and notification_settings template for
realm-level default settings by passing realm-level
settings using settings_source variable.
This commit splites email notification settings in two
parts - one with heading "Email message notifications"
containing batchning period, message-content and
organization-name settings and the other with heading
"Other emails" containing login, digest and marketing
emails settings.
Fixes#19717.
This commit changes the behavior of how we show
animated emojis in the buddy list. We now show still
image of animated emoji and when hovered show the
animated emoji.
Fixes#19521
This commit displays a banner at the top for demo orgs letting the
user know that the org will be automatically deleted in the
specified number of days. The alerts are prioritized as follows:
- If there are <=7 days left till deletion, the alert is prioritized
above desktop notification queries and is colored red.
- If there are more then 7 days left till deletion, the alert is
prioritized below desktop notification queries.
This is a part of #19523.
We change the label of various organization settings
to specify moderators.
Labels for 'admins only', 'admins and moderators'
and 'nobody' are still same.
The updated labels are -
- 'Admins, moderators and full members'
- 'Admins, moderators and members'
- 'Admins, moderators, members and guests'
These options will be replaced by user groups in
future but this is an intermediate fix.
Fixes#19562.
We can use this format to display text in plural form or not based
on a number. This helps translators easily translate text and
users get a better formatted text.
We are renaming stream to stream_name in formatted draft object
just to be more explicit and be clear that we are storing that
stream name in this variable.
This commit changes snapshot_message to store stream_id for
drafts along with stream names. The stream_id field is
undefined if draft is for empty or invalid stream name.
After this change:
- If draft has a valid stream_id stored and it maps to
a stream, then we display the stream name from the
obtained stream object.
- If draft.stream_id is undefined or doesn't map to a stream,
then we display the name stored in draft.stream, which can
be invalid (no stream of this name existed ever), can be
empty and can also be name of a deactivated stream.
This change helps us to show correct stream-name for drafts
in case of renaming a stream.
Fixes#15155.
Along with the extraction, we do some simplifications of inserting
text in compose too. This same function can now be used in
compose formatting popover too.
We use "text-field-edit", which has good cross-browser `undo` /
`redo` support, to do the text replace for us instead of writing
that logic ourselves.
There is a bug when multiple message edit forms are opened at the
same time where undefined value of stream_id is sent to the server.
This happens because a global variable stream_widget is used to get
the id of stream selected in dropdown and value of stream_widget
variable keeps on changing when we open multiple message edit forms.
Thus, stream_widget can have the dropdown widget of already closed
edit form resulting in undefined value of stream id.
This commit changes the save_message_row_edit function to access
the dropdown element directly using message id instead of using
stream_widget.value() and thus we always use the correct dropdown
element to get the stream id.
We also move the stream_widget variable to be inside edit_message
function instead of being global variable for the module.
Fixes#19663.
Before this commit, the message or any draft is deleted as soon
as the compose box is closed. So, it removes that by removing
delete_active_drafts and instead this commit will add the deletion
process of drafts in reify_message_id that is called when a
message is successfully sent and received.
Now, see there are two types of messages, one that are locally
echoed and the second ones are that aren't locally echoed but
sent directly to server. This commit only saves the message in
draft if it is locally echoed as they are the only messages
that show message failed in message list. The non locally echoed
ones aren't remove from the compose box until they are
successfully sent. Now as the draft-id is stored in the message
data for locally echoed messages, as they are echoed from the
server, they are deleted using that draft-id.
This also adds node tests for echo reify_message_id for testing
this feature that this commit is adding.
Fixes#17697
Since, the filter button is replaced with a different button
after click, the `current_focus_elem` points at incorrect
element. `revive_current_focus` follows a good
method to locate the filter button, hence
we use it to correct the element `current_focus_elem` points at.
This commit fixes the live update of notification sound
setting dropdown. We already update the notification sound
source to play the correct sound after changing the setting.
This will be used to check if the narrow being requested by
spectator requires authentication without requesting the server.
Having this check locally, makes this process look snappy to
the user and doesn't result in 404s in the browser log.
For spectators, without sending any request to the server,
check locally if the hash requires authentication or which
shows a feature that requires authentication;
if it does, we show login_to_access modal to the user.
We will use this modal for any narrow / hash or other UI element that
requires an actual account to use, to provide something reasonable to
occur when a user clicks on those things.
We ideally will also add a /help/ page explaining these with an
example, but it seems appropriate to make the labels frame it around
what sort of emails we actually intend to send (newsletters with
updates about Zulip, not a "drip campaign").
We rename class of notification settings except checkboxes
by prefixing them with 'setting_' for clarity.
We do not change class of checkboxes because settings_checkbox
is used by other templates also and if we only change class
of those using notification_settings_checkboxes then live
update code will break and will need to add separate condition
for differentiating between which partial template is used.
This is a prep commit for adding UI for realm-level defaults
of user settings.
This commit renames notification-sound-audio element to
user-notification-sound-audio because we will be adding similar
element for realm-level default of user settings also.
We also change the child elements to have current id as their
class for the same reason.
This is a prep commit for adding UI for realm-level default
of user settings. We refactor the code to use classes
instead of ids such that we can use the common code for the
new settings.
This commit refactors change_display_setting function to accept
url as a paramter instead of directly using '/json/settings'
such that we can use the same function for realm-level settings.
This commit refactors change_display_setting function to
accept container element as a paramter and then determine
the correct status element from it. This is the prep commit
for adding realm-level default settings UI.
We add a prefix to id of default_language_modal.hbs
such that we can use the same code for user settings
and realm-level settings.
We also add a class "default_language_modal" to the
modal div to avoid duplicate css.
This is a prep commit for adding UI for realm-level
default of user settings. We refactor the code to use
classes instead of ids such that we can use the common
code for the new settings.
We were using incorrect element as we changed the input from
select to radio buttons in 439bc2920a.
This code was originally added in 6cb03ea78e (though in a
different file) and was removed from sometime and then added
again in e7311cdf5d. But in the meantime the input was
changed to radio, which resulted in incorrect code.
We were using emojiset_spinner element in destroy_indicator, but
there is no such element and emoji-settings-status element is used
in make_indicator, so destroy_indicator should also use the same
element.
This was added originally in 6cb03ea78e (though in a different
file) and was removed for sometime and then readded in e7311cdf5d.
But between these two commits, the setting was changed to be a
radio element and thus the spinner elements were also changed.
On mobile, when the sidebar is toggled, the following three issues
are encountered:
- When none of the sidebar menus are expanded, the sidebar has no
scrollbar, which is expected. But if you scroll, the background
content scrolls, which is a bug.
- When some of the sidebar menus are expanded such that the content
overflows and is "scrollable", once you get to the end of the
sidebar content, the background content keeps scrolling in a weird
way.
- If the mobile screen is wide enough, if you scroll the sidebar
content, it scrolls as expected. But if you move the pointer to
the side of the background content that is still visible, you
can scroll the background content even though it should be fixed.
This commit fixes all of the above issues.
This commit updates both the stream-level and realm-level message
retention setting to use 'unlimited' instead of 'forever' to set
message retention setting to "retain messages forever".
Earlier copy to the link present above the composebox (which
appears when a message is sent outside the current narrow), it
used to provide the link to the home page which is fixed now.
Now, copy to the link gives the link of the narrow where message
is sent.
When one resizes the window and tries to switch to the vertical
sidebar menu, the CSS transition flickers in and out. This is
less than ideal.
This commit implements a solution to this problem. The solution is
inspired by the following helpful article:
https://ishadeed.com/article/layout-flickering/
We found a bug in our top-level nav on certain Samsung phones where
tapping on the dropdowns didn't do anything. As it turns out, trying
to figure out whether touch is enabled on a device using CSS media
queries such as `(hover: none)` is very tricky business! While it
may work on some devices, it may break spectacularly on others.
This commit presents a potential solution to this. Media queries
about widths are a lot more device-independent than hover media
queries. So, this is what we do now:
- We fire click events regardless of whether the top-level nav is in
the vertical sidebar orientation or the horizontal orientation.
Since we are no longer dependent on hover media queries for click
events, this fixes the mobile bug mentioned above because taps or
clicks will always work regardless of device.
- The mouseover/mouseout events are only fired when the header is
in the horizontal orientation and the primary input mechanism
supports hovering over elements. This allows us to support hovering
over menus on desktop. However, since mouseout/mouseover events are
irrelevant for mobile, we are fine on that front since clicks/taps
will still work.
NOTE that the above approach also takes care of weird edge cases where
you have a horizontal orientation on a device such as the iPad Pro that
is flipped in the horizontal position. Since clicks work anyway, these
edge cases are largely taken care of.
Commit 9049fb3bd4 (#19176) broke these
by changing {page_params} to {...page_params}. We could change it
back, but it’s better to be explicit about which items we use from
page_params.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit makes the compose.update_fade as an exported
function and adds the listener to update the fade manually
as deleting the pill was not calling this functions.
Fixes#18865
We want to avoid passing opaque messages around to
low-level functions that only need certain fields.
This sets us up to handle mark-as-unread messages
that aren't necessarily full messages.
We only care about ~8 fields from a Zulip message
when we're tracking unread counts. For example, we
don't care about the "content" field. This also sets
us up to pull up the "display_recipient" logic in an
upcoming commit.
This reverts commit 1965584eec.
This syntax has a bad interaction with table syntax and needs to be
rethought.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
'corporate_enabled', 'realm_push_notifications_enabled'
and 'realm_digest_emails_enabled' settings are part of
page_params and not user_settings.
This was introduced in 998d710275.
The reason for this was we were not loading extra
parameters while storing the status emoji object.
For this we also need the emoji module to be initialized
early because the right sidebar would need it to display
the status emoji.
YashRE42
Ganesh Pawar
akshatdalton
Aman Agrawal
Sahil Batra
Tim Abbott
juliaBichler01
Anders Kaseorg
Julia Bichler
Johan Ehinger
Manan Rathi
Lauryn Menard
Wesley Aptekar-Cassels
isakhagg
Vishnu KS
Andrew McAfee
Kevin Scott
Jonny Tran
rht
Gaurav Pandey
Pradyumna Sinha
Dinesh
AnshVM
sahil839
Priyansh Garg
Priyank Patel
Riken Shah
Eeshan Garg
Abhijeet Prasad Bodas
Palash Raghuwanshi
Signior-X
Priyam Seth
Steve Howell