From 65d8eb3189f2da28cfaa76d300fc8789d8c9df72 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Thu, 19 Apr 2018 15:47:41 +0000 Subject: [PATCH] buddy list: Extract user_search.js. This was a bit more than moving code. I extracted the following things: $widget (and three helper methods) $input text() empty() expand_column close_widget activity.clear_highlight There was a minor bug before this commit, where we were inconsistent about trimming spaces. The introduction of text() and empty() should prevent bugs where users type the space bar into search. --- .eslintrc.json | 1 + frontend_tests/node_tests/activity.js | 51 ++++++----- static/js/activity.js | 117 ++++++++------------------ static/js/user_search.js | 117 ++++++++++++++++++++++++++ tools/test-js-with-node | 1 + zproject/settings.py | 1 + 6 files changed, 182 insertions(+), 106 deletions(-) create mode 100644 static/js/user_search.js diff --git a/.eslintrc.json b/.eslintrc.json index f4cd608da8..91131abfc6 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -143,6 +143,7 @@ "tab_bar": false, "emoji": false, "presence": false, + "user_search": false, "buddy_data": false, "buddy_list": false, "activity": false, diff --git a/frontend_tests/node_tests/activity.js b/frontend_tests/node_tests/activity.js index 6b96bb8a09..0d746cc85a 100644 --- a/frontend_tests/node_tests/activity.js +++ b/frontend_tests/node_tests/activity.js @@ -38,6 +38,7 @@ zrequire('presence'); zrequire('people'); zrequire('buddy_data'); zrequire('buddy_list'); +zrequire('user_search'); zrequire('activity'); zrequire('stream_list'); @@ -272,26 +273,31 @@ presence.presence_info[norbert.user_id] = { status: activity.ACTIVE }; presence.presence_info[zoe.user_id] = { status: activity.ACTIVE }; presence.presence_info[me.user_id] = { status: activity.ACTIVE }; -activity.set_user_list_filter(); - const user_order = [fred.user_id, jill.user_id, norbert.user_id, zoe.user_id, alice.user_id, mark.user_id]; const user_count = 6; -// Mock the jquery is func -$('.user-list-filter').is = function (sel) { - if (sel === ':focus') { - return $('.user-list-filter').is_focused(); - } -}; +function reset_jquery() { + set_global('$', global.make_zjquery()); + activity.set_user_list_filter(); -// Mock the jquery first func -$('#user_presences li.user_sidebar_entry.narrow-filter').first = function () { - return $('li.user_sidebar_entry[data-user-id="' + user_order[0] + '"]'); -}; -$('#user_presences li.user_sidebar_entry.narrow-filter').last = function () { - return $('li.user_sidebar_entry[data-user-id="' + user_order[user_count - 1] + '"]'); -}; + // Mock the jquery is func + $('.user-list-filter').is = function (sel) { + if (sel === ':focus') { + return $('.user-list-filter').is_focused(); + } + }; + + // Mock the jquery first func + $('#user_presences li.user_sidebar_entry.narrow-filter').first = function () { + return $('li.user_sidebar_entry[data-user-id="' + user_order[0] + '"]'); + }; + $('#user_presences li.user_sidebar_entry.narrow-filter').last = function () { + return $('li.user_sidebar_entry[data-user-id="' + user_order[user_count - 1] + '"]'); + }; +} + +reset_jquery(); (function test_presence_list_full_update() { $('.user-list-filter').focus(); @@ -396,6 +402,8 @@ $('#user_presences li.user_sidebar_entry.narrow-filter').last = function () { assert.equal(value.text(), ''); }()); +reset_jquery(); + (function test_key_input() { let sel_index = 0; // Returns which element is selected @@ -439,8 +447,6 @@ $('#user_presences li.user_sidebar_entry.narrow-filter').last = function () { $('#user_presences li.user_sidebar_entry.narrow-filter').length = user_count; - activity.set_user_list_filter_handlers(); - // Disable scrolling into place stream_list.scroll_element_into_container = function () {}; // up @@ -635,8 +641,7 @@ presence.presence_info[zoe.user_id] = { status: activity.ACTIVE }; }()); // Reset jquery here. -set_global('$', global.make_zjquery()); -activity.set_user_list_filter(); +reset_jquery(); (function test_insert_unfiltered_user_with_filter() { // This test only tests that we do not explode when @@ -670,9 +675,9 @@ $('.user-list-filter').parent = function () { (function test_clear_search() { $('.user-list-filter').val('somevalue'); - activity.clear_search(); + activity.user_filter.clear_search(); assert.equal($('.user-list-filter').val(), ''); - activity.clear_search(); + activity.user_filter.clear_search(); assert($('#user-list .input-append').hasClass('notdisplayed')); }()); @@ -703,13 +708,13 @@ $('.user-list-filter').parent = function () { }()); (function test_toggle_filter_display() { - activity.toggle_filter_displayed(); + activity.user_filter.toggle_filter_displayed(); assert($('#user-list .input-append').hasClass('notdisplayed')); $('.user-list-filter').closest = function (selector) { assert.equal(selector, ".app-main [class^='column-']"); return $.create('sidebar').addClass('column-right'); }; - activity.toggle_filter_displayed(); + activity.user_filter.toggle_filter_displayed(); assert.equal($('#user-list .input-append').hasClass('notdisplayed'), false); }()); diff --git a/static/js/activity.js b/static/js/activity.js index 660d2bd888..b7b5f5456b 100644 --- a/static/js/activity.js +++ b/static/js/activity.js @@ -14,8 +14,6 @@ var ACTIVE_PING_INTERVAL_MS = 50 * 1000; exports.ACTIVE = "active"; exports.IDLE = "idle"; -var meta = {}; - // When you start Zulip, has_focus should be true, but it might not be the // case after a server-initiated reload. exports.has_focus = document.hasFocus && document.hasFocus(); @@ -226,6 +224,16 @@ exports.insert_user_into_list = function (user_id) { compose_fade.update_one_user_row(elt); }; +exports.clear_highlight = function () { + // Undo highlighting + var item = $('#user_presences li.user_sidebar_entry.highlighted_user'); + item.removeClass('highlighted_user'); +}; + +exports.searching = function () { + return exports.user_filter && exports.user_filter.searching(); +}; + exports.build_user_sidebar = function () { if (page_params.realm_presence_disabled) { return; @@ -368,11 +376,6 @@ exports.initialize = function () { exports.build_user_sidebar(); exports.update_huddles(); - exports.set_user_list_filter_handlers(); - - $('#clear_search_people_button').on('click', exports.clear_search); - $('#userlist-header').click(exports.toggle_filter_displayed); - // Let the server know we're here, but pass "false" for // want_redraw, since we just got all this info in page_params. focus_ping(false); @@ -408,73 +411,12 @@ exports.redraw = function () { exports.update_huddles(); }; -exports.searching = function () { - return $('.user-list-filter').expectOne().is(':focus'); -}; - -exports.clear_search = function () { - var filter = $('.user-list-filter').expectOne(); - if (filter.val() === '') { - exports.clear_and_hide_search(); - return; - } - filter.val(''); - filter.blur(); - update_users_for_search(); -}; - -exports.escape_search = function () { - var filter = $('.user-list-filter').expectOne(); - if (filter.val() === '') { - exports.clear_and_hide_search(); - return; - } - filter.val(''); - update_users_for_search(); -}; - -exports.clear_and_hide_search = function () { - var filter = $('.user-list-filter').expectOne(); - if (filter.val() !== '') { - filter.val(''); - update_users_for_search(); - } - filter.blur(); - $('#user-list .input-append').addClass('notdisplayed'); - // Undo highlighting - $('#user_presences li.user_sidebar_entry.highlighted_user').removeClass('highlighted_user'); -}; - -function highlight_first_user() { +exports.highlight_first_user = function () { if ($('#user_presences li.user_sidebar_entry.narrow-filter.highlighted_user').length === 0) { // Highlight var all_streams = $('#user_presences li.user_sidebar_entry.narrow-filter'); stream_list.highlight_first(all_streams, 'highlighted_user'); } -} - -exports.initiate_search = function () { - var filter = $('.user-list-filter').expectOne(); - var column = $('.user-list-filter').closest(".app-main [class^='column-']"); - $('#user-list .input-append').removeClass('notdisplayed'); - if (!column.hasClass("expanded")) { - popovers.hide_all(); - if (column.hasClass('column-left')) { - stream_popover.show_streamlist_sidebar(); - } else if (column.hasClass('column-right')) { - popovers.show_userlist_sidebar(); - } - } - filter.focus(); - highlight_first_user(); -}; - -exports.toggle_filter_displayed = function () { - if ($('#user-list .input-append').hasClass('notdisplayed')) { - exports.initiate_search(); - } else { - exports.clear_and_hide_search(); - } }; exports.narrow_for_user = function (opts) { @@ -482,7 +424,7 @@ exports.narrow_for_user = function (opts) { var email = people.get_person_from_user_id(user_id).email; narrow.by('pm-with', email, {trigger: 'sidebar'}); - exports.clear_and_hide_search(); + exports.user_filter.clear_and_hide_search(); }; function keydown_enter_key() { @@ -500,30 +442,41 @@ function keydown_user_filter(e) { $('#user_presences'), 'highlighted_user', keydown_enter_key); } -function focus_user_filter(e) { - highlight_first_user(); - e.stopPropagation(); -} - function focusout_user_filter() { // Undo highlighting $('#user_presences li.user_sidebar_entry.highlighted_user').removeClass('highlighted_user'); } exports.set_user_list_filter = function () { - meta.$user_list_filter = $(".user-list-filter"); + exports.user_filter = user_search({ + update_list: update_users_for_search, + reset_items: exports.clear_highlight, + initialize_list_for_search: exports.highlight_first_user, + }); + + exports.set_user_list_filter_handlers(); }; exports.set_user_list_filter_handlers = function () { - meta.$user_list_filter.expectOne() - .on('click', focus_user_filter) - .on('input', update_users_for_search) + exports.user_filter.input_field() .on('keydown', keydown_user_filter) .on('blur', focusout_user_filter); }; +exports.initiate_search = function () { + if (exports.user_filter) { + exports.user_filter.initiate_search(); + } +}; + +exports.escape_search = function () { + if (exports.user_filter) { + exports.user_filter.escape_search(); + } +}; + exports.get_filter_text = function () { - if (!meta.$user_list_filter) { + if (!exports.user_filter) { // This may be overly defensive, but there may be // situations where get called before everything is // fully initialized. The empty string is a fine @@ -532,9 +485,7 @@ exports.get_filter_text = function () { return ''; } - var user_filter = meta.$user_list_filter.expectOne().val().trim(); - - return user_filter; + return exports.user_filter.text(); }; return exports; diff --git a/static/js/user_search.js b/static/js/user_search.js new file mode 100644 index 0000000000..c161e82a63 --- /dev/null +++ b/static/js/user_search.js @@ -0,0 +1,117 @@ +var user_search = function (opts) { + // This is mostly view code to manage the user search widget + // above the buddy list. We rely on other code to manage the + // details of populating the list when we change. + + var self = {}; + + var $widget = $('#user-list .input-append').expectOne(); + var $input = $('.user-list-filter').expectOne(); + + self.input_field = function () { + return $input; + }; + + self.text = function () { + return $input.val().trim(); + }; + + self.searching = function () { + return $input.is(':focus'); + }; + + self.empty = function () { + return self.text() === ''; + }; + + self.clear_search = function () { + if (self.empty()) { + self.close_widget(); + return; + } + + $input.val(''); + $input.blur(); + opts.update_list(); + }; + + self.escape_search = function () { + if (self.empty()) { + self.close_widget(); + return; + } + + $input.val(''); + opts.update_list(); + }; + + self.hide_widget = function () { + $widget.addClass('notdisplayed'); + }; + + self.show_widget = function () { + $widget.removeClass('notdisplayed'); + }; + + self.widget_shown = function () { + return $widget.hasClass('notdisplayed'); + }; + + self.clear_and_hide_search = function () { + if (!self.empty()) { + $input.val(''); + opts.update_list(); + } + self.close_widget(); + }; + + self.close_widget = function () { + $input.blur(); + self.hide_widget(); + opts.reset_items(); + }; + + self.expand_column = function () { + var column = $input.closest(".app-main [class^='column-']"); + if (!column.hasClass("expanded")) { + popovers.hide_all(); + if (column.hasClass('column-left')) { + stream_popover.show_streamlist_sidebar(); + } else if (column.hasClass('column-right')) { + popovers.show_userlist_sidebar(); + } + } + }; + + self.initiate_search = function () { + self.expand_column(); + self.show_widget(); + $input.focus(); + opts.initialize_list_for_search(); + }; + + self.toggle_filter_displayed = function () { + if (self.widget_shown()) { + self.initiate_search(); + } else { + self.clear_and_hide_search(); + } + }; + + function on_focus(e) { + opts.initialize_list_for_search(); + e.stopPropagation(); + } + + $('#clear_search_people_button').on('click', self.clear_search); + $('#userlist-header').on('click', self.toggle_filter_displayed); + + $input.on('input', opts.update_list); + $input.on('click', on_focus); + + return self; +}; + +if (typeof module !== 'undefined') { + module.exports = user_search; +} diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 01c17ec01b..c25f101ed5 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -73,6 +73,7 @@ enforce_fully_covered = { 'static/js/user_events.js', 'static/js/user_groups.js', 'static/js/user_pill.js', + 'static/js/user_search.js', 'static/js/util.js', } diff --git a/zproject/settings.py b/zproject/settings.py index ae36f5eb87..e1c54e3724 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -1135,6 +1135,7 @@ JS_SPECS = { 'js/server_events_dispatch.js', 'js/zulip.js', 'js/presence.js', + 'js/user_search.js', 'js/buddy_data.js', 'js/buddy_list.js', 'js/activity.js',