Previously, opening multiple message_edits and then drag-dropping a
file into any one of them would cause all of them to upload ie you'd
get one uploaded file in each message_edit.
This bug was caused by returning multiple elements from
upload.get_item("drag_drop_container", config) when config.mode =
"edit".
This commit changes the selector to use the row provided (config.row),
and so ensures that the above bug doesn't happen.
We convert the following elements to use a class instead of
id for accessing them across the codebase:
* markdown_preview
* undo_markdown_preview
* markdown_preview_spinner
* message_edit_content
* preview_content
Converted them together since changes to one impacted the other in
some modules like click_handlers.
Also, added a function in rows to get `message_row`.
In both compose and `message_edit_form` we use `file_input`
class to identify the file `input` element. This will help
to more easily share common elements between compose and message_edit.
Since we can have multiple instances of `message_edit_form`, it
makes sense to have it as a class.
We track the message_edit_form by setting an id to
`form` element dependent on message_id.
For most functions that we were using __Rewire__ for,
it's better to just use the override helper, which
use __Rewire__ under the hood, but also resets
the reference at the end of run_tests.
Another nice thing about override() is that it reports
when you never actually needed the mock, and this
commit fixes the instances found here.
I didn't replace every call to __Rewire__. The
remaining ones fall under these categories:
* I looked for ") =>" in my code sweep,
so I missed stuff like "noop" helpers.
* Sometimes we directly update something
in a module that's not a function. This
is generally evil, and we should use setters.
* Some tests have setup() helpers or similar
that complicated this code sweep, so I
simply punted.
* Somes modules rely on intra-test leaks. We
should fix those, but I just punted for the
main code sweep.
We now just use a module._load hook to inject
stubs into our code.
For conversion purposes I temporarily maintain
the API of rewiremock, apart from the enable/disable
pieces, but I will make a better wrapper in an
upcoming commit.
We can detect when rewiremock is called after
zrequire now, and I fix all the violations in
this commit, mostly by using override.
We can also detect when a mock is needlessly
created, and I fix all the violations in this
commit.
The one minor nuisance that this commit introduces
is that you can only stub out modules in the Zulip
source tree, which is now static/js. This should
not really be a problem--there are usually better
techniques to deal with third party depenencies.
In the prior commit I show a typical workaround,
which is to create a one-line wrapper in your
test code. It's often the case that you can simply
use override(), as well.
In passing I kill off `reset_modules`, and I
eliminated the second argument to zrequire,
which dates back to pre-es6 days.
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>
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>
On uploading a few files from markdown_preview mode of compose box and
then switching back to edit mode, the compose box doesn't get resized.
It even doesn't allow to scroll through the content.
Fixed this by switching back to the edit mode everytime user uploads
some file in markdown_preview mode as there's no use of staying in
markdown_preview mode anyways after uploading a file as the preview
doesn't get updated.
Also, updated the corresponding test cases.
Fixes: #16296.
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>
YashRE42
Anders Kaseorg
Steve Howell
Riken Shah
Vishnu KS
Aman Agrawal
Priyansh Garg