We store the relevant data to hide/show a topic in the row itself,
and use jquery to hide/show it on filter change.
This also fixes search breaking the set filters.
We reuse the existing logic for displaying and updating stream color
from the stream left sidebar.
Tests fixtures were extracted and updated for this commit.
* Add action to mute topics.
* We don't need to store muted data per topic as previously planned.
* Moved launch topic test to the top so that they run on non-modified
data.
* Show an empty overlay of recent topics.
* Register click event to open recent topics.
* Launch recent topics on "t" keypress.
This is based on the draft overlay.
We now trigger realm day/night logo upload by clicking on realm
day/night logo element itself rather than having a big upload button
and to match our user avatar UI. Added new spinner over the logo
element itself to show while uploading realm logo for both day and
night logos.
Change user avatar spinner implementation to match
realm icon spinner implementation and have common css class
since similar implementation between similar widgets may help
in future deduplication.
Option to disable breadcrumb messages were given in both message edit
form and topic edit stream popover.
User now has the option to select which stream to send the notification
of stream edit of a topic via checkboxes in the UI.
This commit changes the compose_invite_users template to use
data-user-id as property intead of data-useremail.
This is changed to maintain consistency with other parts of the
code where user_ids are used for referring to users.
This also helps in removing some of the checks for the case of
undefined emails.
This isn't a complete fix, but we move the widget's popup to be
on/below the button to open the widget. We also move the bot owner
field to be on the top of the page so that we can see most of the
widget before it is clipped by the parent overlay.
We have discussed some approaches for a permanent fix on:
https://chat.zulip.org/#narrow/stream/321-s/topic/DropdownListWidget/near/894674
This reimplements our Zoom video call integration to use an OAuth
application. In addition to providing a cleaner setup experience,
especially on zulipchat.com where the server administrators can have
done the app registration already, it also fixes the limitation of the
previous integration that it could only have one call active at a time
when set up with typical Zoom API keys.
Fixes#11672.
Co-authored-by: Marco Burstein <marco@marco.how>
Co-authored-by: Tim Abbott <tabbott@zulipchat.com>
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
When migrating to dropdown list widget, we incorrectly used the same label
for both realm_notifications_stream and realm_signup_notifications_stream.
This was introduced in b580baf682.
This commit changes the stream settings UI for adding subscribers to
use our standard user pills in the input box, rather than just
plain-text email addresses. This is important progress towards
removing display email addresses from the Zulip UI.
It also allows subscribing multiple users at the same time, which is a
nice improvement.
This commit changes the update user API endpoint to accept role
as parameter instead of the bool parameters is_guest and is_admin.
User role dropdown in user info modal is also modified to use
"dropdown_options_widget".
Modified by tabbott to document the API change.
We remove the "GROUP PMs" section that used
to be in the lower right sidebar.
Most of this is straightforward code removal.
A couple quick notes:
- The message fetching code now just
calls `huddle_data.process_loaded_messages`,
which we still need for search suggestions.
We removed `activity.process_loaded_messages`.
- The `huddle_data.process_loaded_messages`
function no longer needs to return `need_resize`.
- In `resize.js` we now just calculate
`res.buddy_list_wrapper_max_height` directly
from `usable_height`.
Trigger realm icon upload by clicking on realm icon element itself
rather than having a big upload button and to match our user avatar UI.
Added new spinner over the icon element itself to show while
uploading realm icon.
This commit has us piggy back the conditional for narrow_description
off of the conditional for sub_count, the reason for this approach is
that "narrow_description" needs to handle four unique cases:
- The stream exists and has a description.
- The stream exists and does not have a description.
- The stream does not exist and we must render appropriate text.
- We are not in a stream narrow and the span should not be rendered.
By piggy backing off of sub count we can get the first and last case,
with the inner conditional (on rendered_narrow_description) handling
the second case and the tab_bar.js passing appropriate values to the
template to handle the third case.
This unfortunately makes the template more brittle such that breakage
of the subcount can cause breakage (non rendering) of the description
as well.
This commit replaces fa-file-text-o with fa-file-code-o which is a
better signal for the "view source" action. It also deletes a single
line comment that had suggested the change once we moved into font
awesome 4, which Aditya Bansal <adi.bansal241996@gmail.com> helped
out in doing, first via
91962aa6ab and most recently via
75ae94e459 with several commits in
between.
Rather than showing the "Saving" widget beside stream name, it's more
intuitive to have it in personal settings section because it's the only
section which uses `settings_ui.do_settings_change` function and we follow
having a separate "Saving" widget for each section everywhere.
We wrap the [reset] anchor tag in a button so that we can set 'disabled'
attribute on it. We change the styles to hide the [reset] button and the
pencil icon when the widget is disabled.
We also need to call `e.preventDefault()` in the event handler since now
the anchor tag behaves as a button.
This commit allows non admins to set stream post policy while creating
streams.
Restriction was there to prevent user from creating a stream in which
the user cannot post himself but this will be taken care of with
stream admin feature.
* Stream bar color logic is borrwoed from compose stream bar.
* Use flex containers to align elements and automatically set their
height to be same, them automatically filling the stream color bar
height to be the height of the select box.
* Use flex-wrap to wrap the propagate selector when out of space.
* To make sure stream select box and stream color box are closest possible,
select box has been moved under stream color box.
A separate outer span with the title text needs to be added because:
(1) The default bootstrap popover behavior takes the title as the
popover title, if provided.
(2) We need to avoid having the title area be too big in the
me_message template.
Fixes#12769.
Co-Author-By: Vaibhav <vrongmeal@gmail.com>
* This feature is currently only visible to admins.
* Locally echoed messages are also updated.
* Add UI for editing stream if user is admin.
* Show propagate mode selector if either stream or topic changed.
We use this new widget in bot settings panels
(personal and org). It lets you re-assign a
bot to a new human user.
Ideally we can improve this code to use
our existing list widgets to make it more
performant for realms with lots of users.
We were still sorting them with the generic alphabetic
sort due to the markup, despite passing in a custom
sort to the `list_render` class.
The `sort_email` helper often behaves like a generic
alphabetic sort, so this fix is mostly just making the code
do what it claims to do (and it's consistent with how
we already sort active users).
The nuance with emails is whether we display real
emails or system-generated emails.
We now no longer have to remember that
`is_guest` is on `user` but `is_current_user`
is in `..`.
And we no longer have to remember that
`full_name` is on `user` but `display_email`
is in `..`.
When editing a bot, there are only two fields
that are similar to humans--full name and
email--which are trivial.
Before this commit we used a single codepath
to build the human form and the bot form.
Now we have two simple codepaths.
The tricky nature of the code had already led
to ugly things for the bot codepath that
fortunately weren't user facing, but which
were distracting:
- For bots we would needlessly set things
like is_admin, is_guest in the template
data.
- For bots we would needlessly try to update
custom profile fields.
The code that differs between bots and humans
is nontrivial, and the code was both hard to read
and hard to improve:
- Humans don't have bot owners.
- Bots don't have custom profile fields.
The bot-owner code is nontrivial for performance
reasons. In a big realm there are tens of thousands
of potential bot owners. We avoid the most egregious
performance problems (i.e we don't have multiple
copies of the dropdown), but we may still want
to refine that (at least adding a spinner).
The custom-profile-fields code is nontrivial due
to the dynamic nature of custom profile fields,
which can bring in specialized widgets like
pill fields.
Now each form corresponds to a single endpoint:
* human -> /json/users
* bot -> /json/bots
Before we had a lot of conditional logic in
the template, the code to build to views, and
the code to submit the data. Now everything is
much flatter.
The human code is still a bit messy (more work
coming on that), but the bot code is fairly
pristine. All three components of the bot code
fit on a page, and there are no conditionals:
- admin_bot_form.hbs
- open_bot_form
- handle_bot_form
We may want to grow out the bot code a bit
to allow admins to do more things, such as
adding services, and this will be easier now.
It would also be easier for us now to share
widgets with the per-user bot settings.
Note that the form for editing human data will
continue to be invoked from two panels:
- Users
- Deactivated users
There are some minor differences between
users and deactivated users, but the shape of
the data is the same for both, so that's still
all one codepath.
We eliminate `reset_edit_user` here, since
it was never used.
One nice thing about these forms was that they
had very little custom CSS attached to them
(at form-level specificity), and it turned out
all the custom CSS was for the human-specific
form.
Since production testing of `message_retention_days` is finished, we can
enable this feature in the organization settings page. We already had this
setting in frontend but it was bit rotten and not rendered in templates.
Here we replaced our past text-input based setting with a
dropdown-with-text-input setting approach which is more consistent with our
existing UI.
Along with frontend changes, we also incorporated a backend change to
handle making retention period forever. This change introduces a new
convertor `to_positive_or_allowed_int` which only allows positive integers
and an allowed value for settings like `message_retention_days` which can
be a positive integer or has the value `Realm.RETAIN_MESSAGE_FOREVER` when
we change the setting to retain message forever.
This change made `to_not_negative_int_or_none` redundant so removed it as
well.
Fixes: #14854
It's a preliminary step to enable message_retention_setting in org settings
UI, which is a non-limited plan only feature. So we require a page_param
property that tells us the limited-plan state of the Zulip realm.
This completes the implementation of support for moving a topic to
another stream by adding a basic UI for it.
Fixes#6427, which was previously the most-upvoted issue request in
Zulip.
There are likely to be a bunch of follow-up UI improvements on top of
this change to fully flesh out the feature.
For privacy-minded folks who don't want to leak the
information of whether they're online, this adds an
option to disable sending presence updates to other
users.
The new settings lies in the "Other notification
settings" section of the "Notification settings"
page, under a "Presence" subheading.
Closes#14798.
Member of the org can able see list of invitations sent by him/her.
given permission for the member to revoke and resend the invitations
sent by him/her and added tests for test member can revoke and resend
the invitations only sent by him/her.
Fixes#14007.
Prior to this change, there were reports of 500s in
production due to `export.extra_data` being a
Nonetype. This was reproducible using the s3
backend in development when a row was created in
the `RealmAuditLog` table, but the export failed in
the `DeferredWorker`. This left an entry lying
about that was never updated with an `extra_data`
field.
To fix this, we catch any exceptions in the
`DeferredWorker`, and then update `extra_data` to
encode the failure. We also fix the fact that we
never updated the export UI table with pending exports.
These changes also negated the use for the somewhat
hacky `clear_success_banner` logic.
This removes the weird edit-bot sidebar, replacing it with a modal,
matching our edit-user widget (and various similar ones).
Fixes#13644 by removing the buggy code.
We now prevent these variations:
* <hr/>
* <hr />
* <br/>
* <br />
We could enforce similar consistency for other void
tags, if we wished, but these two are particularly
prevalent.
We use this elsewhere in the frontend codebase with the parents
included in the i18n, so we should stick with that.
Fixes `check-capitalization` failing as a side effect.
1. Replaced the deactivate and reactivate buttons with icons.
2. Added (you) near the current user name to denote his/her account in
the entire user list.
Tweaked by tabbott to reuse the (you) formatting from the right
sidebar here for readability and consistency.
Fixes#6313.
This commit removes most of the duplicate logic for the stream selection
dropdowns for the settings: `realm_signup_notifications_stream_id` and
`realm_notifications_stream_id`.
We also make minot changes to DropdownListWidget to accomodate the stream
rendering of the format: `#stream_name`.
We finally switch to using stream_ids instead of stream_name everywhere
which makes reading data from page_params simpler.
Message_edit.js had a bug where if the inline topic_edit failed, it
would not show an error because it attempted to make a look up for
the message_id as though it were a message row edit, which would not
work. That was changed in a refactor, which made it apparent that
there was no error being rendered at all. This commit corrects it by
rendering the error, it also adds some styling to ensure the error
message is displayed inline and it makes a change to the template so
the error is rendered before the spinner.
The set_up_muted_topics_ui and templates have been
refactored to use list_render.
This is done to support filtering and sorting of
the muted stream topics.
This also includes the addition of a new Date muted header.
Increase brightness on hover effect for "X" icon in upload
avatar area is added and change the text to "delete profile picture"
while hovering over "X" icon in the avatar upload area.
Previously, in narrow viewports, the "filter"
option would disappear, which was very confusing.
This commit moves the filter streams input to the
next line, making it visible at all viewport widths.
@showell modified the commit message and got Casper
tests passing.
Fixes#12898.
Before this change, on clicking a checkbox to toggle subscription to a
stream no UI feedback was shown and users could toggle the checkbox
multiple times to send multiple requests causing bugs. This commit
initializes a spinner on clicking the checkbox, to provide a UI feedback
to the user. This commit also disables the checkbox once a request for
subscription has been sent and re-enables the checkbox only after a
response.
This change has been accomplished by introducing a div to display the
spinner in subscription.hbs. The corresponding styles for the spinner
have been added in subscriptions.scss. The ajaxSubscribe &
ajaxUnsubscribe functions in subs.js have been updated to show & hide
the spinners for the time the request is in process. An additional
parameter, the concerned stream object is passed to these functions(
through the sub_or_unsub function) to get the location where the spinner
is to be displayed. Finally, the checkbox click handler is updated to
support these changes.
The testing for this has been done by adding a wait of 2 secs in
actions.py for the response. This gives sufficient time to test the
working manually. Also, for error cases an error has been sent from
action.py and the behaviour has been manually observed.
Fixes#14481.
Now, the system uses word='' and an editing=True for rendering an
form for addition of alert words. This is a very vulnerable
way to implement said feature and this commit fixes that.
The addition form has been moved to alert_word_settings.hbs
thereby rendering it only once but always. Now, we do not have
to manually add an empty word and editing for the form to be
rendered.
As part of refactoring, the editing parameter has also been
removed as there is no purpose left.
This updates the logged-in top navbar to display the stream/message
name, number of users, and description. It also replaces the search
bar with a search icon that expands into a full-width search bar.
Co-authored-by: Max Nussenbaum <max@maxnuss.com>
Fixes: #164.
Fixes: #5198.
Removed the condition in the settings_save_discard_widget page which
allowed the page to be rendered only for admins. This change was
important so that this widget can be used on the user settings page
for displaying confirmation messages.
For non-admins, trying to access admin-only settings, behaviour remains
the same as before as these settings are disabled for non-admins.
Added UI support for uploding the new profile picture by
clicking on the avatar rather than a button.
Added new spinner for loading indication while uploading
a new avatar over the avatar area.
Fixes#10255
We want to avoid creating jQuery objects that just
get turned right back into strings by the list
widget, so we now have our template just include
`last_active_date` instead of kludging it in
after the fact, and we return the template
string in `modifier` rather than wrapping it.
To deal with plain HTML we switch to using
`render_now`.
Calling `render_now` leads to a more simple
codepath than `render_date`, beyond just dealing
with text.
The `render_date` function has special-case logic
that only applies to our time dividers in our
message view, which is why we were passing the
strange `undefined` parameter to it before this
fix.
The `render_date` function was also putting
the dates into `update_list` for once-a-day
updates, which is overkill for an admin screen.
We don't use this logic for drafts or attachments
either. I'm not sure how well tested that logic
is, and it's prone to slow leaks.
This commit sets us up to simplify the list
widget not to have bit-rot-prone code related
to jQuery objects.
We already know which list widget a `<th>`
tag is associated with when we set up the
event handler, so it's silly to read data
from the DOM to find that widget again
when the handler runs.
This commit eliminates a whole class of possible
errors and busy work.
We extract a general purpose widget to create dropdown lists with
search. This widget is used for default code block language, but can
be easily extended to cover notifications_stream and similar options.
The current usage is:
```js
const widget = DropdownListWidget({
setting_name: 'realm_alpha_beta',
data: [{name: 'hello', value: 'world'}, {...}, ...],
subsection: 'msg-editing',
default_text: 'Nothing is selected',
});
```
and
```handlebars
{{> dropdown_list_widget
setting_name="realm_alpha_beta"
list_placeholder=(t 'Filter the data')
reset_button_text=(t '[Unset]')
label=admin_settings_label.realm_alpha_beta }}
```
This can further be refined by shifting more variables from handlebars
to javascript in the future.
Before this fix, we'd get a traceback if you looked
at invites in the settings (and if one of them was
a multi-user invite link). This commit fixes
that problem by adding a custom sort.
We also rename the "Email" column to "Invitee",
since it's often the case the invitee isn't an
actually an email, but it's instead a multi-use
link.
Note that the invites UI only works the first time you
enter settings. Many of the controls break the second
time you enter it. You can't sort by column header
or use the text filter.
I'll fix that in a subsequent commit.
Giving these functions a name and moving them to
the top-level scope has a couple tactical advantages:
- names show in tracebacks
- code is less indented
- setup code is less cluttered
- will be easier to add unit tests
- will make some upcoming diffs nicer
These are technically more `compare_foo` than `sort_foo`,
but we already had a naming convention that was sort of
in place.
Option is added to video_chat_provider settings for disabling
video calls.
Video call icon is hidden in two cases-
1. video_chat_provider is set to disabled.
2. video_chat_provider is set to Jitsi and settings.JITSI_SERVER_URL
is none.
Relevant tests are added and modified.
Fixes#14483
This adds a new realm setting: default_code_block_language.
This PR also adds a new widget to specify a language, which
behaves somewhat differently from other widgets of the same
kind; instead of exposing methods to the whole module, we
just create a single IIFE that handles all the interactions
with the DOM for the widget.
We also move the code for remapping languages to format_code
function since we want to preserve the original language to
decide if we override it using default_code_clock_language.
Fixes#14404.
This adds a preview of the uploaded emoji image while uploading custom
emoji right below the upload form.
Modified upload_widget.build_widget() to take in the preview
span text and image. In case a parameter isn't passed
for preview text, it defaults to null and the snippets in
build_widgets() related to preview don't run.
Fixes#9229.
Styling tweaked by tabbott.
The todo_widget was using the using a counter to store the key value of
every task. This would cause assiging multiple tasks the same key value
in a race condition. To avoid this we make "sender_id" a part of the key
along with the counter.
Also the `key` now not being a integer value, we can't use it to find the
index of the task using it. Thus, a function is made that will find the
index of task whose key is sent by the user to strike.
Users are unable to modify organization's profile picture, but
disabled buttons for the same are being shown to the user on the
organization profile settings page. This commit removes those
buttons. The file realm-logo-widget.hbs renders those buttons only
if the user is an admin and realm_logo.js has been updated to allow
operations(like click) on the buttons only to admins.
Users are unable to modify organization's logos, but disabled
buttons for the same are being shown to the user on the organization
settings page. This commit removes those buttons. The file
realm-logo-widget.hbs renders those buttons only if the user is an
admin and realm_logo.js has been updated to allow operations
(like click) on the buttons only to admins.
Next to the checkbox of "Show previews of linked websites" added a
documentation link in order to help users not confuse its function.
To do this:
- Added the field 'help' which is the link for the documentation in
organization_settings_admin.hbs
- Added the if statement in settings_checkbox.hbs to check if the above
field exists.In case it exists, a help icon which leads to the documentation
link appears.
The specific field was added in order to be able to add a help link in
other settings as well.
Co-authored-by: Katerina Perikou
<44238834+kPerikou@users.noreply.github.com>
Fixes: #13450
Fixes the problem of recipient_bar_icons being too close to each
other. To improve spacing between them, classes are added, namely
recipient_bar_icon_link (for link icon) and reciepient_bar_icon
(for other icons). CSS for spacing these classes correctly, using
padding-left and padding-right, has been added zulip.scss
Manually tested for cases with single and multiple links present.
Fixes#14364.
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.
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>
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.
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.
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.
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.
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 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.
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.
The user can pass description along with the task name by splitting the input string with hyphen.
Eg: Task Title - Task Description
todo_list: Add index numbers to task.
Original email address is shown to admin users in subscriber list when
email_address_visibilty is set to "Admins only" by passing delivery_email
at required places. Email address are not shown to non-admin users when
visibility is set to "Admins only".
Tweaked by tabbott to fix a few bugs and dead code.
Fixes a part of #13541.
User IDs are more robust than email addresses as they don't change
with time, and also don't have complications with
different email_address_visibility settings.
The bug was in complex `if` condition, which should mean that users should
be allowed to create a User group only when they are either admin or user
group creation policy is set to everyone.
Fixes: #13909.
This commit includes a new `stream_post_policy` setting,
by replacing the `is_announcement_only` field from the Stream model,
which is done by mirroring the structure of the existing
`create_stream_policy`.
It includes the necessary schema and database migrations to migrate
the is_announcement_only boolean field to stream_post_policy,
a smallPositiveInteger field similar to many other settings.
This change is done to allow organization administrators to restrict
new members from creating and posting to a stream. However, this does
not affect admins who are new members.
With many tweaks by tabbott to documentation under /help, etc.
Fixes#13616.
Earlier, on narrowing the window to some particular sizes,
long stream names used to overlap with the subscribe and view stream
buttons.
The issue was resolved by cutting the stream name short and putting
ellipses at the end. A title was provided to the stream name div so that
the entire stream name would be visible on hovering over it.
Fixes: #13139
Edited the warning to clearly state that most members/most stream members
will be notified on using wildcard mentions, along with the specific
mention (e.g. @ALL, @everyone and @stream).
Did a separate check for all wildcard mentions in util.js and stored the
corresponding mention in wildcard_mention inside compose.js.
Fixes: #13636
This change is in series of de-duplication of code in "Other permission"
section for various dropdowns.
Here rather than using "by_anyone" and "disabled" for the `value` attribute
of options, we use actual numeric values. As a result, we don't need to
manually handle to extract the data to be sent to the backend on saving.
This change is in series of de-duplication of code in "Other permission"
section for various dropdowns.
Here rather than using "by_admins_only" and "by_admins_only" for `value`
attribute of options, we use actual numeric values. This helps in
de-duplicating lot of code which is vulnerable to bugs.
For few settings like `waiting_period_threshold` it makes sense to have the
"value" attribute of option to have a value other than the actual setting
value because multiple settings are depending upon this dropdown, so
handling them in JS code makes more sense. But for many settings (which has
integer values), we have followed a wrong trend over the time of
representing every new dropdown with human-readable values and manually
handling them in JS Code, where it makes more sense to use actual setting
value. The result of which is code has become less concise, sensible and
less likely to be mistaken.
This is a preliminary commit for upcoming change where we will use
"bot_creation_policy_values" like approach for many other settings where
dropdown represents the only single setting of integer type.
We now use vdom-ish techniques to track the
list items for the pm list. When we go to update
the list, we only re-render nodes whose data
has changed, with two exceptions:
- Obviously, the first time we do a full render.
- If the keys for the items have changed (i.e.
a new node has come in or the order has changed),
we just re-render the whole list.
If the keys are the same since the last re-render, we
only re-render individual items if their data has
changed.
Most of the new code is in these two modules:
- pm_list_dom.js
- vdom.js
We remove all of the code in pm_list.js that is
related to updating DOM with unread counts.
For presence updates, we are now *never*
re-rendering the whole list, since presence
updates only change individual line items and
don't affect the keys. Instead, we just update
any changed elements in place.
The main thing that makes this all work is the
`update` method in `vdom`, which is totally generic
and essentially does a few simple jobs:
- detect if keys are different
- just render the whole ul as needed
- for items that change, do the appropriate
jQuery to update the item in place
Note that this code seems to play nice with simplebar.
Also, this code continues to use templates to render
the individual list items.
FWIW this code isn't radically different than list_render,
but it's got some key differences:
- There are fewer bells and whistles in this code.
Some of the stuff that list_render does is overkill
for the PM list.
- This code detects data changes.
Note that the vdom scheme is agnostic about templates;
it simply requires the child nodes to provide a render
method. (This is similar to list_render, which is also
technically agnostic about rendering, but which also
does use templates in most cases.)
These fixes are somewhat related to #13605, but we
haven't gotten a solid repro on that issue, and
the scrolling issues there may be orthogonal to the
redraws. But having fewer moving parts here should
help, and we won't get the rug pulled out from under
us on every presence update.
There are two possible extensions to this that are
somewhat overlapping in nature, but can be done
one a time.
* We can do a deeper vdom approach here that
gets us away from templates, and just have
nodes write to an AST. I have this on another
branch, but it might be overkill.
* We can avoid some redraws by detecting where
keys are moving up and down. I'm not completely
sure we need it for the PM list.
If this gets merged, we may want to try similar
things for the stream list, which also does a fairly
complicated mixture of big-hammer re-renders and
surgical updates-in-place (with custom code).
BTW we have 100% line coverage for vdom.js.
We mostly needed this for Casper tests, and that
usage was eliminated in the prior commit.
There was also some strange defensive code from
ecc42bc9f8 that
is really ancient and which I am eliminating:
const email = row.attr("data-email");
if ($("#deactivation_user_modal .email").html() !== email) {
blueslip.error("User deactivation canceled due to non-matching fields.");
ui_report.message(i18n.t("Deactivation encountered an error. Please reload and try again."),
$("#home-error"), 'alert-error');
}
If the code was there to protect against live
updates for email changes, then we no longer
have to worry about that, since we use user_ids
now as keys.
Or it might have to do with some ancient bug
where you could pop open two modals at once
or something. You can actually change users while
the modal is open (which is kinda strange, but ok),
and it works fine.
When testing this, I ran into the glitch that we
don't open redraw the Deactivated Users panel after
going into the User panel and deactivating a user.
Tim Abbott
Aman Agrawal
Aman Agrawal
jagansivam28
Ryan Rehman
sahil839
Rohitt Vashishtha
Anders Kaseorg
Steve Howell
YashRE42
Siddharth Varshney
Pragati Agrawal
Gillian Pantaleon
Steve Howell
Tim Abbott
Hashir Sarwar
Wyatt Hoodes
jiviteshjain
Pranav
Abhishek-Balaji
majordwarf
Vaibhav Raj Singh
MariaGkoulta
Puneeth Chaganti