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.
When there is some error in connecting to server(more specifically to the
tornado server) the "Unable to connect to Zulip" connection error message
gets cleared as Django server could send the response of "get" request of
old messages and hence get_old_messages_success hides the error message
even though the connection is not properly established.
Fixes: #5599.
This commit fixes bug: When user clicks on remove-user-pill-btn,
it closes the parent modal instead of removing user pill from input.
This happens because button has class `exit` and there is click
event listener on all `exit` class buttons, which closes modal.
Fix this by adding `e.stopPropogation` to remove-user-pill listener.
While we don't actually need another tooltip on /stats right now, this
provides a clear approach for how to do that. We've since added
tooltips in various other parts of the webapp, and that code is pretty
copy-pasteable, so I think it's reasonable to say this closes#4612.
Cleaned up by tabbott to remove a bunch of unnecessary changes.
This code prevents the password bar from being incorrectly clear after
the sign up form is rendered again after invalid data is submitted
(generally due to forgetting to agree to ToS).
Fixes#10868.
I think this will fix a Casper flake where there was a race
window with multiple temp DOM elements holding copied text.
I also add a comment to the code I think causes this race
for the tests.
Previously, because the parens were added via CSS, copy-pasting the
EDITED notices resulted in junk like this:
Iago 3:51 PMEDITED
edited message content
Now, you get:
Iago 3:51 PM (EDITED)
edited message content
Status messages were incorrectly not selectable, due to a bug in how
we setup the no-select hierarchy (for making copy-paste not have weird
whitespace issues).
Fixes#10456.
For many years we have been excluding the current user
from the buddy list, since their presence is kind
of implicit, and it saves a line of real estate.
This commit removes various user-is-me checks
and puts the user back for the following reasons:
* explicit is better
* newbies will be less confused when they
can see they're actually online
* even long-time users like myself will
feel more comfortable if it's just there
* having yourself in the buddy list facilitates
things like checking your presence or sending
yourself a message
* showing "me" reinforces the meaning of the
green circle (if my circle is green and I'm
active, then others with green circles must
be active too)
* If you're literally the first user in the
realm, you can now see what the buddy list
looks like and try out the chevron menu.
The biggest tradeoff here is the opportunity cost.
For an org with more people than fit on the screen,
we put the Nth person below the fold to show "me".
I think that's fine--users can still scroll or
search.
This commit doesn't do anything special with the
current user in terms of sorting them higher in the
list or giving specific styling.
Fixes#10476
We reduce nesting of code by just early-exiting
for the `is_current_user` check.
This also forces us to be a bit more thorough
with our tests if we want to maintain line
coverage.
For message groups, I just changed the internal name
to "topic_links".
For uses of "subject_links" that are tied to how the
server names fields, I introduced these wrappers:
* util.set_topic_links(obj, topic_links)
* util.get_topic_links(obj)
These can be used for either messages or events.
Previously, messages were a string of disconnected regions. Modeling them as a list brings several benefits:
* Quickly jump to the message list by using a screen reader's list navigation hotkey.
* Quickly jump between messages by using a screen reader's list item navigation hotkey.
* Quickly jump to the beginning or end of message lists in screen readers that support it.
This is a nice performance optimization for the rare case where the
user does quote-and-reply on a message, aborts the compose, and then
re-does the quote-and-reply.
We split out two new functions and call them
everywhere that we used to call add_display_time():
- `update_group_time_display`
- `update_timestr`
We also make some of the local vars more consistent,
as well as doing more explicit clearing of vars than
`delete`.
Splitting these functions will allow us to muck with date
dividers without affecting the `update_str` functionality.
Change wording of public stream description to
"Any member of the organization" from "Anybody"
to indicate that guest users can't subscribe even
public stream of organization.
Previously, when a new stream was created on a client other than the
current one, the browser would first receive the "stream_created"
event, and make up a client-side display color at that time to use in
the "stream settings" view (it doesn't yet know the color that was
selected when the user was actually subscribed, because it doesn't
even know yet that the user is being subscribed to this stream), and
then moments after it'll receive a "susbcribe" event letting the
client know that the user is subscribed (and specifying the color to
use).
However, due to an argument not being passed through properly and a
missing rerender, we were not properly updating either the data
structures or doing a stream colors rerender in order to show the new
color.
This fixes the issue reported in
https://chat.zulip.org/#narrow/stream/48-mobile/subject/stream.20colors/near/660170
This moves the deactivate account which was previously placed under
"SETTINGS / YOUR ACCOUNT" -> "Deactivate account" to "SETTINGS / YOUR
ACCOUNT" -> "User settings" for making it more visible.
Use the placeholder `[Quoting…]` when quoting and replying before the
quote has been added to the message. Also, add tests to the
`compose_actions` Node tests for the new behavior.
Fix#10705.
Guest users can't access subscribers of any(public or private)
non-subscribed streams. Therefore, hide subscribers list
of all non-subscribed streams from guest users in UI.
Fixes#10749 (the previous parts were fixed already).
This will change the hash of the URL when a new tab
gets selected. Vice versa when the billing page is opened
the appropriate tab is selected according to hash of
the URL. This means when the card gets updated the
page would be reloaded correctly to show #payment-method
tab.
When a user clicks the compose `+` button, create a popover at the
bottom right of the screen including buttons for opening a new stream
message or a new private message.
Use CSS to display a `+` button on mobile but keep the more verbose
buttons on desktop. In the future, this button will be used to display
a popop for a new message.
Guest users can't subscribe themselves to streams, so we shouldn't
display the subscription button at end of stream message view.
Fixes part of #10749.
Guest users can't subscribe themselves to any stream, so we hide the
"Subscribe" button. Previously, it was showing Subscribe button after
a guest user unsubscribed from a stream.
Fixes part of #10749.
The "notification settings" page previously advertised support for
mobile push notifications via checkboxes, even if the server hadn't
yet been registered for push notifications. This was a frequent
source of onboarding pain for new Zulip organizations.
We fix this by providing a clear warning and disabling the relevant
inputs on the settings pages.
Modified significantly by tabbott to correct some tricky logic errors
as well as some copy-paste bugs.
Fixes#10331.
We want to avoid `blueslip.error` in cases where
the root cause could just be bad data that is
human-entered.
There are a few callers here who **should** be
sending good data all the time, but hopefully
they either have good test coverage, other
obvious failure symptoms, or, ideally, just
do what the user would mostly expect in the
face of bad data.
This supports guest user in the user-info-form-modal as well as in the
role section of the admin-user-table.
With some fixes by Tim Abbott and Shubham Dhama.
This is the natural behavior that most users will
probably expect. If you need to go to All Messages when
topics are zoomed in, you can just hit ESC twice.
Before this change, if you hit ESC, then hotkey
code would call search.clear_search, which would
call narrow.deactivate(), which would then use
`$('#search_query')` to clear a value, but then
let search.clear_search blur the input and
disable the exit button. It was all confusing.
Things are a bit more organized now.
Now the code works like this:
hotkey.process_escape_key
Just call narrow.deactivate.
$('#search_exit').on('click', ...):
Just call narrow.deactivate.
narrow.deactivate:
Just call search.clear_search_form
search.clear_search_form:
Just do simple jquery stuff. Don't
change the entire user's narrow, not
even indirectly!
There's still a two-way interaction between
the narrow.js module and the search.js module,
but in each direction it's a one-liner.
The guiding principle here is that we only
want one top-level API, which is narrow.deactivate,
and that does the whole "kitchen sink" of
clearing searches, closing popovers, switching
in views, etc. And then all the functions it
calls out to tend to have much smaller jobs to
do.
This commit can mostly be considered a refactoring, but the
order of operations changes slightly. Basically, as
soon as you hit ESC or click on the search "X", we
clear the search widget. Most users won't notice
any difference, because we don't have to hit the
server to populate the home view. And it's arguably
an improvement to give more immediate feedback.
If you zoom into "more topics" for a stream that has
a LOT of topics, and then scroll down to the bottom,
and then zoom out by selecting "All messages" or
similar upper-left-sidebar options, we now try to scroll
the more recently active stream back into place after we scroll
out.
Before this change, it was possible for your lower left
sidebar to appear empty, as it would keep the
scroll offset from "more topics".
If our topic list isn't zoomed in, avoid calling
stream_list.zoom_out_topics().
This commit also introduces `zoomed_in` to track
our topic zooming state.
This small modules nicely breaks down the
responsibilities of topic_list and stream_list
when it comes to zooming in and out of topics
(also known as hitting "more topics" or "All
Streams).
Before this, neither module was clearly in
charge, and there were kind of complicated
callback mechanisms. The stream_list code
was asking topic_list to create click handlers
that called back into stream_list.
Now we just topic_zoom set up its own click
handlers and delegate out to the other two
modules.
This fixes a regression from here:
88b4a9f2d7
The fix didn't account for how huddles are
represented as comma-delimited strings.
We also simplify the logic by extracting a
function and doing early-exit for simple
cases.
Internally we generally omit our own id and email
in data structures related to PMs, except when we
are the sender, but if we receive "perma links"
we will need to filter out our id.
This reflects the newly selected value of role in "role" column under
active-users section and deletes the redundant admin-icon updation code(
As we already removed bolt admin-icon)
New user avatar width is not maximum when user upload
new image. Because wrong html element is accessed for
setting value of image src attribute.
This commit removes these code from success of ajax call,
cause we already handle this in event `user_events - avatar_url`.
This deduplicate code for the checkboxes which are dependent on other
parameters and it makes no sense changing them when that parameter is
false. For example, changing `message_content_in_email_notifications`
makes sense only when `enable_offline_email_notifications` is true.
There is need for such a helper because `unless` executes to be true even
when we haven't passed the context variable on which we are checking the
conditional statement.
We drop support for usage of `icon-vector` as base class when
including icons from font awesome icons package.
Now on, only icons as specified in font awesome v4.7.0 can be used
in the code base.
We now ask compose_pm_pill to give us a list of user
ids that we are PM'ing to, and we only convert user
ids to emails right before we put requests on the wire.
We also let the "pill system" tell us whether we
have unconverted data.
It also sets up for an upcoming server change where we
can just send user ids to the server.
This change should be transparent to the majority of users.
For Zephyr users we are slightly less aggressive about
sending typing indicators, since we now require valid
user ids.
This module makes it really easy to create are-you-sure
dialogs for dangerous operations.
Basically it's one function with five parameters. You
give three chunks of HTML, a callback function, and
a parent container.
The first use of this will be in settings_user_groups,
coming up in a couple commits.
This removes some unnecessary code duplication in the CSS classes for
Google and GitHub authentication social auth buttons.
This will, in turn, help us avoid extra work every time we add a new
authentication backend.
Changes -
a) Updated the border-radius to 4px for all the buttons.
b) Increased the margins between the labels and inputs.
These changes affect the login and register page's styling.
Previously we only added the active class to the Date uploaded
column, thinking it was already sorted by upload date by default.
However, it wasn't, so now we explicitly make a call to sort it by upload
date to fix an issue with broken sorting.
Fixes#10518.
I don't know how long this has been broken, but it seems
some re-design of our message feed moved the Subscribe
button out #zfilt, so we use a different parent selector
now to turn on the click handler.
Hopefully this was a pretty obscure bug. To reproduce
it go to "Manage Streams" and then select a stream you're
not subscribed to (from "All Streams"), and don't actually
subscribe, but then hit "View stream".
The user experience here is still a bit confusing, but
this is just a quick fix.
Previously, if someone updated his/her name from accounts page and closed
the modal and then reopen the modal, the page still had the old name as
we use `page_params.full_name` in `accounts-settings.handlebars`. This
commit fixes this bug.
Fixes: #10529.
Since now we have email notifications for streams messages too, so
there is no direct dependency of
`message_content_in_email_notifications`checkbox on
`enable_offline_email_notifications` setting and neither we can say it's
dependent on `enable_stream_email_notifications` as well because we may
have email notifications set for individual streams. So removing this
checkbox dependency is the best solution here.
I used line-height: 22px and font-size: 16px in .message-content to take the
screenshots. Requires some additional fiddling for the LaTeX picture, inline
code block, and maybe a few others.
dipu989
Scott Lee
Tim Abbott
Steve Howell
Marco Burstein
Rishi Gupta
Shubham Dhama
Anders Kaseorg
Yashashvi Dave
Balaji2198
Nikhil Kumar Mishra
Abhinav Singh
butchyyyy
Jeswin
Vishnu Ks
Nolan Darilek
Rohitt Vashishtha
Eeshan Garg
Roger Souza
Shikhar Vaish
Pragati Agrawal
Aditya Ramesh
Cynthia Lin
Aditya Bansal
Akash Nimare
Akash Nimare
Lenny Jagielski
Harshit Bansal
Adam Birds