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`
This commit is contained in:
Steve Howell 2019-01-30 17:24:56 +00:00 committed by Tim Abbott
parent a37558b021
commit 9986e41999
9 changed files with 58 additions and 43 deletions

View File

@ -290,13 +290,13 @@ expect_stream();
casper.then(check_narrow_title('Verona - Zulip Dev - Zulip')); 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(); expect_home();
casper.then(check_narrow_title('home - Zulip Dev - Zulip')); 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(); expect_all_pm();

View File

@ -105,7 +105,7 @@ run_test('build_private_messages_list', () => {
expected_data.want_show_more_messages_links = true; expected_data.want_show_more_messages_links = true;
assert.deepEqual(template_data, expected_data); 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 () {}}; var e = { preventDefault: function () {}, stopPropagation: function () {}};
f(e); f(e);
}; };
@ -127,7 +127,7 @@ run_test('expand_and_update_private_messages', () => {
return 'fake-dom-for-pm-list'; 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 alice_li = $.create('alice-li-stub');
var bob_li = $.create('bob-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', () => { run_test('update_dom_with_unread_counts', () => {
var total_value = $.create('total-value-stub'); var total_value = $.create('total-value-stub');
var total_count = $.create('total-count-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); private_li.set_find_results('.count', total_count);
total_count.set_find_results('.value', total_value); total_count.set_find_results('.value', total_value);

View File

@ -64,23 +64,29 @@ run_test('narrowing', () => {
{operator: 'is', operand: 'mentioned'}, {operator: 'is', operand: 'mentioned'},
]); ]);
top_left_corner.handle_narrow_activated(filter); 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([ filter = new Filter([
{operator: 'in', operand: 'home'}, {operator: 'in', operand: 'home'},
]); ]);
top_left_corner.handle_narrow_activated(filter); 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 // deactivating narrow
pm_closed = false; pm_closed = false;
top_left_corner.handle_narrow_deactivated(); top_left_corner.handle_narrow_deactivated();
assert(top_left_corner.get_global_filter_li('home').hasClass('active-filter')); assert($('.top_left_all_messages').hasClass('active-filter'));
assert(!top_left_corner.get_global_filter_li('mentioned').hasClass('active-filter')); assert(!$('.top_left_mentions').hasClass('active-filter'));
assert(!top_left_corner.get_global_filter_li('private').hasClass('active-filter')); assert(!$('.top_left_private_messages').hasClass('active-filter'));
assert(!top_left_corner.get_global_filter_li('starred').hasClass('active-filter')); assert(!$('.top_left_starred_messages').hasClass('active-filter'));
assert(pm_closed); assert(pm_closed);
}); });
@ -101,26 +107,36 @@ run_test('update_count_in_dom', () => {
}; };
make_elem( make_elem(
$("#global_filters li[data-name='mentioned']"), $(".top_left_mentions"),
'<mentioned-count>', '<mentioned-count>',
'<mentioned-value>' '<mentioned-value>'
); );
make_elem( make_elem(
$("#global_filters li[data-name='home']"), $(".top_left_all_messages"),
'<home-count>', '<home-count>',
'<home-value>' '<home-value>'
); );
make_elem(
$(".top_left_starred_messages"),
'<starred-count>',
'<starred-value>'
);
top_left_corner.update_dom_with_unread_counts(counts); top_left_corner.update_dom_with_unread_counts(counts);
top_left_corner.update_starred_count(444);
assert.equal($('<mentioned-value>').text(), '222'); assert.equal($('<mentioned-value>').text(), '222');
assert.equal($('<home-value>').text(), '333'); assert.equal($('<home-value>').text(), '333');
assert.equal($('<starred-value>').text(), '444');
counts.mentioned_message_count = 0; counts.mentioned_message_count = 0;
top_left_corner.update_dom_with_unread_counts(counts); top_left_corner.update_dom_with_unread_counts(counts);
top_left_corner.update_starred_count(0);
assert(!$('<mentioned-count>').visible()); assert(!$('<mentioned-count>').visible());
assert.equal($('<mentioned-value>').text(), ''); assert.equal($('<mentioned-value>').text(), '');
assert.equal($('<starred-value>').text(), '');
}); });

View File

@ -157,7 +157,7 @@ $("#search_query").typeahead = () => {};
const value_stub = $.create('value'); const value_stub = $.create('value');
const count_stub = $.create('count'); const count_stub = $.create('count');
count_stub.set_find_results('.value', value_stub); 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', () => { run_test('initialize_everything', () => {

View File

@ -375,8 +375,7 @@ exports.initialize = function () {
// HOME // HOME
// Capture both the left-sidebar All Messages click and the tab breadcrumb All Messages $(document).on('click', ".top_left_all_messages", function (e) {
$(document).on('click', ".home-link[data-name='home']", function (e) {
ui_util.change_tab_to('#home'); ui_util.change_tab_to('#home');
narrow.deactivate(); narrow.deactivate();
search.update_button_visibility(); search.update_button_visibility();

View File

@ -13,7 +13,7 @@ var zoomed_in = false;
// left corner of the app. This was split out from stream_list.js. // left corner of the app. This was split out from stream_list.js.
function get_filter_li() { 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) { function update_count_in_dom(count_span, value_span, count) {
@ -167,12 +167,12 @@ exports.update_private_messages = function () {
exports.rebuild_recent(""); exports.rebuild_recent("");
} else if (is_pm_filter) { } else if (is_pm_filter) {
exports.rebuild_recent(""); exports.rebuild_recent("");
$("#global_filters li[data-name='private']").addClass('active-filter'); $(".top_left_private_messages").addClass('active-filter');
} }
}; };
exports.set_click_handlers = function () { 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(); popovers.hide_all();
zoom_in(); zoom_in();
e.preventDefault(); e.preventDefault();

View File

@ -40,8 +40,7 @@ exports.rerender_ui = function () {
count = 0; count = 0;
} }
var starred_li = top_left_corner.get_global_filter_li('starred'); top_left_corner.update_starred_count(count);
top_left_corner.update_count_in_dom(starred_li, count);
}; };
return exports; return exports;

View File

@ -2,11 +2,6 @@ var top_left_corner = (function () {
var exports = {}; 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) { exports.update_count_in_dom = function (unread_count_elem, count) {
var count_span = unread_count_elem.find('.count'); var count_span = unread_count_elem.find('.count');
var value_span = count_span.find('.value'); 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); 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) { exports.update_dom_with_unread_counts = function (counts) {
// Note that "Private messages" counts are handled in pm_list.js. // Note that "Private messages" counts are handled in pm_list.js.
// mentioned/home have simple integer counts // mentioned/home have simple integer counts
var mentioned_li = exports.get_global_filter_li('mentioned'); var mentioned_li = $('.top_left_mentions');
var home_li = exports.get_global_filter_li('home'); var home_li = $('.top_left_all_messages');
exports.update_count_in_dom(mentioned_li, counts.mentioned_message_count); exports.update_count_in_dom(mentioned_li, counts.mentioned_message_count);
exports.update_count_in_dom(home_li, counts.home_unread_messages); 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 deselect_top_left_corner_items() {
function remove(name) { function remove(elem) {
var li = exports.get_global_filter_li(name); elem.removeClass('active-filter active-sub-filter');
li.removeClass('active-filter active-sub-filter');
} }
remove('home'); remove($('.top_left_all_messages'));
remove('private'); remove($('.top_left_private_messages'));
remove('starred'); remove($('.top_left_starred_messages'));
remove('mentioned'); remove($('.top_left_mentions'));
} }
exports.handle_narrow_activated = function (filter) { exports.handle_narrow_activated = function (filter) {
@ -60,15 +58,18 @@ exports.handle_narrow_activated = function (filter) {
if (ops.length >= 1) { if (ops.length >= 1) {
filter_name = ops[0]; filter_name = ops[0];
if (filter_name === 'home') { if (filter_name === 'home') {
filter_li = exports.get_global_filter_li(filter_name); filter_li = $('.top_left_all_messages');
filter_li.addClass('active-filter'); filter_li.addClass('active-filter');
} }
} }
ops = filter.operands('is'); ops = filter.operands('is');
if (ops.length >= 1) { if (ops.length >= 1) {
filter_name = ops[0]; filter_name = ops[0];
if (filter_name === 'starred' || filter_name === 'mentioned') { if (filter_name === 'starred') {
filter_li = exports.get_global_filter_li(filter_name); 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'); filter_li.addClass('active-filter');
} }
} }
@ -106,7 +107,7 @@ exports.handle_narrow_deactivated = function () {
deselect_top_left_corner_items(); deselect_top_left_corner_items();
pm_list.close(); pm_list.close();
var filter_li = exports.get_global_filter_li('home'); var filter_li = $('.top_left_all_messages');
filter_li.addClass('active-filter'); filter_li.addClass('active-filter');
}; };

View File

@ -2,7 +2,7 @@
<div class="bottom_sidebar"> <div class="bottom_sidebar">
<ul id="global_filters" class="filters"> <ul id="global_filters" class="filters">
{# Special-case this link so we don't actually go to page top. #} {# Special-case this link so we don't actually go to page top. #}
<li data-name="home" class="global-filter active-filter home-link" title="{{ _('All messages') }} (Esc)"> <li class="top_left_all_messages global-filter active-filter home-link" title="{{ _('All messages') }} (Esc)">
<a href="#"> <a href="#">
<span class="filter-icon"> <span class="filter-icon">
<i class="fa fa-home" aria-hidden="true"></i> <i class="fa fa-home" aria-hidden="true"></i>
@ -14,7 +14,7 @@
</a> </a>
<span class="arrow stream-sidebar-arrow"><i class="fa fa-chevron-down" aria-hidden="true"></i></span> <span class="arrow stream-sidebar-arrow"><i class="fa fa-chevron-down" aria-hidden="true"></i></span>
</li> </li>
<li data-name="private" class="global-filter" title="{{ _('Private messages') }} (P)"> <li class="top_left_private_messages global-filter" title="{{ _('Private messages') }} (P)">
<a href="#narrow/is/private"> <a href="#narrow/is/private">
<span class="filter-icon"> <span class="filter-icon">
<i class="fa fa-envelope" aria-hidden="true"></i> <i class="fa fa-envelope" aria-hidden="true"></i>
@ -25,7 +25,7 @@
</span> </span>
</a> </a>
</li> </li>
<li data-name="starred" class="global-filter"> <li class="top_left_starred_messages global-filter">
<a href="#narrow/is/starred"> <a href="#narrow/is/starred">
<span class="filter-icon"> <span class="filter-icon">
<i class="fa fa-star" aria-hidden="true"></i> <i class="fa fa-star" aria-hidden="true"></i>
@ -36,7 +36,7 @@
</span> </span>
</a> </a>
</li> </li>
<li data-name="mentioned" class="global-filter"> <li class="top_left_mentions global-filter">
<a href="#narrow/is/mentioned"> <a href="#narrow/is/mentioned">
<span class="filter-icon"> <span class="filter-icon">
<i class="fa fa-at" aria-hidden="true"></i> <i class="fa fa-at" aria-hidden="true"></i>