We use css() pretty rarely in our codebase, and
it can sometimes be used mistakenly, when a better
alternative is to toggle a class for external css.
It's hard to support the full API in zjquery, so
we just punt and tell folks to create their own
stubs.
Most of the existing tests that were "fixed" here
weren't actually verifying the behavior of the css()
calls, and for those I just create no-op stubs.
In a few places I verify that css() was called as
expected.
Use fully resolvable request paths because we need to be able to refer
to third party modules, and to increase uniformity and explicitness.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
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.
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>
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>
Prior to commit eb4a2b9d4e the center
area of the navbar was based on a structure that appended crumbs or
"tabs" as <li>s, forming a tab_bar and a tab_list.
However, in eb4a2b9d4e we apply a new
style and structure to the navbar which lets go of the convention of
tabs. Hence, we'd like to purge the tab_bar and tab_list labels from
our code base.
We purged tab_list in 1267caf5009118875f47fdafe312880af08024e1.
This commit purges tab_bar, it includes:
- A blanket search and replace of tab_bar with message_view_header.
- Splitting a single line comment in
tab_bar.js / message_view_header.js.
- The renaming of tab_bar.js to message_view_header.js.
- The renaming of tab_bar.hbs to message_view_header.hbs.
- A blanket search and replace of tab_data with
message_view_header_data.
- Replacing the single occurrence of tabbar with message_view_header
(it was within a comment.)
$.fn.typeahead, on the other hand, returns the jQuery object back (not
the Typeahead object, which also happens to have a select method), so
this should be converted.
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>
This reverts commit 63643c9d9d.
As the commit mentions, it makes a UI change for legacy search which
has largely been considered a regression. We've been running with
this reverted in zulip.com essentially since it was first merged.
A main change is that we now display the navbar if the search box
is not focused. This was already present in the search pills version
but adding it to the legacy version is an improvement.
We sufficiently increase the timeout so that the pills are actually
deleted. This was required when `filter.is_common_narrow()` is true,
as then only we render the narrow description and close the search bar.
This commit also matches another behaviour of the legacy search.
i.e. We narrow every time a search suggestion is clicked.
The now redundant "focusin" and "focusout" event handler tests are
also removed.
This merges the `exports.get_search_result_legacy` and
`exports.get_search_result` function.
The key differences between the two code paths are as follows:
* We only want to generate suggestions for the queries which
the user is typing or can edit.
For the legacy version, suggestions are displayed for the
entire search string in the searchbox. (`all_operators`)
For the pills enabled version, suggestions are displayed
only for the input which hasn't been converted to pills.
(`query_operators`)
`all_operators` = `base_query_operators` + " " + `query_operators`.
trim is added at the end just to handle the legacy case
where we pass the `base_query` as ''.
* It is not possible to detect whether the user wants to
continue typing in the legacy version. However if the
the searchbox is still focused even after pill creation
we can assume the user still wants to continue typing.
To handle this we push an empty term as the `last` operator.
This is possible since the previous queries have been
completely entered as evident from it's generated pill.
* When using the legacy version, `search_operators` are
the same as `all_operators`, as mentioned in point 1.
In the pills enabled version we perform most of the
computations from the `query_operators`, but we do
require all `all_operators`, only for filtering the last
query's suggestion.
* And there is just one block unique to the legacy search
system. More details are mentioned in the comments of that
block.
We also refactor both the search suggestions node tests,
mainly to make them similar and easier to detect differences
when we switch over to the new version.
Previously, the navbar failed at managing the searchbox text state in
cases where, eg, the user performs navigation by browser history.
This commit resolves the issue by ensuring that the searchbox text is
only (and always) set when the searchbox is made visible, and as such
there is no "state" to manage and we will always display the correct
text.
It also adds a test in `search_legacy.js` to make sure that the search
text is placed as intended.
Fixes: #14771.
Now node_tests/search.js and node_tests/search_legacy.js have
almost identical test besides the search pills related data
processing work.
This is possible since we no longer depend on the values stored
in the input pills and can narrow or search for term based on
the `#search_query` box value.
This was done in commit 02ab48a61e.
This updates the logged-in top navbar to display the stream/message
name, number of users, and description. It also replaces the search
bar with a search icon that expands into a full-width search bar.
Co-authored-by: Max Nussenbaum <max@maxnuss.com>
Fixes: #164.
Fixes: #5198.
Once we have max_items results, stop trying
to get more items.
This should really help large realms when
you do a search on streams that turns up
more than N streams (where N is about 12).
We won't even bother to find people.
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.
This large function will need to be modified significantly as part of
the pills effort, and copying it lets us preserve behavior in
production until we're ready to cut things over.
This forks off search_legacy.js and search_suggestion_legacy.js so
that we can continue running automated tests against the legacy search
code while we develop the input pills feature.
Steve Howell
Riken Shah
Priyank Patel
Anders Kaseorg
YashRE42
Tim Abbott
Ryan Rehman
Steve Howell
Anders Kaseorg
Tim Abbott