Make search suggestion messages more concise.

This involves updating filter.js, mostly. The
tests were updated appropriately for this change,
which also involved changing a caspar test for
narrowing.
This commit is contained in:
Cory Lynch 2017-06-05 20:28:12 -04:00 committed by Tim Abbott
parent ab5b0e938d
commit 89b8d07420
5 changed files with 54 additions and 40 deletions

View File

@ -253,28 +253,28 @@ expect_home();
// Narrow by typing in search strings or operators. // Narrow by typing in search strings or operators.
// Test stream / recipient autocomplete in the search bar // Test stream / recipient autocomplete in the search bar
search_and_check('Verona', 'Narrow to stream', expect_stream, search_and_check('Verona', 'Stream', expect_stream,
'Verona - Zulip Dev - Zulip'); 'Verona - Zulip Dev - Zulip');
search_and_check('Cordelia', 'Narrow to private', expect_1on1, search_and_check('Cordelia', 'Private', expect_1on1,
'private - Zulip Dev - Zulip'); 'private - Zulip Dev - Zulip');
// Test operators // Test operators
search_and_check('stream:Verona', 'Narrow', expect_stream, search_and_check('stream:Verona', '', expect_stream,
'Verona - Zulip Dev - Zulip'); 'Verona - Zulip Dev - Zulip');
search_and_check('stream:Verona subject:frontend+test', 'Narrow', expect_stream_subject, search_and_check('stream:Verona subject:frontend+test', '', expect_stream_subject,
'frontend test - Zulip Dev - Zulip'); 'frontend test - Zulip Dev - Zulip');
search_and_check('stream:Verona topic:frontend+test', 'Narrow', expect_stream_subject, search_and_check('stream:Verona topic:frontend+test', '', expect_stream_subject,
'frontend test - Zulip Dev - Zulip'); 'frontend test - Zulip Dev - Zulip');
search_and_check('subject:frontend+test', 'Narrow', expect_subject, search_and_check('subject:frontend+test', '', expect_subject,
'home - Zulip Dev - Zulip'); 'home - Zulip Dev - Zulip');
search_silent_user('sender:emailgateway@zulip.com', 'Narrow'); search_silent_user('sender:emailgateway@zulip.com', '');
search_non_existing_user('sender:dummyuser@zulip.com', 'Narrow'); search_non_existing_user('sender:dummyuser@zulip.com', '');
// Narrow by clicking the left sidebar. // Narrow by clicking the left sidebar.
casper.then(function () { casper.then(function () {

View File

@ -464,6 +464,10 @@ function make_sub(name, stream_id) {
{operator: 'search', operand: ':stream: -:emoji: are cool'}, {operator: 'search', operand: ':stream: -:emoji: are cool'},
]; ];
_test(); _test();
string = '';
operators = [];
_test();
}()); }());
(function test_unparse() { (function test_unparse() {
@ -489,6 +493,12 @@ function make_sub(name, stream_id) {
]; ];
string = 'near:150'; string = 'near:150';
assert.deepEqual(Filter.unparse(operators), string); assert.deepEqual(Filter.unparse(operators), string);
operators = [
{operator: '', operand: ''},
];
string = '';
assert.deepEqual(Filter.unparse(operators), string);
}()); }());
(function test_describe() { (function test_describe() {
@ -499,85 +509,86 @@ function make_sub(name, stream_id) {
{operator: 'stream', operand: 'devel'}, {operator: 'stream', operand: 'devel'},
{operator: 'is', operand: 'starred'}, {operator: 'is', operand: 'starred'},
]; ];
string = 'Narrow to stream devel, Narrow to starred messages'; string = 'stream devel, starred messages';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'stream', operand: 'devel'}, {operator: 'stream', operand: 'devel'},
{operator: 'topic', operand: 'JS'}, {operator: 'topic', operand: 'JS'},
]; ];
string = 'Narrow to devel > JS'; string = 'stream devel > JS';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'is', operand: 'private'}, {operator: 'is', operand: 'private'},
{operator: 'search', operand: 'lunch'}, {operator: 'search', operand: 'lunch'},
]; ];
string = 'Narrow to all private messages, Search for lunch'; string = 'private messages, search for lunch';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'id', operand: 99}, {operator: 'id', operand: 99},
]; ];
string = 'Narrow to message ID 99'; string = 'message ID 99';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'in', operand: 'home'}, {operator: 'in', operand: 'home'},
]; ];
string = 'Narrow to messages in home'; string = 'messages in home';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'is', operand: 'mentioned'}, {operator: 'is', operand: 'mentioned'},
]; ];
string = 'Narrow to mentioned messages'; string = '@-mentions';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'is', operand: 'alerted'}, {operator: 'is', operand: 'alerted'},
]; ];
string = 'Narrow to alerted messages'; string = 'alerted messages';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'is', operand: 'something_we_do_not_support'}, {operator: 'is', operand: 'something_we_do_not_support'},
]; ];
string = 'Narrow to (unknown operator)'; string = 'unknown operand';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
// this should be unreachable, but just in case
narrow = [ narrow = [
{operator: 'bogus', operand: 'foo'}, {operator: 'bogus', operand: 'foo'},
]; ];
string = 'Narrow to (unknown operator)'; string = 'unknown operand';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'stream', operand: 'devel'}, {operator: 'stream', operand: 'devel'},
{operator: 'topic', operand: 'JS', negated: true}, {operator: 'topic', operand: 'JS', negated: true},
]; ];
string = 'Narrow to stream devel, Exclude topic JS'; string = 'stream devel, exclude topic JS';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'is', operand: 'private'}, {operator: 'is', operand: 'private'},
{operator: 'search', operand: 'lunch', negated: true}, {operator: 'search', operand: 'lunch', negated: true},
]; ];
string = 'Narrow to all private messages, Exclude lunch'; string = 'private messages, exclude lunch';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'stream', operand: 'devel'}, {operator: 'stream', operand: 'devel'},
{operator: 'is', operand: 'starred', negated: true}, {operator: 'is', operand: 'starred', negated: true},
]; ];
string = 'Narrow to stream devel, Exclude starred messages'; string = 'stream devel, exclude starred messages';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = [ narrow = [
{operator: 'stream', operand: 'devel'}, {operator: 'stream', operand: 'devel'},
{operator: 'has', operand: 'image', negated: true}, {operator: 'has', operand: 'image', negated: true},
]; ];
string = 'Narrow to stream devel, Exclude messages with one or more image'; string = 'stream devel, exclude messages with one or more image';
assert.equal(Filter.describe(narrow), string); assert.equal(Filter.describe(narrow), string);
narrow = []; narrow = [];

