This is mostly a pure code move.
In passing I remove an unneeded call to
update_calculated_fields in the dispatch code,
plus some tests that don't need them.
This maps pageup/pagedown/home/end to close compose when used in empty
compose box, matching the existing behavior for the Up/Down arrow keys.
Currently, these keys do nothing when used in an empty compose box.
Typically, a user intends to navigate when pressing these keys (and
with empty compose, they can't be expecting to navigate within the
compose box), so it makes sense to map them to navigate the message
feed just to save users from needing to hit `Esc` in these contexts.
Fixes#17917
The `copy_handler` function that this shortcut calls is not useful
unless the body of a Zulip message is selected, so we shouldn't try
running it in other situations.
This logic correctly prevents the hotkeys implemented below it from
being active when "Recent topics" is open. We expect to change some
parts of that soon (see #17685), but in any case, we should always
return false in the hotkey code when we don't process a key, so that
default browser behavior works.
This fixes browser shortcuts like Ctrl+C, Ctrl+V, etc. when recent
topics is loaded.
This bug was introduced in 1eafb1d8b3.
Thanks to ganpa3 for noticing this bug.
This sorts the members imported within each individual declaration; we
use import/order for sorting multiple declarations.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
For filter values which don't exist or are invalid in some
way, we return false to show user that there are no messages
in the filter user is trying to render. Our previous behaviour
was to show all the messages and ignore the filter which
isn't good.
This reverts commit a00f5dd90e (#17801).
That commit introduced a regression in the portico pages as described
in commit 85b3157b47. Since that fix
introduced a regression of its own, we need to revert both commits for
now.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The start time of the last password change was the wrong time to use,
because we could start a password change, start another request,
finish the password change, and then observe that the other request
failed due to the password change.
We could use the end time, but a counter is more robust to
sub-millisecond race conditions.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commits adds on to 9884226f, which was added to
handle a rare race condition that occurs when the
session hash is not updated by the backend during the
password change process.
It handles a variant race situation where the request was initiated
before/during the password change event and completed after it was
completed. Hence, forcing the page to redirect to the login page.
This commit make compose_ui.autosize_textarea handle most of the autosize
logic of the textarea. It audits for any logic that is trying to do
autosize manually and replace it with compose_ui.autosize_textarea.
This allows to have better check for when the textarea is autosized.
Mainly done this so that we can have a check on #compose-textarea when
to autosize and when not to, thus helping to have all the logic in only
one function.
We now unconditionally enable backgroung events when 'hidden.bs.modal'
event is triggered on closing of modal. We do not need to handle them
separateley for closing modal by close_modal, data-dismiss or escape.
We handle this by single handler for modals in settings and subscription
overlay.
Fixes#16688.
This commit changes the code to not remove the modal element from DOM
after closing the modal for the deactivation stream modal and stream
privacy modal.
This is a prep commit for enabling background mouse events
unconditionally using 'hidden.bs.modal', because removing element
using '.remove' remove all events attached to it and this will also
remove the 'hidden.bs.modal' event which we do not want.
This remove behavior is inconsistent as we remove some of the modals
but do not remove some, so for now we are not removing the modal
after closing but it is anyway removed before opening a modal to handle
case of having two elements of same id and avoids any bugs.
This behavior of when to remove the element from DOM and when to not
remove needs to be discussed and may be modified in future.
We show a modal as a warning when unsubscribing a private stream
because it is a irreversible action and one cannot re-subscribe
tovit until added by other member of stream.
Fixes#9254.
This commit renames the class of both cancel button and the cross
icon to close-modal-btn.
This change is a prep commit for enabling background events on
'hidden.bs.modal' event. As we would enable background events in
furhter commit using the 'hidden.bs.modal' event, we would need to
remove the 'hide.bs.modal' event of deactivation_stream_modal which
removes the modal element from DOM.
When we remove this we would need a e.stopPropagation call to avoid
unexpected closing of subscription settings, which was not a problem
before as the element was removed from DOM before the actual closing
of modal.
So instead of adding a separate `e.stopPropagation' call, we can use
the same handler that is being used for stream privacy modal and this
is the reason the class name of cancel button of privacy modal is
being changed.
We do not remove the stream row instantly from the subscribed list in
subscription overlay when unsubscribing from public streams in most
of the cases but we do so when unsubscribing using hotkey.
This commit makes it consistent by not removing the stream row on
unsubscribing using hotkey.
We also not remove the 'active' class as streams settings is still
open in the right seciton and this behavior is also consistent with
the other ways of unsubscribing.
Note that this behavior is only for public streams, we remove the
stream row in case the stream is private, as user cannot
resubscribe himself and this behavior is consistent across all ways
of unsubscribing.
We now consistently set the PM counts for the right
sidebar toggle in unread_ui, similar to what we
do for the overall counts in the left sidebar toggle.
(Use a thin window to see the code in action.)
This breaks a dependency cycle.
In passing I improve the test coverage for the
actual job that pm_list still does (updating its
own total count in the "Private Messages" section).
For messages which we don't have stored locally, we don't update
our data structures. This was actually our behaviour before
59e5f2d8fc, which introduced this
bug.
Not doing this caused a major bug where we ran into errors
moving messages for topics for which we didn't have all
the messages available.
This data structure has never been one that we actually render into
the DOM; instead, its role is to support clicking into view that
contain muted streams and topics quickly.
This downgrade makes that situation much more explicit, and is also
useful refactoring to help simpify the upcoming changes in #16746.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
I lift this function out of message_store to
break some dependencies, and it's also more
consistent with the rest of the codebase:
alert_words.process_message
pm_conversations.process_message
recent_topics.process_messages
recent_senders.process_message_for_senders
We can do further cleanup to make these names
consistent (and possibly have them all work in
bulk), but that's out of the scope of the current PR.
We move the message_store.add_message_metadata function
(and all its dependencies) into a new module called
message_helper and rename the function to process_new_message.
(It does a bit more than adding message metadata, such
as updating our message store.)
We also have a "protected" interface now in message_store,
so that message_helper can access the message store:
update_message_cache
get_cached_message
Because update_message_cache is identical to
the former create_mock_message, we just renamed it
in the tests.
Most callers should use these functions:
message_helper.process_new_message (setting)
message_store.get (getting)
It's slightly annoying that the setter interface
is in a different module than the getter interface,
but that's how you break a bunch of dependencies.
We also extract the tiny message_user_ids class:
user_ids()
add_user_ids()
All the code moves here are pretty trivial, and
the code that was moved maintains 100% line
coverage.
The module name `message_helper` is not ideal, but it's a single
function and it'll save time to just do the topology change now and
leave thinking through the right name to later.
This function should be named similarly to the well-named
end_inline_topic_edit; previously the main hint that this was the
inline code path was just that the recipient_row was the parameter.
Before this we did not have remove event in server_events_dispatch.js
for the user group delete event even though server had. This was
leading to blueslip errors. Extracted the logic which was used in
success() of channel.del for user_groups into the remove case in
server_events_dispatch. Also removed the redundant reload call as
we already do that in server events.
This is a mostly verbatim extraction.
I re-phrased one line of code to work around a lint
false alarm. (Look for `preamble` in the diff.)
There are about 8 lines missing coverage here, so
the new module might be a good candidate to get
100% line coverage on.
Before this change, you would need to remove 74
edges from our dependency graph to make it
acyclic. Now it's 72.
This change does not impact the overall complexity
of our dependency graph (at least in terms of the
number of edges that we would need to remove to get
a tree), but it does clarify the picture a bit.
Rename few functions referencing the "CHOICE" field to
instead use the new "SELECT" name. This is done so that they
can be reused in "SELECT_MULTIPLE" field.
We were loading filters from localstorage after rendering the
filters block which caused the incorrect icons being displayed
for filters.
Since usually recent_topics renders a couple times when it
is loaded directly and the filters were being loaded as part of
show_selected_filters after we rendered recent_topics filters,
it meant the correct filters were being displayed in the
second render.
But, when user loads any other narrow directly when Zulip is loaded,
and opens recent_topics, recent_topics is only rendered once,
hence the bug gets noticed.
Fixes#17496
Extend our markdown system to support mentioning of users
by id also. Following these changes, it would be possible
to mention users with @**|user_id** and silently mention
using @_**|user_id**.
Main intention for extending the mention syntax is to make
it convenient for bots to mention a users using their ids. It
is to be noted that previous syntax are also supported.
Documentation tweaked by tabbott for better readability.
The changes were tested manually in development server, and also
by adding some new backend and frontend tests.
Fixes: #17487.
The only reason to use typeof foo === "undefined" is when foo is a
global identifier that might not have been declared at all, so it
might raise a ReferenceError if evaluated. For a variable declared
with const or let or import, a function argument, or a complex
expression, simply foo === undefined is equivalent.
Some of these conditions have become impossible and can be removed
entirely, and some can be replaced more idiomatically with default
parameters (note that JavaScript does not share the Python misfeature
of evaluating the default parameter at function declaration time).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
These were introduced in ff9a929d7a
with no explanation of why they were necessary.
Generally you only render a few things, and it's
important that they're up to date.
We weren't doing a good job of invalidating the cache.
Eliminating the cache will fix bugs (like presence circles
being out of date) and break some dependencies.
I removed some very fragile test code that was relying
on invalid values taken out of the cache. (We now have
less line coverage, but if we want to test our rendering,
there are much cleaner ways to do it.)
As part of testing this, I renamed Hamlet to "aaron", so
that there are two aarons, and then I logged on as Iago
to see the "secondary" code in action that shows their
emails to distinguish them.
This add the schema checker, openapi schema, and also a test for
realm/deactivated event.
With several block comments by tabbott explaining the logic behind our
behavior here.
Part of #17568.
This helper was added in eac6463031 and
used by the "message.handlebars" file. This is no current call for
this helper in the codebase, hence it is removed to improve coverage.
This commit also marks template.js to have 100% test coverage.
This adds support for unstarring all (starred)
messages from a particular topic, from the topic
popover.
The earlier implementation of this in #16898
was reverted in bc55aa6a01 (#17429)
because it had two problems-
1. The crash reported in bc55aa6a01
was due to message_store returning undefined. This happens
when the message itself hasn't been fetched from the server
yet, but we know that the message is starred from the ids
in `page_params` in `starred_messages.js`.
This commit handles this case explicitly.
Note that, we simply ignore those messages which
we haven't fetched, and because of this, it may
happen that we don't unstar some messages from that
topic. The correct implementation for this would
be to ask the backend for starred IDs in a topic.
2. The earlier implementation actually unstarred **all**
messages. This was because it grabbed the topic and stream_id
from the topic popover `data` attributes, after the topic
popover had been closed. This passed `undefined`, which
the function then interpreted as an action to unstar all
messages.
With this commit, we use the confirm_dialog widget,
which eliminates the need to store this data in the DOM.
* Currently, the confirm_dialog is used only in
the settings pane, which already has the `new-style`
class in the main `settings_overlay.hbs` file. So,
the confirm modal is rendered correctly there. But to
make it available for use outside of the settings pane,
we add the `new-style` class to the confirm container
itself, without which, the buttons look ugly.
* The other change here is the click handler for
removing the modal element. Previously, when the
modal was closed (with any of the "yes"/"no"/"cross"
buttons), there was a small time interval of around
a second during which the modal had disappeared,
but the background content was still in the faded-out
state. This change fixes this glitch. This glitch was
probably not noticable earlier, because the settings
pane itself causes the background to be slightly faded
out.
This code was added in 2d414fa897, after
the `window.exports` variable was removed from
`stream_popovers.js`, while converting it to an ES6 module
in c71af35461. This resulted
in opening the starred messages or all messages
popovers throw `Error: exports in undefined.`.
Follow up to #14768. This feature was already non-functional due to
.alert-display { display: none; }, and if we want to reimplement it,
we should do it using a modern library.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Remove the unused notifications-area wrapper. Remove the feature
detection code as all browsers recognize the <audio> element. Create
the <audio> statically with the page template. Use multiple <source>s
to let the browser detect the appropriate format instead of trying to
do its job for it. Remove the absurd loop="yes" attribute, which had
fortunately been specified on the wrong element.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This is mostly a refactoring to break the unnecessary
dependency of bot_data on settings_bots.
This is a bit more than a refactoring, as I remove all
the debounced calls to render bots during the
initialization of bot_data. (The debouncing probably
meant we only rendered once, but it was still needless
work.)
We don't need to explicitly render bots during
bot_data.initialize(), which you can verify by loading
"#settings/your-bots" as the home page. It was just an
artifact of how add() was implemented.
Note that for the **admin** screen, we did not and
still do not do live updates for add/remove; we only do
it for updates. Fixing that is out of the scope of this
change. The code that was moved here affects
**personal** bot settings.
Note that the debounce code is quite fragile. See my
code comment that explains it. I don't have time to go
down the rabbit hole of a deep fix here. The puppeteer
tests would fail without the debounce, even though I
was able to eliminate the debounce in an earlier
version of this fix and see good results during manual
testing. (My testing may have just been on the "lucky"
side of the race.) I created #17743 to address this
problem.
The keyboard-shortcuts icon currently has a fix position
causing design related bugs such as overlapping with userlist
in the sidebar.
The fix wraps the invite-more-users link and keyboard icon inside
a div with display property as flex instead of just using the anchor
tags inside the side-bar items.
This mainly extracts a new module called
browser_history. It has much fewer dependencies
than hashchange.js, so any modules that just
need the smaller API from browser_history now
have fewer transitive dependencies.
Here are some details:
* Move is_overlay_hash to hash_util.
* Rename hashchange.update_browser_history to
brower_history.update
* Move go_to_location verbatim.
* Remove unused argument for exit_overlay.
* Introduce helper functions:
* old_hash()
* set_hash_before_overlay()
* save_old_hash()
We now have 100% line coverage on the extracted
code.
I moved four functions, verbatim, to a new module.
They were in message_util before, which led to
filter.js having several accidental indirect
dependencies.
I considered just putting these four functions in
filter.js, but I think it's a nice abstraction boundary
that filter.js delegates actual message parsing, and
the original author apparently had a similar thought
process.
I also wanted to make it so that a casual reader of
filter.js doesn't think we are manipulating DOM. It's
true that we still indirectly require jquery here, but
it's only for parsing, and it seems plausible we would
eventually use a more low-level parser.
I can see us maybe using these functions in something
like MessageListData in the future, so speculatively
splitting them out might future-proof us from some
cyclical dependencies.
I also think it's plausible that we will just modify
our two markdown processors to attach that kind of
metadata to the messages.
Last but not least, I think there might be opportunity
here to simplify the filter tests and remove some of
the zjquery hacks. We would instead just mock the
message_has_* helpers for the filter tests, and then
do more detailed direct testing on the functions
themselves.
The only caller for this function was settings_config,
so we put it there.
For the stream_edit test we no longer mock the function.
(The reason we mocked the function was more about avoiding
the heavy settings_notifications import than the function
itself.) This gives some incidental coverage, but then I
also add some more real coverage on it.
We extract compose_fade_users and compose_fade_helper.
This is a pretty verbatim extraction of code, apart from adding a few
exports and changing the callers.
This change makes the buddy_data module no longer sit "above" these
files in the dependency graph (at least not via compose_fade):
* jquery
* lodash (not a big deal)
* compose_state
* floating_recipient_bar
* message_viewport
* rows
The new moules have dependencies that buddy_data already
had directly for other reasons:
* people
* util
And then buddy_data still depends on stream_data indirectly through
the compose-fade logic for stream_data. Even without compose-fade, it
would depend indirectly on stream_data via hash_util.
Note that we could have lifted the calls to compose_fade out of
buddy_data to move some dependencies around, but it's useful to have
buddy_data fully encapsulate what goes into the buddy list without
spreading responsibilities to things like activity.js and
buddy_list.js. We can now unit-test the logic at the level of
buddy_data, which is a lot easier than trying to do it via modules
that delegate drawing or do drawing (such as activity.js and
buddy_list.js).
Note that we still don't have 100% line coverage on the
compose_fade.js module, but all the code that we extracted now is
covered, mostly via buddy_data tests.
Previously, it was tedious to create actual message
objects in message_store for use in node tests.
This was mainly because, `add_message_metadata`
in message_store has many dependencies and
validation checks. Since it was difficult to create
actual message objects, many tests just mocked
the `message_store.get()` method to return the desired
message.
This commit adds a new helper method (`create_mock_message`)
to message_store, for use in node tests. This just stores
the object passed to it in the `stores_messages` map,
without any validation. We do not add any
default fields to the message object before saving
it from this helper, because doing so would decrease
the utility of this helper, and, if a test
depends on some field having a particular value,
then it would be better to just pass the field: value
pair from the test itself, for readability, rather
than relying on the helper to add the field for us.
This helper allows us to write deeper tests.
This commit also replaces some instances of mocking
`message_store.get()` to use this new helper method.
Previously, if a user had zero total starred messages,
we would still show the "Unstar all messages" in the
left sidebar on opening the starred messages popover.
This commit adds a check to show button only if the
user had non-zero starred messages. This is done
because-
1. The button, when shown when the user has zero
starred messages, is redundant and may be confusing.
2. Clicking on the button when having zero starred
messages sends a zero-length array to the backend,
resulting in HTTP 400 error.
Computed indexing into an object, especially with a user-provided key,
can be dangerous in JavaScript because of nonsense features like
obj["__proto__"]. In this case there’s no vulnerability because the
possible keys are strictly limited by the regex, but it’s always
better practice to use a Map for computed indexing.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit removes the unless msg/locally_echoed condition for the
edit content div, which has the consequence of making the "view
message source" widget always available for locally echoed
messages. This ensures that the message source can be seen if a very
long message has been drafted and it fails due to a server-side error
(See #17425 for the original report).
Fixes#17650.
This commit takes the blocks of code from "build_message_groups" that are the
same as "_rerender_message", and move those into a function called
"set_calculated_message_container_variables". This helps to avoid bugs in
future as in #17663. Like timestr was being updated in one of them, but needed
in both. So, it takes care that message variables are correctly set.
Part of #17663
This commit updates the _rerender_message to update the message_time
string with the current timestamp on the message rerender.
When we locally echo a message, we store a local timestamp that will
generally not be used as it is replaced by the server time in
echo.process_from_server when we confirm receipt of the message.
echo.process_from_server correctly updates the .timestamp field on
the message and triggers a rerender but that rerender reuses
the message_container object without recomputing the
message_container.timestr due to which wrong older timestr was shown
on the message box.
This commit fix this by calling set_timestr in the rerender code path,
alongside calls to update similar data structures like
this._maybe_format_me_message.
Fixes#17655
In responsive narrow windows where the left sidebar is an overlay, clicking the \vdots menus for
'All messages' and 'Starred messages' would result in the navigation closing and the menu appearing
somewhere weird.
We fix this the same way that we address this issue with the similar stream/topic menus, by calling
the function to show this sidebar after closing all popovers.
Fixes: #17537.
Fixes#17466
This commit will change encoding logic. Initial logic
was not encoding parenthesis, and this creates conflicts
with the markdown link format. To resolve this while encoding,
we're now replacing parenthesis with ".28" and ".29."
There is no need to change decoding logic because before
decoding any URL, we first convert all the “.” to “%.”
optimization: No need to replace parenthesis in popovers.js.
The scroll position of recent topics table is according to the
element which is in focus.
While this behaviour is correct, when
user clicks on an element in recent topics after scrolling to
a different position, the scroll position is lost as the focus
was not being set on the element. This commit ensures that
we set focus on the element when user clicks on it. Thus, the
scroll position being lost is naturally fixed.
Fixes#17587
This commit removes the option to add more streams out of scrollbar
as it is not visible on mobile devices or organizations with large number of
streams until scrolled down.
There is no element on the settings page with
id="admin_page_filters_loading_indicator", so
the indicator doesn't appear. And even if we make
a div for an indicator, it would be hardly visible,
because we don't call the server to fetch linkifiers
in this page, and there generally won't be too many
linkifiers to render.
Add input to filter in sortablejs config. This prevents drag
and drog from being called on clicking input field. Also
set preventOnFilter to false. This prevents disabling the
default behaviour on the click event.
Fixes#17619
Previously we could navigate the user info popover on messages by
using the up/down arrow keys, but we could not use the enter key to
select an item, this commit fixes the bug.
Fixes: #17589.
These sigils will help make it easier to see that this is a special
Zulip syntax feature, not just something the other user typed, which
might help set the expectation that we're showing the time in the
user's timezone.
Tweaked by tabbott to improve variable/template naming.
Apparently, we never tested the unlikely behavior of deleting the last stream,
and doing so would result in exceptions being thrown (and thus no UI update).
Fixes: #16691
This is needed not because the DOM isn’t ready here (we’re in a
<script defer>), but because we want to wait an asynchronous tick
until after all the other callbacks that waited an asynchronous tick
for the DOM to be ready.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We have generally gone away from using $(...)
initialization in modules that we test with
zjsunit, but there are a few remaining special
cases related to our billing and portico
codebases.
I remove an obsolete comment--we use get_streams()
for the `n` key now.
I also remove a guard statement from sort_groups()
that returned `undefined` for empty lists.
That guard statement would break this code:
const stream_groups = stream_sort.sort_groups(streams, get_search_term());
if (stream_groups.same_as_before && ...
The calling code prevents the situation anyway:
const streams = stream_data.subscribed_stream_ids();
if (streams.length === 0) {
return;
}
I modify the "no_subscribed_streams" test to test
the new behavior. (Even though stream_list currently
short-circuits the call here, that may change in the future.)
I also introduce the test() wrapper to explicitly clear
our data.
Don't focus on search box when user is at end or start of
the table and is using vim keys. This ends up being a
bad UX as once user is inside the search box, vim
navigation keys cannot be used to take user out of
the search box.
This introduces the make_stream_message()
helper to avoid all the strange
`messages[0] === message1` confusion.
We also clear data explicitly at the beginning
of the test.
It's still a messy test.
I considered using set_info({}, 0) to clear data,
but it has a lot of machinery that could lead
to accidental line coverage and/or extra test
complexity.
This commit removes presence circles for special users like
all, stream, and everyone. This was discussed at
#design>Presence circles in typeahead, and this was justified
as presence circles for these special users will always be grey
circle and do not convey any information about presence of anyone.
Use confirm_dialog here as this change is destructive and thus not
easy to undo.
We may want to consider using settings_ui.do_settings_change()
instead.
Fixes#17073.
Split the logic of check_profile_incomplete into two functions
show_profile_incomplete and check_profile_incomplete.
The latter is passed to the former which shows the message if the
profile is incomplete.
Use a regex to check for the pattern "Organization
imported from ..." instead of the previous approach
where we just checked if pattern startswith "Organiztion
imported from".
This allows users to extend the description from the original
"Organization imported from Slack." with a few extra sentences without
this warning remaining indefinitely.
Fixes#17463
TextField is used to allow users to set long stream + topic narrow
names in the urls.
We currently restrict users to only set "all_messages" and
"recent_topics" as narrows.
This commit achieves 3 things:
* Removes recent topics as the default view which loads when
hash is empty.
* Loads default_view when hash is empty.
* Loads default_view on pressing escape key when it is unhandled by
other present UI elements.
NOTE: After this commit loading zulip with an empty hash will
automatically set hash to default_view. Ideally, we'd just display
the default view without a hash, but that involves extra complexity.
One exception is when user is trying to load an overlay directly,
i.e. zulip is loaded with an overlay hash. In this case,
we render recent topics is background irrespective of default_view.
We consider this last detail to be a bug not important enough to block
adding this setting.
When user presses escape but there is no action that recent topics
can perform on it, it returns false.
The final state of focus is the focus on topics table. If the focus
is on table and not on RT search or filters, we return false to
indicate that the key was unhandled by recent topics.
This will allow escape to take user to another view if recent topics
is not the default view.
"Alert Words" is one of Zulip's oldest settings UI elements, and as a
result is buggy. This commit converts it to use our standard
progressive-table-wrapper system used for settings tables, which has
the side effect of fixing a bug that mad ethe tables look pretty bad
if one adds a very long word.
Fixes#17172.
This commit addresses the problem of user's status visibility to
some extent. It adds presence circles, like we have in buddy_list to the
typeahead suggestions that are given for mentioning users in messages.
Tweaked by tabbott to adjust vertical alignment of group mentions as well.
Testing for the changes is done manually in the developement server,
and also by updating frontend tests to address these changes.
Fixes: #17138
This === comparison between two Date objects added by commit
9896782fd1 (#17220) always returned
false, so the body of timerender was running every minute instead of
every day.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Commit 13915740bb (#5199) added a loop
in update_timestamps that appended an entry to update_list once for
each element that its className matched. If there were two such
elements, this would double the length of update_list each time the
body of update_timestamps ran. So let’s not do that.
Also fix the incorrect elements !== null check from the same commit.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This reverts commit 6fba17599f (#16898).
@chrisbobbe reported this crash:
Uncaught TypeError: Cannot read property 'stream_id' of undefined
at starred_messages.js:43
at Array.filter (<anonymous>)
at Object.e.get_topic_starred_msg_ids (starred_messages.js:40)
at stream_popover.js:221
at HTMLSpanElement.<anonymous> (stream_popover.js:358)
at HTMLUListElement.dispatch (jquery.js:5429)
at HTMLUListElement.v.handle (jquery.js:5233)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We don't need to handle user clicking on Zulip logo since
changing the hash via the `a` tag takes care of it automatically.
Also, cleanup the narrow.restore_home_state function since
it is no longer being used.
We manually trigger a re-render of RT after a stream is muted
to update the list of topic in RT for the active filter.
This fixes the bug that RT doesn't update correctly
after a stream is muted.
If user is in private message narrow, we reduce height of stream
list to allow height for pm list in the left sidebar. We need
to recalculate it when moving out of pm narrow and moving in
rt narrow.
Since All messages narrow is no longer home page for webapp,
we change its icon to align-left which also shows a concept of
interleaved topics / messages.
When idle, we try to backfill messages and in the end reselect
the closest message in the list, which can be a unread message
if present.
When recent topics is open, we can backfill messages; but
shouldn't select the message_id otherwise it will mark the
message as read if the message is unread while triggering
`message_selected.zulip`.
User can go from recent topics to stream / topic narrow via various
means, but all go through narrow.activate, hence we make sure all the
state changes we do in recent_topics.hide are actually applied when we
hide recent topics and go to another narrow.
This fixes the bug that narrowing from left sidebar to a stream
takes user to the top of the narrow.
The top row of the RT can be hidden sometimes after scrolling down
and then scrolling up. This is because the focus is applied before
the row is rendered. Applying the focus after the topic row is
rendered by the browser makes sure it is always visisble when
it needs to be.
For inputs to recent topics which were unhandled, we return false
so that the browser can handle them.
This also fixes the issue of search box not able input `t` key.
We land user on the first row of the table instead of the search
box because here user can access hotkeys like `w`, `q`, `/`, etc,
which will not be directly available if user is focused in
recent topics search box.
For tests:
We set focus to search by default to avoid mocking a lot of
table html for getting the tests passing.
This fixes the bug where a user cannot type vim keys in the
general search box / user search / stream search,
since they are captured by recent topics.
The behaviour was flaky for stream search, but can be reproduced
consistently after previous commit fixing the popovers.any_active
output.
Previously the filter would be reset every time the page was
refreshed. This commit adds persistence via localstorage, the tests
follow the pattern used in tests for drafts.
Fixes: #15676.
When user directly has hash for overlay in the URL when app loads,
we need to still show recent topics in the background. This
doesn't need to happen in other cases when user is accessing
the overlay after UI is loaded.
Go to Recent Topics on "#", no hash and "#recent_topics".
Go to Recent Topics as the last destination for escape key.
Map `a` key to All messages and change its hash to
`#all_messages`.
throttled mousewheel handler marks messages as read in the
message_list regardless of if the message_list is visible or not.
We don't trigger it if recent_topics is visible.
Recent Topics is no longer an overlay now, but note that it is
also not a typical messages narrow. It can reside between
an overlay and a Filter in the sense that it is dispalyed as
a typical Filter narrow but has properties of an Overlay.
Compose box is not visible in this view as it will be confusing
to many users and hence compose shortcuts have also been disabled.
Keyboard shortcuts that apply on messages have also been disabled.
The remaining shortcuts that apply to a narrow are still accessible
here.
We now only assign target once, rather than
assigning it then overwriting it for the
not-sent-by-me use case.
I tried to extract a "selector" here but the linter
complained.
Splitting up the tests here ensures we don't
needlessly do extra work here. (In the prior
clumsy implementation, the second test that
I split out here would fail due to the lack
of us setting up the jquery stub.)
On realms with large numbers of custom emoji, the typeahead emoji picker
often isn't useful. This is exacerbated by the fact the picker prefers
longer matches, so if there are five emoji that share a prefix, and an
emoji that is just the prefix, the only-prefix emoji will never show in
the typeahead emoji picker. This means that if someone thinks that there
is an emoji that shares a prefix with many other emoji, but they don't
remember for sure, they cannot use the typeahead emoji picker to check
that the emoji that they are entering exists.
There are two "real" fixes to this, neither of which this commit
addresses:
First, we should adjust the emoji ranking code such that exact string
matches for existing emojis are always shown in the picker. This would
be an improvement overall (the current behaviour is surprising and
frustrating), but it doesn't fundamentally solve the problem - if there
are many matching emoji, some of them will be pushed off the list.
Second, we should allow scrolling through the entire list of matching
entries in a typeahead, instead of only looping through the top N
matches. This will completely fix the problem (although there is some
UI/UX consideration in how to make it clear that the box is scrollable),
but seems like significantly more work to implement.
However, increasing the typeahead box size should improve the user
experience here independently of either of those changes.
I've chosen 8 as the max size for no particularly principled reason -
the fact that it's larger than 5 makes it more useful, but it's not so
large that it covers an obnoxious amount of the screen. Possibly it
would make sense to make it a bit bigger, but 8 seems like a good place
to start.
I've tested this on my laptop, which has a Intel i5-7200U CPU (~4.5 years
old, middle of the line when it was released) on a test instance with
5000 users, as well as on chat.zulip.org, and didn't see any noticeable
performance regression in completing @-mentions or emoji on either.
This warning was added in #6551. It’s not for any version of the
current Electron app, which we warn about on the server side with
DESKTOP_WARNING_VERSION, but rather some pre-Electron app so ancient I
don’t even know what it is. Apparently it communicated using the
window.bridge global, so eradicate that too.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Appling i18 to reaction tooltips (#16585) caused usernames to be
double-escaped, for instance, if there is a single-quote in a username.
This disables escaping of usernames by i18next, since they're escaped
again later by the rendering code.
Fixes: #16785
We weren't exercising this method in any
meaningful way during the tests, and when
do add coverage, we probably want to just
test it directly.
We also kill off stub_selector(), which was
never well-documented.
Now we just update the whole row any time a sub
changes. This prevents a whole class of bugs.
As the TODOs indicate here, some of the post-processing
that we have to do on rows after rendering the
template will soon go away.
I audited all the functions in stream_ui_updates and
added TODO comments to functions that are clearly just
updating rows in the left panel of Manage Streams.
In an upcoming commit I will simplify the approach so
that we just re-render the entire row.
The tooltips for the left panel of stream settings
have been broken since November 2018 due to my
commit 8f915da2ca.
The code prior to 2018 was restoring tooltips
right inside the loop where we were detaching
the row from the DOM to put it back into the
DOM at another place. And then I tried to
just add them in bulk, forgetting that I was
in the middle of all the DOM manipulation (and
hence my selector for the loop was a noop).
Also, I don't think we've ever had them for live
events that add streams. (I fixed that too.)
It's not clear to me that this code is actually
necessary, as we get hover help without
calling $(...).tooltip(...) properly.
This is probably why we didn't notice any
breakage when we merged my 2018 commit.
Checking for the button was a brittle way to do this.
Note that the code on master is flawed insofar as
we don't respect the search filters. I don't fix that
bug here. This is a tactical change to eliminate
another function.
Upcoming changes will make it so that all the bugs
related to "notdisplayed" will simply go away.
The "Narrow to PM with" notification above the composebox was
double-escaped, mangling names with single quotes in them. This removes
the escaping in i18next, causing the name to be escaped only in
handlebars.
We just want to reset the scrollbar here, which
we still do via ui.reset_scrollbar.
You don't want to preserve scroll position if
you are filtering or re-sorting.
We have long had this annoying two-pass way of building the
DOM that I am trying to eliminate.
The function names that I introduce here describe the current
situation more accurately.
In passing I make it so that we only throttle redraws when
users are actually typing. Using a throttled redraw when
you click on the sort icons is at best unnecessary, and it
may actually aggravate double clicks.
This bug was caught thanks to the earlier commit which
introduces the "Restart tutorial" feature. To reproduce
the bug,
1. Restart tutorial
2. Click "Got it!" on the intro_reply hotspot
3. Repeat steps 1 and 2
The hotspot for intro_reply won't disappear the second
time around and the intro_stream hotspot would be displayed
simultaneously.
The reason for this was the intro_reply's "Got it!"
button codepath never removing the item completely from
the DOM. This would then conflict with the new intro_reply
hotspot which would get assigned a different 'id'.
Adds a "unstar messages in topic foo" option to the topic sidebar
popover, if there are any starred messages in that topic, known
to the frontend.
Altered existing "unstar all messages" confirmation modal to mention
the topic name, in the case that it was opened by the topic sidebar
codepath.
This is just a v1, and will not unstar old messages from that
topic, if they have not been fetched by the frontend.
Fixes#12194
Co-authored-by: Abhijeet Bodas <abhijeetbodas2001@gmail.com>
This prevents a bug where we interpret "2something"
as a modern slug instead of a legacy stream name.
The bug was probably somewhat unlikely to happen in
practice, since it only manifests if 2 is an actual
stream_id.
Steve Howell
Ganesh Pawar
Tim Abbott
Aman Agrawal
Anders Kaseorg
Mateusz Mandera
YashRE42
Riken Shah
Signior-X
sahil839
Sumanth V Rao
tushar912
PIG208
Gaurav Pandey
Vishnu KS
Abhijeet Prasad Bodas
Nishant Mittal
m-e-l-u-h-a-n
shanukun
100RABHpy
yasiruRathnayaka97
pilgrim2308
Nikhil Maske
Harsh Srivastava
Josh Gilley
Wesley Aptekar-Cassels
Abhishek-Balaji