This check caused us to only run the code inside that block if the
message mentioned the current user (since that's when the `mention`
class is added to the main message row).
While this was a useful performance optimization, it probably was a
small one, not worth it for the correctness cost.
This adds a new realm_logo field, which is a horizontal-format logo to
be displayed in the top-left corner of the webapp, and any other
places where we might want a wide-format branding of the organization.
Tweaked significantly by tabbott to rebase, fix styling, etc.
Fixing the styling of this feature's loading indicator caused me to
notice the loading indicator for the realm_icon feature was also ugly,
so I fixed that too.
Fixes#7995.
Logic for checking if the last message in the current table is visible was
already written in message_viewport.js; Code in notifications.js is changed
to reduce redundancy.
We are trying to carve room for a more specific
"user_status" concept, which refers to statuses
that users specifically set, like "I'm away".
So we call this function "update_presence_info",
which reflects that it's more about actual
"presence"--i.e. the user really is present
in the browser, even though the actual human
may not want to be disturbed.
The current user gets excluded from all non-empty
searches, even ones that match the user, since
it can look funny when the user's at the top of a
search, and you'd never need to search for yourself
(again, since the current user is at the top of
the buddy list).
Apparently, we didn't have one of these, and thus had a moderate
number of generally very old violations in the codebase. Fix this and
clear the ones that exist..
We move all of its logic into settings_sections.
Note that this is slightly more than a refactor.
We are slightly more aggressive about resetting
sections. For example, if you go into Settings,
then exit the overlay, then go into Manage
Organization, we will now reset sections for both
groups.
We now rely on set_up() methods to call their
own module-specific versions of maybe_disable_widgets()
in the codepath for admin_sections.load_admin_section().
And then for live updates, we just explicitly call
all four modules that support maybe_disable_widgets().
This should make switching between sections slightly faster,
and it also reduces the risk of module A messing with
module B's state. (Granted, we have lots of other ways
that modules can mess with each other's state.)
Bootstrap's typeahead is the main part of the project that we've
forked, and moving it to its own module should help unlock our ability
to upgrade bootstrap itself.
Since we're adding this to a large number of portico pages, there's no
guarantee that these pages actually have a CSRF input.
Though given that the logout template contains a CSRF input,
realistically it should always be present.
We weren't using the actual sorted data to find
the last element. This probably worked by accident
in some cases, but this commit makes it more
deterministic.
We want the return value from this for the next
commit, so we no longer call `format_drafts` indirectly
from `populate_and_fill`, and we rename the latter
to `render_widgets`.
We had an anonymous callback for drafts that was
hard to read. It's much easier to flatten the code,
give functions actual names, and stub them as needed
in the unit tests.
Since we're adding this to a large number of portico pages, there's no
guarantee that these pages actually have a CSRF input.
Though given that the logout template contains a CSRF input,
realistically it should always be present.
If a user has an old mention and has since been renamed, there's
really nothing for us to do to render it; we should just return as
though we have no data.
The `replying_to_message` field was used in some
early versions of compose fade, but it has no more
use in the current code.
The drafts implementation didn't really make any sense,
anyway, as we were claiming to reply to the same
message we were drafting.
A common source of confusion for new users is sending a message when
you're scrolled up in the message feed; in this case, it's nice to
communicate to the user why the message is not in view.
Fixes#10792.
Restructured by tabbott to replace overly complex logic for getting
the position of the new message with a `message_list.get_row()` call.
Now, we correctly avoid calling various password quality/strength
functions in the registration flow in the event that there isn't a
password form on the current page.
Before, some code wasn't inside a block at all, while other code was
using an incorrect check (an empty jQuery object is not falsey).
The overall result was that this would often crash on certain
pages/flows, stopping JS execution and causing various secondary
problems.
The first bug fixed here has been around for a long
time--we were redundantly updating unread counts
indirectly via muting_ui.initialize(). The
unread counts also get updated in
unread_ui.initialize(), when we have more valid
state. (And it's worth noting here that the unread
counts get updated yet again once message fetches
complete.)
The second bug was a very recent regression from
my recent stream name -> stream id cleanup in the
muting system. We now depend on stream_data to
initialize muting data, so we need to initialize
muting.js slightly later in the process.
These fixes are intertwined, because they were both
somewhat caused by the anti-pattern of having
muting_ui.js initialize unread_ui.js and muting.js,
instead of doing more direct, fine-grained initialization
from ui_init.js.
Essentially we replace this code:
exports.update_muted_topics = function (muted_topics) {
muting.set_muted_topics(muted_topics);
unread_ui.update_unread_counts();
};
with this:
exports.initialize = function () {
exports.set_muted_topics(page_params.muted_topics);
};
And the modules load like this:
stream_data
...
muting
...
unread_ui
And we don't need any page-load initialization for muting_ui,
which is mostly used for Settings/Muted topics.
This function used to be called initialize_from_page_params(),
and we called it indirectly through `subs.js`.
Now we call it directly from `ui_init.js`, which gives us a
bit more control over how things are initialized. In fact,
this sets us up for the next commit, where I fix a recent
regression I introduced.
The data attribute here has some value if you're
inspecting the HTML in the browser, but it's not
worth the extra code.
All the list items have data-stream-id, so there's
no need for the parent to have it.
The stream_list test that was fixed here was sort of
broken. It accomplished the main goal of verifying
what gets rendered, but now the data setup part is
more like the actual app code (and simpler, too).
This fixes the most core data structures inside of
muting.js. We still use stream names for incoming
data to set_muted_topics and outgoing data from
get_muted_topics.
This will make us more resilient to stream name changes.
Before, if you were logged on when a stream rename
occured, topics that were muted under that stream would
appear to be unmuted. (You could fix it with a reload,
but it can be jarring to have a bunch of unread messages
appear in your feed suddenly.)
Fixes#11033
The fixture changes are because self.upgrade formerly used to cause a page load
of /billing, which in turn calls Customer.retrieve.
If we ran the full test suite with GENERATE_STRIPE_FIXTURES=True, we would
likely see several more Customer.retrieve.N.json's being deleted. But
keeping them there for now to keep the diff small.
Like the other similar commits, we were doing the same work in all
code paths, just with a much more error-prone approach.
We can also now remove the now-unused finish_initial_narrow function.
Like the other commits in this series, we were already doing this in
all of the callers of load_messages; this centralizes that logic in a
less ad-hoc feeling way.
We no longer use or need the start_initial_narrow function.
Previously, each individual caller of load_messages that passed
num_before > 0 would do its own manual management of fetch_status;
now, we just do it inside load_messages.
Apparently, the older side of the FetchStatus object for home_msg_list
was incorrectly not being maintained. We got away with this, because
the do_backfill code path (which runs after we're done with the
load_more cycle) will correct the error for found_oldest. But we
didn't have proper handling for history_limited here.
When we're doing the load_more frontfill, we were not correctly
declaring that we were in the process of doing a fetch. Because the
next load_more call clears this state anyway, this was generally a
short race, off-screen, but it is still a data flow bug.
See the upcoming commits for a refactor that will eliminate the
possibility of this sort of bug.
When a user's name is edited, currently we still show the old name is
mentions (though clicking on the item does the right thing).
However, at present, it creates a new problem in search results, where
the highlighting is removed by this substitution.
A bug caused background links to open even when a modal in the user
settings overlay was active in the foreground. This commit fixes this
by disabling mouse events for the background when the modal is active,
and restoring them as soon as the modal starts closing.
Fixes#10654.
This adds a line to static/js/hotkey.js for focusing the "Close"
button. Tweaked by tabbott to make more clear that we don't expect
there to ever be both a close button and a save button, since in that
case this code would be busted.
Fixes: #3830.
The stream/topic edit areas now have these ids:
#stream_message_recipient_stream
#stream_message_recipient_topic
They are pretty verbose, but being able to grep
for these without noise does have some value.
Also, add a new notification sound, "ding". It comes from
https://freesound.org, where the original Zulip notification sound comes
from as well. In the future, new sounds can be added by adding audio
files to the `static/audio/notification_sounds` directory.
Tweaked significantly by tabbott:
* Avoided removing static/audio/zulip.ogg, because that file is
checked for by old versions of the desktop app.
* Added a views check for the sound being valid + tests.
* Added additional tests.
* Restructured the test_events test to be cleaner.
* Removed check_bool_or_string.
* Increased max length of notification_sound.
* Provide available_notification_sounds in events data set if global
notifications settings are requested.
Fixes#8051.
The Casper code that I eliminated here seems to be
bogus, in that I don't think it really waited for
all the clicks.
I **think** the intent of the test was to verify that
when you leave settings and go back into it, it remembers
the panel. I was able to verify this manually.
We have an upcoming change that lets us use the
back button after going arrowing through multiple
settings pages.
Without first adding this commit, we would have an
infinite loop when you came back to '#settings' and
then '#settings' would rewrite the url with the current
hash.
Just replacing the browser state allows the browser
to do the right thing.
The history protocol is pretty well supported:
https://caniuse.com/#search=history
We can eliminate the janky `setup_page` methods
and just pass in section from `hashchanged`.
This sets us up to handle browser history more
nicely when you load '#settings' and we could essentially
redirect you to '#settings/your-account' (or similar
things). A future commit will address that.
We also use `launch` as the new entry point, which
is more consistent with other modules.
The prior name of this was a bit inaccurate, as we no
longer ever hide the menu item for non-admins. Also,
it belongs more naturally in `gear_menu.js` at this point.
Also, we remove one call to this, which was in a place
where it was no longer necessary.
We now run the code to disable widgets every time
we reload a section, which was the original intention
of the code, but the call to it only happened when
you first launched the page.
We also continue to run this logic for live updates
of is_admin, although it's worth noting that the
code still only handles the "demotion" case of going
from admin to non-admin. (If somebody makes you an
admin, you continue to need to reload to get
widgets enabled.)
This ensures the "account settings" UI for managing a user's own email
address uses the delivery email, since that's what users care most about.
Eventually, we'll need to add support for at least viewing both email
addresses in "account settings", but this is the right long-term
behavior.
This new setting is still hidden in the UI when not in the development
environment, because the feature isn't ready for production, but
merging this will help simplify future work on the feature.
Previously, Topic editing was offered in the UI even to message
senders and organizations admins only if the message was no more than
one day old. This was correct for the "community topic editing" case,
but not for message senders and organization admins.
While we're at it, this also centralizes some previously haphazard
logic to always call message_edit.is_topic_editable().
Tweaked significantly by tabbott to fix the logic.
Closes#10568.
The goal here was to enforce 100% coverage on
parse_narrow, but the code has an unreachable line
and is overly tolerant of bogus urls. This will
be fixed in the next commit.
Also adds relevant tests and documentation. We currently
do not narrow to a new topic, and instead just narrow to
the stream. Similarly, we do not narrow to a PM if any of
the recipients are invalid.
We stopped setting this nearly five years ago, as part
of bd9cccffce
The big conditional that I removed here should have
always evaluated to false, as I understand the code.
Presumably either the browsers handle # -> '' redirects
better now, or we address this somewhere else in our
codebase.
We ignore keystrokes like alt-left-arrow and alt-right-arrow,
so that the browser can do back/forward.
We may need to refine the handling of ctrl/alt/shift in the
future, but now we only support single-key operations.
This change removes all the complexity around
get_hash_group(), and we now only go into the
"same overlay" logic within Settings or within
Manage Organization, but not between them.
This means if you're in Settings but hit the back
button to something under "#organization" we now
do "more stuff", since we want to err on the side
of reloading sections, etc.
There's not much flicker in my testing, and
this is not a super common transition, anyway.
This code brings the focus to the first input field with errors rather
than just the first input field present in the form after the sign up
form is rendered again after invalid data is submitted.
Note from tabbott: This still doesn't handle the ToS checkbox being
the source of the error, but that's an independent issue.
Fixes#10869.
Positioning using flexbox makes life much easier for everyone. With
this change we make positioning of icon relative to the label in the
dropdown menu much easier to do and alter if required. We now no
longer need to fiddle with tedious pixel measurements for placing the
icon in the right place.
As a result of this commit we had to change a click event binding
back to be associated with .dropdown-toggle class rather than being
associated with the h3, i because of the re-arrangement of the
dropdown configs.
Here we just fix the behaviour of angle icon which is present
in the integration categories dropdown. It used to change direction
from down to right only if "All" options from the dropdown was
selected (which is also the initial and default option). This behaviour
was pretty inconsistent and looked odd. Rather than having a direction
changing icon here, it migth be just better to stick with just the
down facing angle arrow. Arrow direction in general represents in
which direction the dropdown is gonna open up (in addition to the
fact that a dropdown exits here).
We make the integration categories dropdown gradually slide down/up
rather than appearing instantenously. I believe this gives a better
look to the dropdown and how it behaves.
We also fiddle a bit with the code relating to angle icon in the
dropdown. Basically though its behaviour was already buggy and
will be addressed in an upcoming commit, we try to maintain whatever
behaviour it had before introduction of the annimation effect.
The issue here was that if we opened up integrations page in
responsive mode (so the integrations category sidebar turns into a
dropdown) and click a few centimeters outside the actual dropdown
or perhaps the dropdown menu when its open, it is possible to toggle
or select a integration category.
What this essentially means is that clicking in blank area outside
visible boundaries of dropdown menu its possible to interact with it.
Fix: We change elements on which the click event is tied to and
adjust a bit of CSS for relevant elements so things look as they
used to but function in correct or better manner.
What is the buggy behaviour?
Before this commit if you were to open the integrations docs page
in a smaller window so that the integrations categories sidebar
changed into a dropdown (so that our page is responsive to
screen size), one would notice that selecting a category from the
dropdown menu didn't make dropdown to auto collapse. This feels very
uncomfortable from users prespective since an ugly dropdown with all
the categories sticking around uncollapsed kind of defeats the purpose
of having a dropdown.
Fix: We make the categories dropdown toggle/auto collapse upon
selection of a category.
Fixes part of #10026.
Adds additional option to typeahead:
`tabOpensEmptyTypeahead`(default: false):
tabOpensEmptyTypeahead overrides helpOnEmptyStrings.
This commit sets helpOnEmptyStrings to false and
tabOpensEmptyTypeahead to true. Now typeahead will
open on an empty string only if Tab has been pressed.
Even prior to my recent change in settings_panel_menu.js,
we were assigning window.location.hash a value that doesn't
have a '#' prefix. This probably doesn't matter too much
for the browser, but it does confuse our own checks about
whether we're redundantly updating browser history.
Now we prefix the settings hash with '#' and we encorce
this convention with a blueslip error.
Just calling update_browser_history is sufficient
here, and we end up short-circuiting some code
in hashchanged():
* we don't need to set state.old_hash, because
that's what update_browser_history does
* we bypass the is_overlay_check, which is always
false in this context
For stream links inside messages (like "#social") we
now use these functions:
hashchange.go_to_location:
We don't need to set href. Relative paths
are more standard, and the url is already
encoded.
hash_util.by_stream_uri:
This saves a step in building the URL.
We call hashchanged.update_browser_history() when
we switch panels. This API short circuits the
hashchanged callback and avoids code churn.
(We weren't actually double rendering, as the downstream
code does nothing at this point, so this is more
just preventig a pitfall and moving to a consistent
API.)
Before this commit, we would sometimes have
the toggler handle clicking or arrowing to
the All tab, but then also rewrite the hash
which caused us to re-process the event.
Now we only call update_browser_history()
in the callback handler from the toggle widget.
There's a bit of refactoring to make this happen,
but the call stacks end up being this:
call toggler.goto(...)
# callback is dispatched
call subs.switch_stream_tab
actually_filter_streams
update_browser_history
While they can share some code, opening the edit panel
for a stream and clearing the panel are pretty different
actions, so we simplify the API for each thing.
You no longer have to pass in booleans, and for the clear
case, you don't have to pass in a bogus node that just
gets ignored.
This fixes a bug where hitting the "n" hotkey was
causing double work related to the hashchange system.
The code is now organized like this:
do_open_create_stream() does the GUI piece
We call the above directly for hash changes.
For in-app actions, whether clicks or hotkeys,
we call open_create_stream(), which delegates
most of the work to do_open_create_stream() but
also updates the hash.
This diff looks a bit more complicated than it really is.
We had a bug where we'd call subs.change_state for
non-streams-related changes. The bug probably barely
impacted customers, since it's hard to get into that
situation unless you're in "Settings", and then the
code mostly did nothing. There's still a deeper issue
of what we actually do want to for settings changes,
but this fix does not address that.
We invert the conditionals related to internal state
changes, so that we can handle internal state changes.
And we make sure to only call subs.change_state if our
"base" is "streams".
This is mostly extracting the code within the `if`
block, as well as setting `base`, which wasn't used
elsewhere.
Also, the `else` no longer calls `is_overlay_hash`,
which was a redundant check.
This is definitely a micro-optimization, but avoiding
creating an extra object speeds up page loads by about
20ms per 1000 streams.
It's slightly sketchy to mutate the value in place, but
the original value never gets used again.
varunvaruns9
Joshua Pan
Tim Abbott
Vaibhav
Steve Howell
Vishnu Ks
Aditya Jain
Amadeo234
Marco Burstein
onkar shedge
Anders Kaseorg
Rohitt Vashishtha
Abhinav Singh
Aditya Bansal
Shubham Padia
Rishi Gupta