View File

@ -422,7 +422,7 @@ init();
assert.equal(describe('is:mentioned'), '@-mentions'); assert.equal(describe('is:mentioned'), '@-mentions');
assert.equal(describe('is:alerted'), 'Alerted messages'); assert.equal(describe('is:alerted'), 'Alerted messages');
assert.equal(describe('sender:bob@zulip.com'), 'Sent by me'); assert.equal(describe('sender:bob@zulip.com'), 'Sent by me');
assert.equal(describe('stream:devel'), 'Narrow to stream <strong>devel</strong>'); assert.equal(describe('stream:devel'), 'Stream <strong>devel</strong>');
}()); }());
(function test_sent_by_me_suggestions() { (function test_sent_by_me_suggestions() {
@ -557,7 +557,7 @@ init();
return suggestions.lookup_table[q].description; return suggestions.lookup_table[q].description;
} }
assert.equal(describe('te'), "Search for te"); assert.equal(describe('te'), "Search for te");
assert.equal(describe('stream:office topic:team'), "Narrow to office > team"); assert.equal(describe('stream:office topic:team'), "Stream office > team");
suggestions = search.get_suggestions('topic:staplers stream:office'); suggestions = search.get_suggestions('topic:staplers stream:office');
expected = [ expected = [
@ -718,9 +718,9 @@ init();
return suggestions.lookup_table[q].description; return suggestions.lookup_table[q].description;
} }
assert.equal(describe('pm-with:ted@zulip.com'), assert.equal(describe('pm-with:ted@zulip.com'),
"Narrow to private messages with <strong>Te</strong>d Smith &lt;<strong>te</strong>d@zulip.com&gt;"); "Private messages with <strong>Te</strong>d Smith &lt;<strong>te</strong>d@zulip.com&gt;");
assert.equal(describe('sender:ted@zulip.com'), assert.equal(describe('sender:ted@zulip.com'),
"Narrow to messages sent by <strong>Te</strong>d Smith &lt;<strong>te</strong>d@zulip.com&gt;"); "Messages sent by <strong>Te</strong>d Smith &lt;<strong>te</strong>d@zulip.com&gt;");
suggestions = search.get_suggestions('Ted '); // note space suggestions = search.get_suggestions('Ted '); // note space

View File

@ -200,8 +200,6 @@ Filter.canonicalize_term = function (opts) {
}; };
}; };
/* We use a variant of URI encoding which looks reasonably /* We use a variant of URI encoding which looks reasonably
nice and still handles unambiguously cases such as nice and still handles unambiguously cases such as
spaces in operands. spaces in operands.
@ -419,10 +417,10 @@ Filter.operator_to_prefix = function (operator, negated) {
var verb; var verb;
if (operator === 'search') { if (operator === 'search') {
return negated ? 'Exclude' : 'Search for'; return negated ? 'exclude' : 'search for';
} }
verb = negated ? 'Exclude ' : 'Narrow to '; verb = negated ? 'exclude ' : '';
switch (operator) { switch (operator) {
case 'stream': case 'stream':
@ -474,7 +472,7 @@ Filter.describe = function (operators) {
if (is(operators[0], 'stream') && is(operators[1], 'topic')) { if (is(operators[0], 'stream') && is(operators[1], 'topic')) {
var stream = operators[0].operand; var stream = operators[0].operand;
var topic = operators[1].operand; var topic = operators[1].operand;
var part = 'Narrow to ' + stream + ' > ' + topic; var part = "stream " + stream + ' > ' + topic;
parts = [part]; parts = [part];
operators = operators.slice(2); operators = operators.slice(2);
} }
@ -484,13 +482,13 @@ Filter.describe = function (operators) {
var operand = elem.operand; var operand = elem.operand;
var canonicalized_operator = Filter.canonicalize_operator(elem.operator); var canonicalized_operator = Filter.canonicalize_operator(elem.operator);
if (canonicalized_operator ==='is') { if (canonicalized_operator ==='is') {
var verb = elem.negated ? 'Exclude ' : 'Narrow to '; var verb = elem.negated ? 'exclude ' : '';
if (operand === 'private') { if (operand === 'private') {
return verb + 'all private messages'; return verb + 'private messages';
} else if (operand === 'starred') { } else if (operand === 'starred') {
return verb + 'starred messages'; return verb + 'starred messages';
} else if (operand === 'mentioned') { } else if (operand === 'mentioned') {
return verb + 'mentioned messages'; return verb + '@-mentions';
} else if (operand === 'alerted') { } else if (operand === 'alerted') {
return verb + 'alerted messages'; return verb + 'alerted messages';
} }
@ -501,7 +499,7 @@ Filter.describe = function (operators) {
return prefix_for_operator + ' ' + operand; return prefix_for_operator + ' ' + operand;
} }
} }
return 'Narrow to (unknown operator)'; return "unknown operand";
}); });
return parts.concat(more_parts).join(', '); return parts.concat(more_parts).join(', ');
}; };

View File

@ -69,7 +69,7 @@ function get_stream_suggestions(last, operators) {
streams = typeahead_helper.sorter(query, streams); streams = typeahead_helper.sorter(query, streams);
var objs = _.map(streams, function (stream) { var objs = _.map(streams, function (stream) {
var prefix = 'Narrow to stream'; var prefix = 'stream';
var highlighted_stream = typeahead_helper.highlight_query_in_phrase(query, stream); var highlighted_stream = typeahead_helper.highlight_query_in_phrase(query, stream);
var description = prefix + ' ' + highlighted_stream; var description = prefix + ' ' + highlighted_stream;
var term = { var term = {
@ -359,7 +359,7 @@ function get_special_filter_suggestions(last, operators) {
var suggestions = [ var suggestions = [
{ {
search_string: 'in:all', search_string: 'in:all',
description: 'All messages', description: 'all messages',
invalid: [ invalid: [
{operator: 'in'}, {operator: 'in'},
{operator: 'stream'}, {operator: 'stream'},
@ -369,7 +369,7 @@ function get_special_filter_suggestions(last, operators) {
}, },
{ {
search_string: 'is:private', search_string: 'is:private',
description: 'Private messages', description: 'private messages',
invalid: [ invalid: [
{operator: 'is', operand: 'private'}, {operator: 'is', operand: 'private'},
{operator: 'stream'}, {operator: 'stream'},
@ -380,7 +380,7 @@ function get_special_filter_suggestions(last, operators) {
}, },
{ {
search_string: 'is:starred', search_string: 'is:starred',
description: 'Starred messages', description: 'starred messages',
invalid: [ invalid: [
{operator: 'is', operand: 'starred'}, {operator: 'is', operand: 'starred'},
], ],
@ -394,7 +394,7 @@ function get_special_filter_suggestions(last, operators) {
}, },
{ {
search_string: 'is:alerted', search_string: 'is:alerted',
description: 'Alerted messages', description: 'alerted messages',
invalid: [ invalid: [
{operator: 'is', operand: 'alerted'}, {operator: 'is', operand: 'alerted'},
], ],
@ -424,7 +424,7 @@ function get_sent_by_me_suggestions(last, operators) {
var last_string = Filter.unparse([last]).toLowerCase(); var last_string = Filter.unparse([last]).toLowerCase();
var sender_query = 'sender:' + people.my_current_email(); var sender_query = 'sender:' + people.my_current_email();
var from_query = 'from:' + people.my_current_email(); var from_query = 'from:' + people.my_current_email();
var description = 'Sent by me'; var description = 'sent by me';
var invalid = [ var invalid = [
{operator: 'sender'}, {operator: 'sender'},
{operator: 'from'}, {operator: 'from'},
@ -545,6 +545,11 @@ exports.get_suggestions = function (query) {
suggestions = get_operator_subset_suggestions(operators); suggestions = get_operator_subset_suggestions(operators);
result = result.concat(suggestions); result = result.concat(suggestions);
_.each(result, function (sug) {
var first = sug.description.charAt(0).toUpperCase();
sug.description = first + sug.description.slice(1);
});
// Typeahead expects us to give it strings, not objects, so we maintain our own hash // Typeahead expects us to give it strings, not objects, so we maintain our own hash
// back to our objects, and we also filter duplicates here. // back to our objects, and we also filter duplicates here.
var lookup_table = {}; var lookup_table = {};