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.
We now let color_data keep its own state for
unused_colors, so that we longer have to pass in
a large list of unused_colors every time we want
to assign a new stream color.
This mostly matters at startup, where we might
be cycling through 5000 streams. We claim all
the unused colors up front.
Each operation now has an upper bound of expensiveness,
where the worst case scenario is basically popping
off the first element of a list of <= 24 colors.
The algorithm is now deterministic, too, to make
it easier to test. It's unclear whether random color
assignment ever had much benefit, and it made unit
testing the algorithm difficult. Now we have 100%
line coverage.
Fixes part of #10902.
Tim Abbott
Steve Howell
Aditya Jain
Amadeo234
Marco Burstein
Joshua Pan
onkar shedge
Anders Kaseorg
Rohitt Vashishtha
Abhinav Singh
Aditya Bansal
Shubham Padia
Rishi Gupta
Shubham Dhama