We now call $.clear_all_elements at the top
of run_test.
We have to exempt two modules from the new regime:
compose
settings_user_groups
Also, if modules do set_global("$", ...) we don't
try to call the non-existent function.
It's possible we'll want to move to something like
this, but we might want to clean up the two
sloppy_$ modules first:
// AVOID THIS:
// const $ = require("zjquery")
run_test("test widget", ({override, $}) => {
override(foo, "bar", ...);
$.create(...);
// do stuff
});
We no longer export make_zjquery().
We now instead have a singleton zjquery instance
that we attach to global.$ in index.js.
We call $.clear_all_elements() before each module.
(We will soon get even more aggressive about doing
it in run_test.)
Test functions can still override $ with set_global.
A good example of this is copy_and_paste using the
real jquery module.
We no longer exempt $ as a global variable, so
test modules that use the zjquery $ need to do:
const $ = require("../zjsunit/zjquery");
We still need to write to these globals with set_global because the
code being tested reads from them, but the tests themselves should
never need to read from them.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
I have a local branch with a hacked up version of
zjquery that lets you basically detect when zjquery
stubs are never actually invoked by real code.
There are some nuances to that kind of audit, so
I haven't pushed the auditing code, but these
are low hanging fruit.
Using web_public_guest for anonymous users is confusing since
'guest' is actually a logged-in user compared to
web_public_guest which is not logged-in and has only
read access to messages. So, we rename it to
web_public_visitor.
Instead of prohibiting ‘return undefined’ (#8669), we require that a
function must return an explicit value always or never. This prevents
you from forgetting to return a value in some cases. It will also be
important for TypeScript, which distinguishes between undefined and
void.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
ES and TypeScript modules are strict by default and don’t need this
directive. ESLint will remind us to add it to new CommonJS files and
remove it from ES and TypeScript modules.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We remove the "GROUP PMs" section that used
to be in the lower right sidebar.
Most of this is straightforward code removal.
A couple quick notes:
- The message fetching code now just
calls `huddle_data.process_loaded_messages`,
which we still need for search suggestions.
We removed `activity.process_loaded_messages`.
- The `huddle_data.process_loaded_messages`
function no longer needs to return `need_resize`.
- In `resize.js` we now just calculate
`res.buddy_list_wrapper_max_height` directly
from `usable_height`.
This is mostly just moving code from the `activity.js`
tests, but I also now explicitly cover the "100%"
use case (i.e. all four folks in the huddle are present).
This makes it so that search_suggestion.js
does not depend on activity.js.
That dependency hasn't really been "elegant"
for quite some time, but it will become particularly
unnecessary when we go to remove the "Group PMs"
section from the right sidebar.
This commit introduces a temporary wart
where we have these two functions with the
same name in a sort of unnecessarily
complicated code stack:
activity.process_loaded_messages
huddle_data.process_loaded_messages
But we will eliminate the former function
very soon, and our message-related codepaths
will just call the `huddle_data` version
directly.
TESTING NOTES:
Now that `huddle_data` is a tiny leaf
module, it's super easy to just use the
real implementation of what was formerly
called `activity.get_huddles()` (and is
now in `huddle_data`).
When I first wrote this commit, introducing
the real implementation of `get_huddles` exposed
some bugs that I fixed in the immediately
prior commits to this.
When the tests were originally written,
I believe `activity.js` had some annoying
`jQuery` dependencies that made it hard
to unit test against. We've slimmed it over
time to be mostly just a "controller" module.
But even in its current state it would have
been a bit of a bloated dependency.
The other friction for using the actual
version of `get_huddles` was setting up
the message data, but that's pretty minor.
This exists in all versions of the desktop app that we still support,
and will eventually let us delete a bit of annoying compatibility code
from the desktop app’s injected JavaScript.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This flag was used to delay unread count updates while the bankruptcy
modal was visible. Now that bankrupcty is no longer a modal, we don't
need this flag at all.
We had this API:
people.add_in_realm = full-fledged user
people.add = not necessarily in realm
Now the API is this:
people.add = full-fledged user
people._add_user = internal API for cross-realm bots
and deactivated users
I think in most of our tests the distinction between
people.add() and people.add_in_realm() was just an
accident of history and didn't reflect any real intention.
And if I had to guess the intention in 99% of the cases,
folks probably thought they were just creating ordinary,
active users in the current realm.
In places where the distinction was obviously important
(because a test failed), I deactivated the user via
`people.deactivate`.
For the 'basics' test in the people test suite, I clean
up the test setup for Isaac. Before this commit I was
adding him first as a non-realm user then as a full-fledged
user, but this was contrived and confusing, and we
didn't really need it for test coverage purposes.
Explicitly stubbing i18n in 48 different files
is mostly busy work at this point, and it doesn't
provide much signal, since often it's invoked
only to satisfy transitive dependencies.
We now treat util like a leaf module and
use "require" to import it everywhere it's used.
An earlier version of this commit moved
util into our "shared" library, but we
decided to wait on that. Once we're ready
to do that, we should only need to do a
simple search/replace on various
require/zrequire statements plus a small
tweak to one of the custom linter checks.
It turns out we don't really need util.js
for our most immediate code-sharing goal,
which is to reuse our markdown code on
mobile. There's a little bit of cleanup
still remaining to break the dependency,
but it's minor.
The util module still calls the global
blueslip module in one place, but that
code is about to be removed in the next
few commits.
I am pretty confident that once we start
sharing things like the typeahead code
more aggressively, we'll start having
dependencies on util. The module is barely
more than 300 lines long, so we'll probably
just move the whole thing into shared
rather than break it apart. Also, we
can continue to nibble away at the
cruftier parts of the module.
We had a plan at some point to use this to display a phone icon or
something for users who would receive push notifications if you
messaged them. IT's not clear that feature was a good idea in any
case, but it certainly shouldn't be synced as presence data; it would
change >100x less often than the rest of presence and so should likely
be synced differently, maybe as a property on user. So it's best to
delete this prototype.
This required lots of manual testing:
- search/navigate user presence
- send PM and mention user
- pay attention to compose fade
- send stream msg and mention user
- open Private Messages in top-left and click
- test unread counts
- invite user who already has account
- search for users in search bar
- check user settings
- User Groups
- Users
- Deactivated Users
- Bots
- create a bot
- mention user groups
- send group PM then click on lower right
- view/edit/create streams
If there are still pieces of code that don't convert
ids to ints, the code should still work but report
blueslip errors.
I try to mostly convert user_ids to ints in the callers,
since often the callers are dealing with small amounts
of data, like user ids from huddles.
We now require the actual tests to explicitly
to zrequire Dict, rather than magically adding this.
In one case, the use of Dict was clearly just for
the test (not the app), so I converted that an ordinary
JS object (see timerender.js).
The compose_state.recipient field was only actually the recipient for
the message if it was a private_message_recipient (in the sense of
other code); we store the stream in compose_state.stream instead.
As a result, the name was quite confusing, resulting in the
possibility of problematic correctness bugs where code assumes this
field has a valid value for stream messages. Fix this by changing it
to compose_state.private_message_recipient for clarity.
This commit was originally automatically generated using `tools/lint
--only=eslint --fix`. It was then modified by tabbott to contain only
changes to a set of files that are unlikely to result in significant
merge conflicts with any open pull request, excluding about 20 files.
His plan is to merge the remaining changes with more precise care,
potentially involving merging parts of conflicting pull requests
before running the `eslint --fix` operation.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The count_span element is parented by a .selectable_sidebar_block element
which is parented by the li element that the class is supposed to be added
to. Thus, use the parents() jQuery method for locating the li parent so
that the class gets added to the correct element.
Combined with work in the desktop app, this makes it possible for the
desktop app to clearly indicate to other users whether the current
user is active on the system and thus would see a desktop
notification, not just whether they are active in the current Zulip
window.
Essentially rewritten by tabbott to add unit tests and consider the
desktop app data authoritative.
After migration to an ES6 module, `suppress_unread_counts` would no
longer be mutable from outside the module.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
With perfectScrollbar, we needed to call a function from JavaScript to
enable a scrollbar on a new element, but simplebar has a much simpler
default API one can do by using data-simplebar attributes in the HTML.
So we can delete all the scrollbar creation/deletion code.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
perfect-scrollbar replaces both the appearance and the behavior of the
scrollbar, and its emulated behavior will never feel native on most
platforms. SimpleBar customizes the appearance while preserving the
native behavior.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
We no longer store handlers as an array of functions,
and instead we assume that code will only ever set up
one handler per sel/event or sel/event/child. This is
almost always a sane policy for the app itself.
We also try to improve error handling when devs write
incorrect tests.
The only tests that required changes here are the
activity tests, which were a little careless about how
data got reset between tests.
Adds possibility for users to use | as an OR-operator (besides ,)
when searching for other users.
This is a thing reasonable folks might try, and | in the thing to
search for isn't a realisitic possibility, so there's no real downside
to adding this.
Fixes#4109.
This is mostly adding markup, calling some convenient
functions in buddy_data.js, and adjusting CSS.
To make the circles update dynamically, I mostly
orchestrate this though activity.js for now. It's
possible we'll want to adjust that eventually to
happen through something like a `presence_events`
dispatcher, but that's essentially what
a good part of `activity.js` does now.
We now have a function get_user_circle_class
that returns one of these values:
"user_circle_green"
"user_circle_orange"
"user_circle_empty"
And we put that in the templates.
And then CSS renders the circle of the appropriate
color.
The unit tests now explicitly capture whether
we are rendering the correct kind of circle.
In small screen sizes, when the user presses shortcut `w` to search
for another user, the hide_all function calls in the search code path
would hide the right sidebar, immediately after opening it, making the
hotkey basically unusable.
We fix this by extracting a separate hide method that hides all true
popovers, but not the user list sidebar.
Fixes#11463.
The name `insert_user_into_list` is sort of misleading,
since we are often just redrawing the user's existing
item in the buddy list.
I chose `redraw_user` over `update_user` to emphasize
that we're just going to redraw it with whatever data
has been updated by the callers.
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.
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
This is part of work to break some of our
nastier circular dependencies in preparation
for our es6 migration.
This commit should facilitate loading leaf-like
modules such as people.js before all of the things
that reload.js depends on.
We add a padded div to our container for the buddy
list to give scrolling the illusion that we've
rendered every list item, while still letting
the browser do the heavy lifting instead of trying
to fake it out too much.
This version of progressive scrolling lazily
renders buddy list items, but it doesn't
provide the browser with any notion of upcoming
list items, so as you scroll down and the size
of the rendered list grows, the scrollbar shows
you being too close to the bottom.
This maintains 100% coverage on buddy_list.js.
We now keep track of keys in buddy_list.js, so that
when we insert/remove items, we no longer need to
traverse all the DOM. Instead, we just find out
which position in the list we need to insert the
key in (where "key" is "user_id") and then find
the relevant DOM node directly and insert the new
HTML before that node. (And of course we still
account for the "append" case.)
There's a little more bookkeeping to make this
happen, but it should help reduce some code in
upcoming commits and pave the way toward
progressive rendering optimizations.
This commit should produce a minor speedup
for activity-related events that go through
buddy_list.insert_or_move(), since we are
not traversing the DOM to find insertion points
any more.
Anders Kaseorg
Steve Howell
Aman Agrawal
Priyank Patel
clarammdantas
Anders Kaseorg
Rohitt Vashishtha
Steve Howell
Tim Abbott
Cynthia Lin
vsvipul
Anders Kaseorg
Casper
Abhinav Singh
Shubham Dhama
Joshua Pan