From 194619c667d7a2ae9a641506f694cb0fc3673464 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Fri, 1 Mar 2019 20:06:53 +0000 Subject: [PATCH] css: Clean up left sidebar chevrons. This is a fairly big commit, but at the end it simplifies a lot of things. It's difficult to fix highly coupled code in incremental steps because, well, it's highly coupled code. The main thing this does is give each type of chevron in the left sidebar its own class * all-messages-arrow (NEW) * stream-sidebar-arrow * topic-sidebar-arrow Before this change, the "All messages" chevron was using stream-sidebar-arrow, which was a strange name for something that's not actually in the stream sidebar. Obviously this was cargo culted. There was not much JS to change here--we just fix the click handler for "All messsages". And then there's a one-line change to the template, and the rest is re-organizing the CSS. --- static/js/stream_popover.js | 2 +- static/styles/left-sidebar.scss | 91 +++++++++++--------------- static/styles/night_mode.scss | 15 +++-- templates/zerver/app/left_sidebar.html | 2 +- 4 files changed, 52 insertions(+), 58 deletions(-) diff --git a/static/js/stream_popover.js b/static/js/stream_popover.js index a56372c2de..12dbdc1b5d 100644 --- a/static/js/stream_popover.js +++ b/static/js/stream_popover.js @@ -222,7 +222,7 @@ exports.register_click_handlers = function () { }); }); - $('#global_filters').on('click', '.stream-sidebar-arrow', build_all_messages_popover); + $('#global_filters').on('click', '.all-messages-arrow', build_all_messages_popover); exports.register_stream_handlers(); exports.register_topic_handlers(); diff --git a/static/styles/left-sidebar.scss b/static/styles/left-sidebar.scss index dbab32ed1f..e69377ae9c 100644 --- a/static/styles/left-sidebar.scss +++ b/static/styles/left-sidebar.scss @@ -245,36 +245,6 @@ a.conversation-partners:hover { text-decoration: underline; } -/* - - The .arrow class is used in Zulip for chevrons, - but it's also used by popover for the little - triangle in the popovers. We are moving to - eliminating the arrow class and using more concrete - classes for chevrons. Here is an audit of the existing - state: - - All messages: - has stream-sidebar-arrow (why?????) - ul.filters .arrow (right: 10px, etc.) - - Private messages: - no chevrons - - stream chevron - has stream-sidebar-arrow (ok) - ul.filters .arrow (right: 10px, etc.) - - topic chevron: - ul.filters .topic-sidebar-arrow (ok, smaller font) - - ul.filters .arrow: - keeps position, right - ignores top, font-size, display - display would be none either way - -*/ - ul.filters i { padding-right: 0.25em; /* Make filter icons the same width so labels line up. */ @@ -282,42 +252,59 @@ ul.filters i { width: 13px; } -ul.filters .arrow { +/* + All of our left sidebar handlers use absolute + positioning. We should fix that. +*/ +.all-messages-arrow, +.stream-sidebar-arrow, +.topic-sidebar-arrow { position: absolute; - top: 2px; - right: 10px; - font-size: 0.8em; display: none; } -ul.filters li:hover .arrow { - display: inline; - cursor: pointer; - color: hsl(0, 0%, 53%); +/* + The All Messages and Stream chevrons are + pretty similar. +*/ +.all-messages-arrow, +.stream-sidebar-arrow { + top: 2px; + right: 10px; + font-size: 0.8em; } -ul.filters li .arrow:hover { - display: inline; - cursor: pointer; - color: hsl(0, 0%, 0%); -} - -ul.filters .topic-sidebar-arrow { - font-size: 0.7em; +/* + For topic chevrons we use a slightly smaller + font to show they're "lower" in the hierarchy, + which also affects it positioning. +*/ +.topic-sidebar-arrow { top: 1px; - display: none !important; + right: 10px; + font-size: 0.7em; } +/* + When you hover over list items, we hover + the relevant chevrons in light gray. +*/ +li.top_left_all_messages:hover .all-messages-arrow, +#stream_filters li:hover .stream-sidebar-arrow, li.topic-list-item:hover .topic-sidebar-arrow { - display: inline !important; + display: inline; cursor: pointer; color: hsl(0, 0%, 53%); } -li.topic-list-item .topic-sidebar-arrow:hover { - display: inline; - cursor: pointer; - color: hsl(0, 0%, 0%); +/* + If you hover directly over the chevron itself, + show it in black. +*/ +.all-messages-arrow:hover, +.stream-sidebar-arrow:hover, +.topic-sidebar-arrow:hover { + color: hsl(0, 0%, 0%) !important; } ul.filters li.muted_topic, diff --git a/static/styles/night_mode.scss b/static/styles/night_mode.scss index 11af724983..e2a7d8473c 100644 --- a/static/styles/night_mode.scss +++ b/static/styles/night_mode.scss @@ -256,7 +256,9 @@ on a dark background, and don't change the dark labels dark either. */ #searchbox .search_button, .close, #user_presences li:hover .user-list-arrow, - ul.filters li:hover .arrow { + li.top_left_all_messages:hover .all-messages-arrow, + #stream_filters li:hover .stream-sidebar-arrow, + li.topic-list-item:hover .topic-sidebar-arrow { color: hsl(236, 33%, 80%); } @@ -266,12 +268,17 @@ on a dark background, and don't change the dark labels dark either. */ #searchbox_legacy .search_button:hover, #searchbox a.search_icon:hover, #searchbox .search_button:hover, - .close:hover, - #user_presences li .user-list-arrow:hover, - ul.filters li .arrow:hover { + .close:hover { color: hsl(0, 0%, 100%); } + #user_presences li .user-list-arrow:hover, + .all-messages-arrow:hover, + .stream-sidebar-arrow:hover, + .topic-sidebar-arrow:hover { + color: hsl(0, 0%, 100%) !important; + } + #streamlist-toggle, #userlist-toggle { border-color: hsla(199, 33%, 46%, 0.2); diff --git a/templates/zerver/app/left_sidebar.html b/templates/zerver/app/left_sidebar.html index 27c45e2a61..1605cfebb1 100644 --- a/templates/zerver/app/left_sidebar.html +++ b/templates/zerver/app/left_sidebar.html @@ -12,7 +12,7 @@ - +