From 9986e4199985a539cd7091f217f9a2cf62fec127 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Wed, 30 Jan 2019 17:24:56 +0000 Subject: [PATCH] top left: Simplify markup for main links. The following elements in the top left corner are major components of our app: All messages Private messages Starred messages Mentions We can now find them directly: $('.top_left_all_messages') $('.top_left_private_messages') $('.top_left_starred_messages') $('.top_left_mentions') Before this, we had to build up complicated selectors like below: exports.get_global_filter_li = function (filter_name) { var selector = "#global_filters li[data-name='" + filter_name + "']"; return $(selector); }; I don't think any newbie would know to grep for "global_filter", and I've seen a PR where somebody added specific markup here to "Private messages" because they couldn't grok the old scheme. Another thing to note is that we still have a "home-link" class for "All messages", which overlapped with portico code that had the same name. (There were some inaccurate comments in the code relating to the tab bar, but we don't actually have a way to click to the home view in the tab bar any more.) I'll eliminate that cruft in another commit. For this commit the four elements still have the "global-filter" class, since there's some benefit to being able to style them all as a group, although we should give it a nicer name in a subsequent commit. Most of this PR is basic search/replace, but I did add a two-line helper: `top_left_corner.update_starred_count` --- frontend_tests/casper_tests/03-narrow.js | 4 +-- frontend_tests/node_tests/pm_list.js | 6 ++-- frontend_tests/node_tests/top_left_corner.js | 32 ++++++++++++----- frontend_tests/node_tests/ui_init.js | 2 +- static/js/click_handlers.js | 3 +- static/js/pm_list.js | 6 ++-- static/js/starred_messages.js | 3 +- static/js/top_left_corner.js | 37 ++++++++++---------- templates/zerver/app/left_sidebar.html | 8 ++--- 9 files changed, 58 insertions(+), 43 deletions(-) diff --git a/frontend_tests/casper_tests/03-narrow.js b/frontend_tests/casper_tests/03-narrow.js index 15e2f70df6..e94bcb0450 100644 --- a/frontend_tests/casper_tests/03-narrow.js +++ b/frontend_tests/casper_tests/03-narrow.js @@ -290,13 +290,13 @@ expect_stream(); casper.then(check_narrow_title('Verona - Zulip Dev - Zulip')); -casper.thenClick('#global_filters [data-name="home"] a'); +casper.thenClick('.top_left_all_messages a'); expect_home(); casper.then(check_narrow_title('home - Zulip Dev - Zulip')); -casper.thenClick('#global_filters [data-name="private"] a'); +casper.thenClick('.top_left_private_messages a'); expect_all_pm(); diff --git a/frontend_tests/node_tests/pm_list.js b/frontend_tests/node_tests/pm_list.js index c98f937901..3eaac95b82 100644 --- a/frontend_tests/node_tests/pm_list.js +++ b/frontend_tests/node_tests/pm_list.js @@ -105,7 +105,7 @@ run_test('build_private_messages_list', () => { expected_data.want_show_more_messages_links = true; assert.deepEqual(template_data, expected_data); - $('#global_filters').on = function (action, selector, f) { + $('.top_left_private_messages').on = function (action, selector, f) { var e = { preventDefault: function () {}, stopPropagation: function () {}}; f(e); }; @@ -127,7 +127,7 @@ run_test('expand_and_update_private_messages', () => { return 'fake-dom-for-pm-list'; }; - var private_li = $("#global_filters > li[data-name='private']"); + var private_li = $(".top_left_private_messages"); var alice_li = $.create('alice-li-stub'); var bob_li = $.create('bob-li-stub'); @@ -221,7 +221,7 @@ run_test('expand_and_update_private_messages', () => { run_test('update_dom_with_unread_counts', () => { var total_value = $.create('total-value-stub'); var total_count = $.create('total-count-stub'); - var private_li = $("#global_filters > li[data-name='private']"); + var private_li = $(".top_left_private_messages"); private_li.set_find_results('.count', total_count); total_count.set_find_results('.value', total_value); diff --git a/frontend_tests/node_tests/top_left_corner.js b/frontend_tests/node_tests/top_left_corner.js index be60261edc..7f6f3931d2 100644 --- a/frontend_tests/node_tests/top_left_corner.js +++ b/frontend_tests/node_tests/top_left_corner.js @@ -64,23 +64,29 @@ run_test('narrowing', () => { {operator: 'is', operand: 'mentioned'}, ]); top_left_corner.handle_narrow_activated(filter); - assert(top_left_corner.get_global_filter_li('mentioned').hasClass('active-filter')); + assert($('.top_left_mentions').hasClass('active-filter')); + + filter = new Filter([ + {operator: 'is', operand: 'starred'}, + ]); + top_left_corner.handle_narrow_activated(filter); + assert($('.top_left_starred_messages').hasClass('active-filter')); filter = new Filter([ {operator: 'in', operand: 'home'}, ]); top_left_corner.handle_narrow_activated(filter); - assert(top_left_corner.get_global_filter_li('home').hasClass('active-filter')); + assert($('.top_left_all_messages').hasClass('active-filter')); // deactivating narrow pm_closed = false; top_left_corner.handle_narrow_deactivated(); - assert(top_left_corner.get_global_filter_li('home').hasClass('active-filter')); - assert(!top_left_corner.get_global_filter_li('mentioned').hasClass('active-filter')); - assert(!top_left_corner.get_global_filter_li('private').hasClass('active-filter')); - assert(!top_left_corner.get_global_filter_li('starred').hasClass('active-filter')); + assert($('.top_left_all_messages').hasClass('active-filter')); + assert(!$('.top_left_mentions').hasClass('active-filter')); + assert(!$('.top_left_private_messages').hasClass('active-filter')); + assert(!$('.top_left_starred_messages').hasClass('active-filter')); assert(pm_closed); }); @@ -101,26 +107,36 @@ run_test('update_count_in_dom', () => { }; make_elem( - $("#global_filters li[data-name='mentioned']"), + $(".top_left_mentions"), '', '' ); make_elem( - $("#global_filters li[data-name='home']"), + $(".top_left_all_messages"), '', '' ); + make_elem( + $(".top_left_starred_messages"), + '', + '' + ); + top_left_corner.update_dom_with_unread_counts(counts); + top_left_corner.update_starred_count(444); assert.equal($('').text(), '222'); assert.equal($('').text(), '333'); + assert.equal($('').text(), '444'); counts.mentioned_message_count = 0; top_left_corner.update_dom_with_unread_counts(counts); + top_left_corner.update_starred_count(0); assert(!$('').visible()); assert.equal($('').text(), ''); + assert.equal($('').text(), ''); }); diff --git a/frontend_tests/node_tests/ui_init.js b/frontend_tests/node_tests/ui_init.js index 07661fb686..e7b10e5f71 100644 --- a/frontend_tests/node_tests/ui_init.js +++ b/frontend_tests/node_tests/ui_init.js @@ -157,7 +157,7 @@ $("#search_query").typeahead = () => {}; const value_stub = $.create('value'); const count_stub = $.create('count'); count_stub.set_find_results('.value', value_stub); -$("#global_filters li[data-name='starred']").set_find_results('.count', count_stub); +$(".top_left_starred_messages").set_find_results('.count', count_stub); run_test('initialize_everything', () => { diff --git a/static/js/click_handlers.js b/static/js/click_handlers.js index 6262e19ba4..eefe7d8c95 100644 --- a/static/js/click_handlers.js +++ b/static/js/click_handlers.js @@ -375,8 +375,7 @@ exports.initialize = function () { // HOME - // Capture both the left-sidebar All Messages click and the tab breadcrumb All Messages - $(document).on('click', ".home-link[data-name='home']", function (e) { + $(document).on('click', ".top_left_all_messages", function (e) { ui_util.change_tab_to('#home'); narrow.deactivate(); search.update_button_visibility(); diff --git a/static/js/pm_list.js b/static/js/pm_list.js index 553bc315fd..cbc6486249 100644 --- a/static/js/pm_list.js +++ b/static/js/pm_list.js @@ -13,7 +13,7 @@ var zoomed_in = false; // left corner of the app. This was split out from stream_list.js. function get_filter_li() { - return $("#global_filters > li[data-name='private']"); + return $(".top_left_private_messages"); } function update_count_in_dom(count_span, value_span, count) { @@ -167,12 +167,12 @@ exports.update_private_messages = function () { exports.rebuild_recent(""); } else if (is_pm_filter) { exports.rebuild_recent(""); - $("#global_filters li[data-name='private']").addClass('active-filter'); + $(".top_left_private_messages").addClass('active-filter'); } }; exports.set_click_handlers = function () { - $('#global_filters').on('click', '.show-more-private-messages', function (e) { + $('.top_left_private_messages').on('click', '.show-more-private-messages', function (e) { popovers.hide_all(); zoom_in(); e.preventDefault(); diff --git a/static/js/starred_messages.js b/static/js/starred_messages.js index 172da5359c..ac5d992041 100644 --- a/static/js/starred_messages.js +++ b/static/js/starred_messages.js @@ -40,8 +40,7 @@ exports.rerender_ui = function () { count = 0; } - var starred_li = top_left_corner.get_global_filter_li('starred'); - top_left_corner.update_count_in_dom(starred_li, count); + top_left_corner.update_starred_count(count); }; return exports; diff --git a/static/js/top_left_corner.js b/static/js/top_left_corner.js index 5dddca8f7f..2b26f56efd 100644 --- a/static/js/top_left_corner.js +++ b/static/js/top_left_corner.js @@ -2,11 +2,6 @@ var top_left_corner = (function () { var exports = {}; -exports.get_global_filter_li = function (filter_name) { - var selector = "#global_filters li[data-name='" + filter_name + "']"; - return $(selector); -}; - exports.update_count_in_dom = function (unread_count_elem, count) { var count_span = unread_count_elem.find('.count'); var value_span = count_span.find('.value'); @@ -21,13 +16,17 @@ exports.update_count_in_dom = function (unread_count_elem, count) { value_span.text(count); }; +exports.update_starred_count = function (count) { + var starred_li = $('.top_left_starred_messages'); + exports.update_count_in_dom(starred_li, count); +}; exports.update_dom_with_unread_counts = function (counts) { // Note that "Private messages" counts are handled in pm_list.js. // mentioned/home have simple integer counts - var mentioned_li = exports.get_global_filter_li('mentioned'); - var home_li = exports.get_global_filter_li('home'); + var mentioned_li = $('.top_left_mentions'); + var home_li = $('.top_left_all_messages'); exports.update_count_in_dom(mentioned_li, counts.mentioned_message_count); exports.update_count_in_dom(home_li, counts.home_unread_messages); @@ -37,15 +36,14 @@ exports.update_dom_with_unread_counts = function (counts) { }; function deselect_top_left_corner_items() { - function remove(name) { - var li = exports.get_global_filter_li(name); - li.removeClass('active-filter active-sub-filter'); + function remove(elem) { + elem.removeClass('active-filter active-sub-filter'); } - remove('home'); - remove('private'); - remove('starred'); - remove('mentioned'); + remove($('.top_left_all_messages')); + remove($('.top_left_private_messages')); + remove($('.top_left_starred_messages')); + remove($('.top_left_mentions')); } exports.handle_narrow_activated = function (filter) { @@ -60,15 +58,18 @@ exports.handle_narrow_activated = function (filter) { if (ops.length >= 1) { filter_name = ops[0]; if (filter_name === 'home') { - filter_li = exports.get_global_filter_li(filter_name); + filter_li = $('.top_left_all_messages'); filter_li.addClass('active-filter'); } } ops = filter.operands('is'); if (ops.length >= 1) { filter_name = ops[0]; - if (filter_name === 'starred' || filter_name === 'mentioned') { - filter_li = exports.get_global_filter_li(filter_name); + if (filter_name === 'starred') { + filter_li = $('.top_left_starred_messages'); + filter_li.addClass('active-filter'); + } else if (filter_name === 'mentioned') { + filter_li = $('.top_left_mentions'); filter_li.addClass('active-filter'); } } @@ -106,7 +107,7 @@ exports.handle_narrow_deactivated = function () { deselect_top_left_corner_items(); pm_list.close(); - var filter_li = exports.get_global_filter_li('home'); + var filter_li = $('.top_left_all_messages'); filter_li.addClass('active-filter'); }; diff --git a/templates/zerver/app/left_sidebar.html b/templates/zerver/app/left_sidebar.html index 815222a0a3..54b5860c8e 100644 --- a/templates/zerver/app/left_sidebar.html +++ b/templates/zerver/app/left_sidebar.html @@ -2,7 +2,7 @@