mirror of https://github.com/zulip/zulip.git
search: Simplify `narrow_or_search_for_term` code path.
The main reasoning for this change is as follows: * When the search bar contains multiple search queries but no search results, the last search operand does not get displayed. This happens due to the fact that filter object contained 2 terms having the operator key value as "search" instead of a single term where operator is "search" and operand is a single string containing the space seperated search queries. This condition occurs for search_pills_enabled case only because we used to Filter.parse the query twice (once for the `base_operators` and once for the `suggestion_operator instead of doing both at once). Thus the `search_query` value inside the `narrow.show_search_query` function which only selected the operands of the first term displayed an incomplete result. * Another benefit of this commit is to display the narrow operators in the URL fragment the same way as when search_pills_enabled = False. For example, On entering the queries in the mentioned order -> 'is: starred', 'abc', 'def', 'is: private', 'ghi'. This is the URL: Previously: /#narrow/is/starred/is/private/search/abc.20def/search/ghi Now (same as pills disabled case): /#narrow/is/starred/is/private/search/abc.20def.20ghi * We are also able to de-duplicate the non-typeahead search query code path.
This commit is contained in:
parent
d3f2bbc4bb
commit
02ab48a61e
|
@ -20,6 +20,7 @@ set_global('ui_util', {
|
||||||
set_global('narrow', {});
|
set_global('narrow', {});
|
||||||
|
|
||||||
search_pill.append_search_string = noop;
|
search_pill.append_search_string = noop;
|
||||||
|
search_pill.get_search_string_for_current_filter = noop;
|
||||||
|
|
||||||
global.patch_builtin('setTimeout', func => func());
|
global.patch_builtin('setTimeout', func => func());
|
||||||
|
|
||||||
|
@ -139,7 +140,7 @@ run_test('initizalize', () => {
|
||||||
assert.deepEqual(options, {trigger: 'search'});
|
assert.deepEqual(options, {trigger: 'search'});
|
||||||
};
|
};
|
||||||
search_pill.get_search_string_for_current_filter = () => {
|
search_pill.get_search_string_for_current_filter = () => {
|
||||||
return '';
|
return search_box_val;
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -150,7 +151,7 @@ run_test('initizalize', () => {
|
||||||
}];
|
}];
|
||||||
_setup('ver');
|
_setup('ver');
|
||||||
opts.updater('ver');
|
opts.updater('ver');
|
||||||
assert(is_blurred);
|
assert(!is_blurred);
|
||||||
assert(is_append_search_string_called);
|
assert(is_append_search_string_called);
|
||||||
|
|
||||||
operators = [{
|
operators = [{
|
||||||
|
@ -160,7 +161,7 @@ run_test('initizalize', () => {
|
||||||
}];
|
}];
|
||||||
_setup('stream:Verona');
|
_setup('stream:Verona');
|
||||||
opts.updater('stream:Verona');
|
opts.updater('stream:Verona');
|
||||||
assert(is_blurred);
|
assert(!is_blurred);
|
||||||
assert(is_append_search_string_called);
|
assert(is_append_search_string_called);
|
||||||
|
|
||||||
search.is_using_input_method = true;
|
search.is_using_input_method = true;
|
||||||
|
@ -206,7 +207,7 @@ run_test('initizalize', () => {
|
||||||
assert.deepEqual(options, {trigger: 'search'});
|
assert.deepEqual(options, {trigger: 'search'});
|
||||||
};
|
};
|
||||||
search_pill.get_search_string_for_current_filter = () => {
|
search_pill.get_search_string_for_current_filter = () => {
|
||||||
return '';
|
return search_box_val;
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
// Exported for unit testing
|
// Exported for unit testing
|
||||||
exports.is_using_input_method = false;
|
exports.is_using_input_method = false;
|
||||||
|
|
||||||
function narrow_or_search_for_term(search_string) {
|
exports.narrow_or_search_for_term = function (search_string) {
|
||||||
const search_query_box = $("#search_query");
|
const search_query_box = $("#search_query");
|
||||||
if (exports.is_using_input_method) {
|
if (exports.is_using_input_method) {
|
||||||
// Neither narrow nor search when using input tools as
|
// Neither narrow nor search when using input tools as
|
||||||
|
@ -13,14 +13,12 @@ function narrow_or_search_for_term(search_string) {
|
||||||
|
|
||||||
let operators;
|
let operators;
|
||||||
if (page_params.search_pills_enabled) {
|
if (page_params.search_pills_enabled) {
|
||||||
// search_string only contains the suggestion selected
|
// We have to take care to append the new pill before calling this
|
||||||
// from the typeahead. base_query stores the query
|
// function, so that the base_query includes the suggestion selected
|
||||||
// corresponding to the existing pills.
|
// along with query corresponding to the existing pills.
|
||||||
const base_query = search_pill.get_search_string_for_current_filter(
|
const base_query = search_pill.get_search_string_for_current_filter(
|
||||||
search_pill_widget.widget);
|
search_pill_widget.widget);
|
||||||
const base_operators = Filter.parse(base_query);
|
operators = Filter.parse(base_query);
|
||||||
const suggestion_operator = Filter.parse(search_string);
|
|
||||||
operators = base_operators.concat(suggestion_operator);
|
|
||||||
} else {
|
} else {
|
||||||
operators = Filter.parse(search_string);
|
operators = Filter.parse(search_string);
|
||||||
}
|
}
|
||||||
|
@ -32,9 +30,11 @@ function narrow_or_search_for_term(search_string) {
|
||||||
|
|
||||||
// Narrowing will have already put some operators in the search box,
|
// Narrowing will have already put some operators in the search box,
|
||||||
// so leave the current text in.
|
// so leave the current text in.
|
||||||
|
if (!page_params.search_pills_enabled) {
|
||||||
search_query_box.blur();
|
search_query_box.blur();
|
||||||
return search_query_box.val();
|
|
||||||
}
|
}
|
||||||
|
return search_query_box.val();
|
||||||
|
};
|
||||||
|
|
||||||
function update_buttons_with_focus(focused) {
|
function update_buttons_with_focus(focused) {
|
||||||
const search_query_box = $('#search_query');
|
const search_query_box = $('#search_query');
|
||||||
|
@ -90,22 +90,11 @@ exports.initialize = function () {
|
||||||
return true;
|
return true;
|
||||||
},
|
},
|
||||||
updater: function (search_string) {
|
updater: function (search_string) {
|
||||||
// Order is important here. narrow_or_search_for_term
|
|
||||||
// gets a search string from existing pills and obtains
|
|
||||||
// existing operators. Newly selected suggestion is added
|
|
||||||
// to those operators. If narrow_or_search_for_term was
|
|
||||||
// called after append_search_string, the existing search
|
|
||||||
// pills at the time for calling that function would also
|
|
||||||
// have the newly selected suggestion, and appending it again
|
|
||||||
// would cause duplication.
|
|
||||||
const result = narrow_or_search_for_term(search_string);
|
|
||||||
if (page_params.search_pills_enabled) {
|
if (page_params.search_pills_enabled) {
|
||||||
search_pill.append_search_string(search_string,
|
search_pill.append_search_string(search_string,
|
||||||
search_pill_widget.widget);
|
search_pill_widget.widget);
|
||||||
$("#search_query").focus();
|
|
||||||
} else {
|
|
||||||
return result;
|
|
||||||
}
|
}
|
||||||
|
return exports.narrow_or_search_for_term(search_string);
|
||||||
},
|
},
|
||||||
sorter: function (items) {
|
sorter: function (items) {
|
||||||
return items;
|
return items;
|
||||||
|
@ -150,16 +139,8 @@ exports.initialize = function () {
|
||||||
// this codepath is that they first all blur the box to
|
// this codepath is that they first all blur the box to
|
||||||
// indicate that they've done what they need to do)
|
// indicate that they've done what they need to do)
|
||||||
|
|
||||||
let operators = Filter.parse(search_query_box.val());
|
|
||||||
if (page_params.search_pills_enabled) {
|
|
||||||
// Pill is already added during keydown event of input pills.
|
// Pill is already added during keydown event of input pills.
|
||||||
// Thus we can't call narrow_or_search_for_term as the
|
exports.narrow_or_search_for_term(search_query_box.val());
|
||||||
// search_query_box has empty value.
|
|
||||||
const base_query = search_pill.get_search_string_for_current_filter(
|
|
||||||
search_pill_widget.widget);
|
|
||||||
operators = Filter.parse(base_query);
|
|
||||||
}
|
|
||||||
narrow.activate(operators, {trigger: 'search'});
|
|
||||||
search_query_box.blur();
|
search_query_box.blur();
|
||||||
update_buttons_with_focus(false);
|
update_buttons_with_focus(false);
|
||||||
}
|
}
|
||||||
|
|
|
@ -6,9 +6,7 @@ exports.initialize = function () {
|
||||||
exports.widget = search_pill.create_pills(container);
|
exports.widget = search_pill.create_pills(container);
|
||||||
|
|
||||||
exports.widget.onPillRemove(function () {
|
exports.widget.onPillRemove(function () {
|
||||||
const base_query = search_pill.get_search_string_for_current_filter(exports.widget);
|
search.narrow_or_search_for_term();
|
||||||
const operators = Filter.parse(base_query);
|
|
||||||
narrow.activate(operators, {trigger: 'search'});
|
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue