From acb71493860c4035050a43c52c5bdeb40e515f3d Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Mon, 10 Sep 2018 12:52:58 +0000 Subject: [PATCH] Extract topic_zoom.js. This small modules nicely breaks down the responsibilities of topic_list and stream_list when it comes to zooming in and out of topics (also known as hitting "more topics" or "All Streams). Before this, neither module was clearly in charge, and there were kind of complicated callback mechanisms. The stream_list code was asking topic_list to create click handlers that called back into stream_list. Now we just topic_zoom set up its own click handlers and delegate out to the other two modules. --- .eslintrc.json | 1 + frontend_tests/node_tests/general.js | 2 +- frontend_tests/node_tests/stream_list.js | 48 ++---------------------- static/js/bundles/app.js | 1 + static/js/stream_list.js | 32 ++++------------ static/js/topic_list.js | 29 +++++--------- static/js/topic_zoom.js | 48 ++++++++++++++++++++++++ static/js/ui_init.js | 2 + 8 files changed, 74 insertions(+), 89 deletions(-) create mode 100644 static/js/topic_zoom.js diff --git a/.eslintrc.json b/.eslintrc.json index 94ebfbc832..51bc28404d 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -176,6 +176,7 @@ "topic_data": false, "topic_generator": false, "topic_list": false, + "topic_zoom": false, "transmit": false, "tutorial": false, "typeahead_helper": false, diff --git a/frontend_tests/node_tests/general.js b/frontend_tests/node_tests/general.js index 88b3bbe58d..7945630f0b 100644 --- a/frontend_tests/node_tests/general.js +++ b/frontend_tests/node_tests/general.js @@ -706,7 +706,7 @@ run_test('stream_list', () => { const topic_list_helper = make_topic_list_helper(); var streams_shown; - stream_list.show_all_streams = () => { + stream_list.zoom_out_topics = () => { streams_shown = true; }; diff --git a/frontend_tests/node_tests/stream_list.js b/frontend_tests/node_tests/stream_list.js index 76c896bb37..4ec50e087e 100644 --- a/frontend_tests/node_tests/stream_list.js +++ b/frontend_tests/node_tests/stream_list.js @@ -233,37 +233,11 @@ function initialize_stream_data() { add_row(carSub); } -function test_helper() { - var events = []; - - return { - redirect: (module_name, func_name) => { - const full_name = module_name + '.' + func_name; - global[module_name][func_name] = () => { - events.push(full_name); - }; - }, - events: events, - }; -} - function elem($obj) { return {to_$: () => $obj}; } run_test('zoom_in_and_zoom_out', () => { - var helper; - - var callbacks; - topic_list.set_click_handlers = (opts) => { - callbacks = opts; - }; - - helper = test_helper(); - - helper.redirect('popovers', 'hide_all'); - helper.redirect('topic_list', 'zoom_in'); - var label1 = $.create('label1 stub'); var label2 = $.create('label2 stub'); @@ -307,12 +281,7 @@ run_test('zoom_in_and_zoom_out', () => { }; stream_list.initialize(); - callbacks.zoom_in({stream_id: 42}); - - assert.deepEqual(helper.events, [ - 'popovers.hide_all', - 'topic_list.zoom_in', - ]); + stream_list.zoom_in_topics({stream_id: 42}); assert(!label1.visible()); assert(!label2.visible()); @@ -321,24 +290,13 @@ run_test('zoom_in_and_zoom_out', () => { assert(!stream_li2.visible()); assert($('#streams_list').hasClass('zoom-in')); - helper = test_helper(); - helper.redirect('popovers', 'hide_all'); - helper.redirect('topic_list', 'zoom_out'); - helper.redirect('scroll_util', 'scroll_element_into_container'); - $('#stream_filters li.narrow-filter').show = () => { stream_li1.show(); stream_li2.show(); }; stream_li1.length = 1; - callbacks.zoom_out({stream_li: stream_li1}); - - assert.deepEqual(helper.events, [ - 'popovers.hide_all', - 'topic_list.zoom_out', - 'scroll_util.scroll_element_into_container', - ]); + stream_list.zoom_out_topics({stream_li: stream_li1}); assert(label1.visible()); assert(label2.visible()); @@ -363,7 +321,7 @@ run_test('narrowing', () => { topic_list.remove_expanded_topics = noop; topic_list.rebuild = noop; topic_list.active_stream_id = noop; - stream_list.show_all_streams = noop; + stream_list.zoom_out_topics = noop; scroll_util.scroll_element_into_container = noop; var scrollbar_updated = false; diff --git a/static/js/bundles/app.js b/static/js/bundles/app.js index 95d2c33e2e..59753c2ae5 100644 --- a/static/js/bundles/app.js +++ b/static/js/bundles/app.js @@ -64,6 +64,7 @@ import "js/stream_sort.js"; import "js/topic_generator.js"; import "js/top_left_corner.js"; import "js/stream_list.js"; +import "js/topic_zoom.js"; import "js/filter.js"; import 'js/voting_widget.js'; import 'js/todo_widget.js'; diff --git a/static/js/stream_list.js b/static/js/stream_list.js index 5e75511091..38f38c943b 100644 --- a/static/js/stream_list.js +++ b/static/js/stream_list.js @@ -153,9 +153,11 @@ exports.get_stream_li = function (stream_id) { return li; }; -function zoom_in(options) { - popovers.hide_all(); - topic_list.zoom_in(); +exports.zoom_in_topics = function (options) { + // This only does stream-related tasks related to zooming + // in to more topics, which is basically hiding all the + // other streams. + $("#streams_list").expectOne().removeClass("zoom-out").addClass("zoom-in"); // Hide stream list titles and pinned stream splitter @@ -176,19 +178,9 @@ function zoom_in(options) { elt.hide(); } }); -} +}; -function zoom_out(options) { - popovers.hide_all(); - topic_list.zoom_out(); - exports.show_all_streams(); - - if (options.stream_li) { - exports.scroll_stream_into_view(options.stream_li); - } -} - -exports.show_all_streams = function () { +exports.zoom_out_topics = function () { // Show stream list titles and pinned stream splitter $(".stream-filters-label").each(function () { $(this).show(); @@ -357,7 +349,7 @@ exports.refresh_pinned_or_unpinned_stream = function (sub) { function clear_topics() { topic_list.close(); - exports.show_all_streams(); + exports.zoom_out_topics(); } exports.get_sidebar_stream_topic_info = function (filter) { @@ -482,14 +474,6 @@ function actually_update_streams_for_search() { var update_streams_for_search = _.throttle(actually_update_streams_for_search, 50); exports.initialize = function () { - // TODO, Eventually topic_list won't be a big singleton, - // and we can create more component-based click handlers for - // each stream. - topic_list.set_click_handlers({ - zoom_in: zoom_in, - zoom_out: zoom_out, - }); - $(document).on('subscription_add_done.zulip', function (event) { exports.create_sidebar_row(event.sub); exports.build_stream_list(); diff --git a/static/js/topic_list.js b/static/js/topic_list.js index fd75228030..688f3ba288 100644 --- a/static/js/topic_list.js +++ b/static/js/topic_list.js @@ -197,6 +197,15 @@ exports.active_stream_id = function () { return active_widget.get_stream_id(); }; +exports.get_stream_li = function () { + if (!active_widget) { + return; + } + + var stream_li = active_widget.get_parent(); + return stream_li; +}; + exports.need_to_show_no_more_topics = function (stream_id) { // This function is important, and the use case here is kind of // subtle. We do complete redraws of the topic list when new @@ -267,25 +276,7 @@ exports.zoom_in = function () { topic_data.get_server_history(stream_id, on_success); }; -exports.set_click_handlers = function (callbacks) { - $('#stream_filters').on('click', '.show-more-topics', function (e) { - callbacks.zoom_in({ - stream_id: active_widget.get_stream_id(), - }); - - e.preventDefault(); - e.stopPropagation(); - }); - - $('.show-all-streams').on('click', function (e) { - callbacks.zoom_out({ - stream_li: active_widget.get_parent(), - }); - - e.preventDefault(); - e.stopPropagation(); - }); - +exports.initialize = function () { $('#stream_filters').on('click', '.topic-box', function (e) { if (e.metaKey || e.ctrlKey) { return; diff --git a/static/js/topic_zoom.js b/static/js/topic_zoom.js new file mode 100644 index 0000000000..62a2b69a1c --- /dev/null +++ b/static/js/topic_zoom.js @@ -0,0 +1,48 @@ +var topic_zoom = (function () { + +var exports = {}; + +function zoom_in() { + var stream_id = topic_list.active_stream_id(); + + popovers.hide_all(); + topic_list.zoom_in(); + stream_list.zoom_in_topics({ + stream_id: stream_id, + }); +} + +function zoom_out() { + var stream_li = topic_list.get_stream_li(); + + popovers.hide_all(); + topic_list.zoom_out(); + stream_list.zoom_out_topics(); + + if (stream_li) { + stream_list.scroll_stream_into_view(stream_li); + } +} + +exports.initialize = function () { + $('#stream_filters').on('click', '.show-more-topics', function (e) { + zoom_in(); + + e.preventDefault(); + e.stopPropagation(); + }); + + $('.show-all-streams').on('click', function (e) { + zoom_out(); + + e.preventDefault(); + e.stopPropagation(); + }); +}; + +return exports; +}()); +if (typeof module !== 'undefined') { + module.exports = topic_zoom; +} +window.topic_zoom = topic_zoom; diff --git a/static/js/ui_init.js b/static/js/ui_init.js index 22ab3925bb..1fbd252606 100644 --- a/static/js/ui_init.js +++ b/static/js/ui_init.js @@ -321,6 +321,8 @@ $(function () { compose_fade.initialize(); pm_list.initialize(); stream_list.initialize(); + topic_list.initialize(); + topic_zoom.initialize(); drafts.initialize(); sent_messages.initialize(); hotspots.initialize();