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.
Previously, if an admin created a private stream with shared history
or a private stream with protected history, they would see the general
tab for that stream in the right side of the subscriptions_overlay as
expected, but, they would not see the pencil button to change stream
privacy unless they clicked a different stream and came back.
The reason for this has to do with how we receive events when we
create a sub. We first get an event with type "stream" and op
"create", we then get an event with type "subscription" and op "add"
ie we create the stream and then sub ourselves to it. Now, we render
`stream_settings.hbs` while handling the "stream create" event, at
this time we pass `can_change_stream_permissions` as false since
`(!sub.invite_only || sub.subscribed)` is false because we're not
subscribed yet. This causes us to skip the insertion of the
"change-stream-privacy" block which is a problem because when we're
handling the "subscription add" event, we run
`stream_ui_updates.update_change_stream_privacy_settings(sub)` which
tries to show the element via `.show()` but can't since the element
does not exist and as a result the admin user does not see the pencil
edit button.
This commit fixes the above bug by changing the template such that we
always insert the button, but conditionally apply
`style="display:none"`.
Fixes: #20345.
I rewrote most of tools/lib/pretty-printer.py, which
was fairly easy due to being able to crib some
important details from the previous implementation.
The main motivation for the rewrite was that we weren't
handling else/elif blocks correctly, and it was difficult
to modify the previous code. The else/elif shortcomings
were somewhat historical in nature--the original parser
didn't recognize them (since they weren't in any Zulip
templates at the time), and then the pretty printer was
mostly able to hack around that due to the "nudge"
strategy. Eventually the nudge strategy became too
brittle.
The "nudge" strategy was that we would mostly trust
the existing templates, and we would just nudge over
some lines in cases of obviously faulty indentation.
Now we are bit more opinionated and rigorous, and
we basically set the indentation explicitly for any
line that is not in a code/script block. This leads
to this diff touching several templates for mostly
minor fix-ups.
We aren't completely opinionated, as we respect the
author's line wrapping decisions in many cases, and
we also allow authors not to indent blocks within
the template language's block constructs.
In cases where an opening tag is so long that we stretch
it to 2+ lines of code, we should try to use block-style
formatting in the template code.
Unfortunately, we have lots of legacy code that violates
this concept, so this is a timid fix.
There are also legit use cases like textarea where we
probably need to keep the ugly template syntax for things
to render properly.
This fixes various visual glitches that resulted from reusing
components and overriding key elements of them. The specific logical
changes are as follows:
* Delete custom checkbox positioning for stream settings; we now just
use the common app_components.css code.
* Remove custom subscription-control-label styling; just use settings
defaults.
* Copy the h3/h4 styling from settings.css. Ideally we'll deduplicate
this in further cleanup.
* Add the inline property to stream_settings_checkbox elements, to
reduce variable with settings_checkbox.hbs.
* Place every individual input inside an input-group, so that we can
use the standard settings.css styling.
Previously, the stream_edit modal relied on the new-style class to set
the margin-bottom value for stream-message-retention-days-input to 0,
in order to override the value set by bootstrap. The class new-style
is unhelpful because of its generic name, and in addition, time has /
will eroded away the significance of its name.
Hence, this commit adds the necessary rules to subscriptions.css and
removes the new-style class.
In order to make this change, this commit adds a block to
`subscriptions.css` with the selector `#stream_privacy_modal
.stream-message-retention-days-input input[type="text"]` one important
rule that this adds is `height: inherit;`. Adding this rule solves a
minor UI glitch where selecting "retain N days after posting" would
cause the save and cancel buttons to jump down by a pixel or so.
Fixes: #20222.
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.
In d62e44fcba we migrate to using
micromodal for this pop up (via dialog_widget), as a result the
.modal-body style no longer applied as that class is not used.
In 55adf88e667da02284f0a6ffb6bcfdf73b5427cb we remove the grey-box
class from the stream_types template, hence even if the above wasn't
true, this rule would still not apply.
This commit thus removes this rule.
Previously, the presence of the styles applied by grey-box caused a
visual disparity between the stream settings overlay and the
personal/organization settings overlay, hence, this commit removes
this class.
Previously, the presence of the styles applied by grey-box caused a
visual disparity between this modal and similar settings in our
organisation settings view, hence, this commit removes this class.
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.
YashRE42
Aman Agrawal
Sahil Batra
juliaBichler01
Tim Abbott
Steve Howell
Ganesh Pawar
Anders Kaseorg