This pulls the essential bucketing/sorting logic out
of filter_table().
The diff isn't quite as clean as I'd like, but some
of the code that got added back to filter_table() can be
eliminated in the future. Basically, all the stuff
related to hidden ids can just be zapped if we go
to an approach of just re-building the DOM cleanly
whenever our filters change.
We replace two calls to stream_matches_query() with
a single call to triage_stream(), which prevents us
from doing the same is-subscribed checks twice.
We probably should have done this a while ago, even
though these functions are pretty tiny. The goal here
is to make it easier to have more consistent search
semantics.
Our first use case is subs.js. In this case we
are able to decouple a bit of generic string
matching from the subs-specific code.
We move some data code from subs.js to stream_data.js.
It's not clear we have been using the optimal sort for
dealing with locales, but this change preserves the
current behavior. The only subtle change here is that
we look up subs using a Dict now instead of a plain
JS object.
The values of this dictionary used to be raw DOM elements,
but get_row() wraps them again, so there's not a huge
reason to store them as raw DOM elements internally. It
is slightly easier to reason about the code if everything
stays at the jQuery level.
To preserve the old behavior here, we have to do something
that is kind of ugly, but at least it's explicit now. In
the old code, our cache was DOM elements, and if an id
wasn't in the cache, we would sneakily return $(undefined)
with this code in get_row():
return $(this._rows[id]);
And it turns out that $(undefined) is basically just a
zero-element jQuery object. A lot of our code depends
on this behavior and just works around the zero-element
objects as needed with checks like this:
if (this.selected_row()).length === 0) {
// don't try to get offset
}
For now we just preserve this behavior. We could eventually
be more strict here, or at least have aggressive warnings
on cache misses, but we'd need to retrofit code to be
able to call something like `has_rendered_selection()`
and/or deal with `undefined` as the return value for the case
where the selection hasn't been rendered.
Here is some example code that would cause tracebacks if
we just returned `undefined` for cache misses:
rerender_preserving_scrolltop: function () {
// old_offset is the number of pixels between the top of the
// viewable window and the selected message
var old_offset;
var selected_row = this.selected_row();
var selected_in_view = selected_row.length > 0;
if (selected_in_view) {
old_offset = selected_row.offset().top;
}
return this.rerender_with_target_scrolltop(selected_row,
old_offset);
},
This function is more cohesive and always takes in
a jQuery object containing exactly one DOM element,
and it does all stuff at the jQuery level of
abstraction (no raw DOM).
It's a pretty simple extraction--removing the level
of indentation makes the diff a bit noisy.
We shorten the name of the function and avoid having
all the callers call `.get()`. Now we mostly stay
in jQuery "space", which avoids some confusion about
when we're dealing with raw DOM elements and which
will facilitate unit testing.
Currently on zoom out from stream topics, scrollbar didn't scroll back
to opened stream. Because call to scroll-to-stream func isn't called
after all streams view is displayed. So wrong stream element is
passed to func.
Fix this by calling scroll-to-stream func after all-stream-list view
is displayed.
We use these new functions in the message compose typeahead so that they
can also be used in a PM recipients typeahead with both people and user
groups.
We now render the "skin" part of "Stream Settings" before
adding in the actual streams. The new function
populate_stream_settings_left_panel() takes care of adding
the streams. It uses a new template called
`subscriptions.handlebars`.
Splitting out this function will give us more flexibility
for various improvements.
First, we can decide to render the list after we open the
overlay, just to avoid the problem that users don't know why
the modal's opening. (And we could add a loader spinner as
needed.)
Second, we can improve our filter features so that we do
filtering in the data instead of moving DOM rows around,
which is expensive.
Third, we can eventually introduce progressive rendering.
Finally, having the function broken out will make profiling
more precise about where bottlenecks exist.
We were passing this in before, but having it as
a data member reinforces the idea that we'll want
this to be a first-class concept in the list, since
we depend on ordering for various things.
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.
This will be useful for lazy rendering, where our
buddy_list widget already knows the keys (aka "userids")
it wants to render as you start scrolling them into
view.
Typing "tim " did not did not produce any match when suggesting person
in composebox typeahead or user group typeahead as the space at the
end of the "tim " string passed by the browser was a
`no break-space (U+00A0)` instead of `space (U+0020)`.
Although there are unicode characters other than `no break-space` which
represent spaces, only U+00A0 is replaced as it was the only space
character encountered when testing this issue manually.
Fixes#10039.
Currently, if you access an article link with an anchor link that isn't
featured in the sidebar, the main article won't be highlighted. Thus, we
exclude the anchor link hash from the article-searching selector if
the full article pathname wasn't found.
To reduce code duplication when creating hotkey deprecation notices,
create the `get_hotkey_deprecation_notice` function. Also, create a
`ui` testing file with a test for the new function.
Fix#10004.
This replaces some old code with calls to topic_data.js.
Now our topic typeahead uses the same data as our
sidebar, stream suggestions, and the "n" key, so any
future improvements to that data will benefit all
features the same.
This is an important piece of #9857.
Now that `emoji_collection` and `emojis_by_name` are global
datasources in the webapp we need to rename things carefully
to reflect their actual meaning. The fact that emoji code is
used as a css class for unicode emoji is one thing but it is
not its sole use so renaming it seems a good idea.
This commit moves the `emoji_collection` datasource in the emoji
picker to emoji.js and renames it to `emojis_by_name`. It is a
mapping from emoji name to object where each object describes an
emoji. This is an effort in the direction of de-duplicating and
unifying the datasets being used by various our widgets(like
emoji picker and composebox typeahead) in the webapp. Migrating
all the widgets to a single datasource will help us in removing
the whole class of annoying bugs which causes some emojis to be
missing from some widgets.
This commit closes a long pending issue which involved moving the
`EMOTICON_CONVERSION` mapping to build_emoji infrastructure so
that there is only one source of truth. This was pending from the
time when this feature was implemented.
Pressing `Esc` did not blur a contenteditable div by default, while
an input field was blurred by default. Due to this when a user tried
to unnarrow using `Esc` key when the searchbox had focus, the focus
remained stuck in the div itself and no further action was taken.
If search pills are not enabled, the text present in the search bar
will be selected on pressing '/' and writing someting without deselecting
the text will clear the search text. Since selecting the pills would
not make sense in this context, the search box is focused instead.
Adds box-shadow to `#searchbox` when either `#search_query` or any
of the pills have focus. Uses jquery instead of pure css as the
`:focus` event occurs on `#search_query`, while we want to add
box-shadow to `#searchbox`. This could have been done with
`:focus-within` CSS selector, but it is not supported in IE or Opera.
`#search_query` already had an onfocus/focusout listener, adding
listeners to `#searchbox.pills` for those events wouldn't have worked
as you don't want the focusout event to fire when the focus shifts
from input to pill.
Also adds `focusin`, `focusout` and `css()` to zjquery. `css` is
same as `val`, except it returns an empty object in case of no value
instead of an empty string. I don't think `css()` is valid syntax
in actual jquery.
After adding search pills, suggestions were based only on the
current input and no validation against the existing pills was done.
operator_subset_suggestions have been removed. Default suggestions
for base_operators have also been removed.
Handle multiple operators:
if `is:starred stream:Ver` was typed without selecting the typeahead
or pressing enter in between i.e search pill for is:starred has not yet
been added, then the description of `is:starred` will act as a prefix
in every suggestion.
Also makes changes re-enabling person suggestions for names with spaces.
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.
tab_bar.js becomes redundant after implementation of search pills.
This commit adds a comment to tab_bar.initiliaze, so the event
listeners related to it do not get initiated. This does not remove
any code related to tab_bar.js.
Also adds left and right border around the search icon.
Following points have been implemented in this commit:
1.) Add search pill on selecting typeahead.
2.) Re-narrow after removing a search pill.
3.) Add quiet optional parameter to removeLastPill.
4.) Pre populate search pills in narrow.activate.
5.) Clear existing search pills on narrow.deactivate.
Description of above points:
1.) I tried out using the description from suggestions.lookup_table
to append a pill using appendValidatedData so that the description
had not to be calculated again. But the description in the suggestions
lookup contains html due to highlighting. This html is escaped when
inputed in a pill. An attempt was also made to remove the higlighting
by replacing the tags. But other espaced characters like < also
popped up, so it was better to use append_search_string.
3.) If one wants to refresh the pill using pill.clear and wants to
repopulate them, evaluating the event_handler associated with the
action of removing the pill may not be desired.
4.) Pill population code is added to narrow.activate. Pills are not
populated if the narrow was triggered by search as search handles the
addition and removal of pill by itself. The reason for not handling
search too in narrow.activate is to avoid clearing the pills and
repopulating them. Example of some of the triggers for narrow.activate
include `restore draft`, `topic change`,`sidebar`.
Also modifies tests for search.js
Adds an optional parameter `quiet` to removeLastPill and removeAllPills.
If `quiet` is a truthy value, the event handler associated with the
pill will not be evaluated. This is useful when using clear to reset
the pills.
Also adds the file to the static asset pipeline.
search_pill_widget.js will be used to access the pills object for
the search query box. It will act in a similar way to
compose_pm_pill.js. Why is this needed: Consider you've initiated
a pills object in search.js for the search query box. Now you want to
also access that pills object to pre-populate pills after a reload in
hashchange.js. search_pill_widget.js makes this easy without the use
of events.
When Pan & Zoom (canvas) is enabled, the `v` hotkey does not work due to
`LightboxCanvas` overriding the `keydown` event. Add `v` as an option in
the new listener.
Fix#9777.
This commit updates the `emoji-datasource` packages to version 4.0.4.
This update brings following changes to emoji infra:
1: Fix for the bleeding sprite sheets.
2: The category of some emojis has been changed. Categorywise breakup of
net gain or loss is as follows:
Travel & Places: 58 (gain)
Symbols: 47 (loss)
Smileys & People: 52 (gain)
Objects: 11 (loss)
Food & Drink: 3 (gain)
Animals and Nature: 46 (gain)
Activities: 9 (loss)
3: There were some changes in the image farm of the package which were
breaking our old emoji farm. I fixed them by modifying the remapped
emoji map.
Fixes: #8235.
This behavior was originally implemented in commit 6993f89, but due to not
specifying a toggle option, the Subscribed/All streams switcher tab was
focused after the input was focused, leading to the input's loss of focus.
Fixes#9981.
Steve Howell
Yashashvi Dave
Cynthia Lin
Shubham Padia
Marco Burstein
Harshit Bansal
Tim Abbott