From fbe9a9e539cf7fe042352eb968843341a1034f96 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Thu, 10 Nov 2022 11:32:37 +0000 Subject: [PATCH] left_side_userlist: Remove feature from frontend. Fixes #23517. While this feature was added to Zulip very early, it has been troubled for most of that time; it never looked great visually, had a lot of implementation complexity around resize.js, and has a weird model (a setting that changes the UI only in certain window sizes). This option is not commonly used; while a significant portion of users have it enabled, many of them just don't use window sizes where it actually has an effect. So it's not clear that it will be missed if removed; we got very few bug reports when it was completely broken for a few days after we first integrated the new left sidebar private messages design. Even with it no longer being broken, it does not work very well with the addition of the new PMs section in the left sidebar. (Having two scrollbars in the sidebar looks quite awkward.) The new private messages section in the left sidebar also addresses some of the use cases for always keeping the Users list always visible, even in narrow windows. This option is only removed from frontend for now. To make this decision easily reversible, the backend code of this feature is still kept. --- docs/testing/manual-testing.md | 1 - frontend_tests/node_tests/dispatch.js | 5 -- frontend_tests/node_tests/i18n.js | 1 - frontend_tests/node_tests/lib/events.js | 7 -- static/js/message_viewport.js | 8 -- static/js/realm_user_settings_defaults.ts | 1 - static/js/resize.js | 89 +------------------ static/js/server_events_dispatch.js | 5 -- static/js/settings_config.ts | 4 - static/js/settings_display.js | 21 +---- static/js/stream_popover.js | 3 - static/js/user_settings.ts | 1 - static/styles/compose.css | 1 - static/styles/left_sidebar.css | 10 --- static/styles/right_sidebar.css | 1 - static/styles/typing_notifications.css | 1 - static/styles/zulip.css | 1 - .../zerver/help/include/sidebar_index.md | 1 - ...move-the-users-list-to-the-left-sidebar.md | 20 ----- zerver/models.py | 7 +- 20 files changed, 9 insertions(+), 179 deletions(-) delete mode 100644 templates/zerver/help/move-the-users-list-to-the-left-sidebar.md diff --git a/docs/testing/manual-testing.md b/docs/testing/manual-testing.md index f1418a5854..2656770970 100644 --- a/docs/testing/manual-testing.md +++ b/docs/testing/manual-testing.md @@ -486,7 +486,6 @@ Do these tasks as Cordelia. - Display settings - Right now, these unfortunately require reloads to take effect. - Default language (change to Spanish) - - Show user list on left sidebar in narrow windows (verify by making window thinner) - 24-hour time (and then test going back to AM/PM) - Notifications - Stream message diff --git a/frontend_tests/node_tests/dispatch.js b/frontend_tests/node_tests/dispatch.js index d6161ebf46..2a6de1fe8b 100644 --- a/frontend_tests/node_tests/dispatch.js +++ b/frontend_tests/node_tests/dispatch.js @@ -743,11 +743,6 @@ run_test("user_settings", ({override}) => { dispatch(event); assert_same(user_settings.default_language, "fr"); - event = event_fixtures.user_settings__left_side_userlist; - user_settings.left_side_userlist = false; - dispatch(event); - assert_same(user_settings.left_side_userlist, true); - event = event_fixtures.user_settings__escape_navigates_to_default_view; user_settings.escape_navigates_to_default_view = false; let toggled = []; diff --git a/frontend_tests/node_tests/i18n.js b/frontend_tests/node_tests/i18n.js index 5f046b9395..e0db82a237 100644 --- a/frontend_tests/node_tests/i18n.js +++ b/frontend_tests/node_tests/i18n.js @@ -100,7 +100,6 @@ run_test("tr_tag", ({mock_template}) => { avatar_url: "http://example.com", }, user_settings: { - left_side_userlist: false, twenty_four_hour_time: false, enable_stream_desktop_notifications: false, enable_stream_push_notifications: false, diff --git a/frontend_tests/node_tests/lib/events.js b/frontend_tests/node_tests/lib/events.js index 466c667d87..df6d167a55 100644 --- a/frontend_tests/node_tests/lib/events.js +++ b/frontend_tests/node_tests/lib/events.js @@ -898,13 +898,6 @@ exports.fixtures = { value: true, }, - user_settings__left_side_userlist: { - type: "user_settings", - op: "update", - property: "left_side_userlist", - value: true, - }, - user_settings__presence_disabled: { type: "user_settings", op: "update", diff --git a/static/js/message_viewport.js b/static/js/message_viewport.js index aad68bb74f..3d243d613b 100644 --- a/static/js/message_viewport.js +++ b/static/js/message_viewport.js @@ -1,7 +1,6 @@ import $ from "jquery"; import * as blueslip from "./blueslip"; -import {media_breakpoints_num} from "./css_variables"; import * as message_lists from "./message_lists"; import * as message_scroll from "./message_scroll"; import * as notifications from "./notifications"; @@ -313,13 +312,6 @@ export function stop_auto_scrolling() { } } -export function is_narrow() { - // This basically returns true when we hide the right sidebar for - // the left_side_userlist skinny mode. It would be nice to have a less brittle - // test for this. - return window.innerWidth < media_breakpoints_num.xl; -} - export function system_initiated_animate_scroll(scroll_amount) { message_scroll.suppress_selection_update_on_next_scroll(); const viewport_offset = scrollTop(); diff --git a/static/js/realm_user_settings_defaults.ts b/static/js/realm_user_settings_defaults.ts index 52181eb16c..c3cfcb258e 100644 --- a/static/js/realm_user_settings_defaults.ts +++ b/static/js/realm_user_settings_defaults.ts @@ -25,7 +25,6 @@ export type RealmDefaultSettings = { escape_navigates_to_default_view: boolean; fluid_layout_width: boolean; high_contrast_mode: boolean; - left_side_userlist: boolean; message_content_in_email_notifications: boolean; notification_sound: string; pm_content_in_desktop_notifications: boolean; diff --git a/static/js/resize.js b/static/js/resize.js index fa74978513..5d1f4f6007 100644 --- a/static/js/resize.js +++ b/static/js/resize.js @@ -10,12 +10,8 @@ import * as navbar_alerts from "./navbar_alerts"; import * as navigate from "./navigate"; import * as popovers from "./popovers"; import * as recent_topics_util from "./recent_topics_util"; -import * as ui from "./ui"; -import {user_settings} from "./user_settings"; import * as util from "./util"; -let narrow_window = false; - function get_new_heights() { const res = {}; const viewport_height = message_viewport.height(); @@ -51,50 +47,6 @@ function get_new_heights() { return res; } -function left_userlist_get_new_heights() { - const res = {}; - const viewport_height = message_viewport.height(); - const viewport_width = message_viewport.width(); - res.viewport_height = viewport_height; - res.viewport_width = viewport_width; - - // main div - const top_navbar_height = $(".header").safeOuterHeight(true); - res.bottom_whitespace_height = viewport_height * 0.4; - res.main_div_min_height = viewport_height - top_navbar_height; - - // left sidebar - const $stream_filters = $("#left_sidebar_scroll_container").expectOne(); - const $buddy_list_wrapper = $("#buddy_list_wrapper").expectOne(); - - const stream_filters_real_height = ui.get_scroll_element($stream_filters).prop("scrollHeight"); - const user_list_real_height = ui.get_scroll_element($buddy_list_wrapper).prop("scrollHeight"); - - res.total_leftlist_height = - viewport_height - - Number.parseInt($("#left-sidebar").css("marginTop"), 10) - - Number.parseInt($(".narrows_panel").css("marginTop"), 10) - - Number.parseInt($(".narrows_panel").css("marginBottom"), 10) - - $("#global_filters").safeOuterHeight(true) - - $("#userlist-header").safeOuterHeight(true) - - $("#user_search_section").safeOuterHeight(true) - - $("#private_messages_sticky_header").safeOuterHeight(true); - - if (res.total_leftlist_height - user_list_real_height > stream_filters_real_height) { - // There is enough space for the both lists to be fully displayed. - res.stream_filters_max_height = "100%"; - res.buddy_list_wrapper_max_height = "100%"; - } else { - res.stream_filters_max_height = Math.min( - res.total_leftlist_height / 2, - stream_filters_real_height, - ); - res.buddy_list_wrapper_max_height = - res.total_leftlist_height - res.stream_filters_max_height; - } - return res; -} - export function watch_manual_resize(element) { const box = document.querySelector(element); @@ -141,7 +93,7 @@ export function reset_compose_message_max_height(bottom_whitespace_height) { // Compute bottom_whitespace_height if not provided by caller. if (bottom_whitespace_height === undefined) { - const h = narrow_window ? left_userlist_get_new_heights() : get_new_heights(); + const h = get_new_heights(); bottom_whitespace_height = h.bottom_whitespace_height; } @@ -182,50 +134,15 @@ export function resize_bottom_whitespace(h) { } export function resize_stream_filters_container() { - const h = narrow_window ? left_userlist_get_new_heights() : get_new_heights(); + const h = get_new_heights(); resize_bottom_whitespace(h); $("#left_sidebar_scroll_container").css("max-height", h.stream_filters_max_height); - if (user_settings.left_side_userlist) { - $("#buddy_list_wrapper").css("max-height", h.buddy_list_wrapper_max_height); - } } export function resize_sidebars() { - let $sidebar; - - if (user_settings.left_side_userlist) { - const css_narrow_mode = message_viewport.is_narrow(); - - $("#top_navbar").removeClass("rightside-userlist"); - - const $right_items = $(".right-sidebar-items").expectOne(); - - if (css_narrow_mode && !narrow_window) { - // move stuff to the left sidebar (skinny mode) - narrow_window = true; - popovers.set_userlist_placement("left"); - $sidebar = $("#left-sidebar").expectOne(); - $sidebar.append($right_items); - $("#buddy_list_wrapper").css("margin", "0px"); - $("#userlist-toggle").css("display", "none"); - $("#invite-user-link").hide(); - } else if (!css_narrow_mode && narrow_window) { - // move stuff to the right sidebar (wide mode) - narrow_window = false; - popovers.set_userlist_placement("right"); - $sidebar = $("#right-sidebar").expectOne(); - $sidebar.append($right_items); - $("#buddy_list_wrapper").css("margin", ""); - $("#userlist-toggle").css("display", ""); - $("#invite-user-link").show(); - } - } - - const h = narrow_window ? left_userlist_get_new_heights() : get_new_heights(); - + const h = get_new_heights(); $("#buddy_list_wrapper").css("max-height", h.buddy_list_wrapper_max_height); $("#left_sidebar_scroll_container").css("max-height", h.stream_filters_max_height); - return h; } diff --git a/static/js/server_events_dispatch.js b/static/js/server_events_dispatch.js index 50e44f4fff..58feb23059 100644 --- a/static/js/server_events_dispatch.js +++ b/static/js/server_events_dispatch.js @@ -616,7 +616,6 @@ export function dispatch_normal_event(event) { "escape_navigates_to_default_view", "fluid_layout_width", "high_contrast_mode", - "left_side_userlist", "timezone", "twenty_four_hour_time", "translate_emoticons", @@ -685,10 +684,6 @@ export function dispatch_normal_event(event) { if (event.property === "fluid_layout_width") { scroll_bar.set_layout_width(); } - if (event.property === "left_side_userlist") { - // TODO: Make this change the view immediately rather - // than requiring a reload or page resize. - } if (event.property === "default_language") { // TODO: Make this change the view immediately rather than // requiring a reload. This is likely fairly difficult, diff --git a/static/js/settings_config.ts b/static/js/settings_config.ts index 19e7d561ef..92740c8f3f 100644 --- a/static/js/settings_config.ts +++ b/static/js/settings_config.ts @@ -108,7 +108,6 @@ export const get_all_display_settings = (): DisplaySettings => ({ user_display_settings: [ "dense_mode", "high_contrast_mode", - "left_side_userlist", "fluid_layout_width", "starred_message_counts", ], @@ -479,9 +478,6 @@ export const display_settings_labels = { dense_mode: $t({defaultMessage: "Dense mode"}), fluid_layout_width: $t({defaultMessage: "Use full width on wide screens"}), high_contrast_mode: $t({defaultMessage: "High contrast mode"}), - left_side_userlist: $t({ - defaultMessage: "Show user list on left sidebar in narrow windows", - }), starred_message_counts: $t({defaultMessage: "Show counts for starred messages"}), twenty_four_hour_time: $t({defaultMessage: "Time format"}), translate_emoticons: new Handlebars.SafeString( diff --git a/static/js/settings_display.js b/static/js/settings_display.js index 3c0239f9ca..581773670f 100644 --- a/static/js/settings_display.js +++ b/static/js/settings_display.js @@ -189,26 +189,7 @@ export function set_up(settings_panel) { const $status_element = $input_elem .closest(".subsection-parent") .find(".alert-notification"); - - if (["left_side_userlist"].includes(setting)) { - change_display_setting( - data, - $status_element, - $t_html( - { - defaultMessage: - "Saved. Please reload for the change to take effect.", - }, - { - "z-link": (content_html) => - `${content_html.join("")}`, - }, - ), - true, - ); - } else { - change_display_setting(data, $status_element); - } + change_display_setting(data, $status_element); }); $container.find(".setting_emojiset_choice").on("click", function () { diff --git a/static/js/stream_popover.js b/static/js/stream_popover.js index d1c1b0dd9f..3e854bde2f 100644 --- a/static/js/stream_popover.js +++ b/static/js/stream_popover.js @@ -170,9 +170,6 @@ export function hide_drafts_popover() { export function show_streamlist_sidebar() { $(".app-main .column-left").addClass("expanded"); - - // Redo the calculation for how large the sidebar is; this is - // important for the left_side_userlist setting. resize.resize_stream_filters_container(); } diff --git a/static/js/user_settings.ts b/static/js/user_settings.ts index c7fc42d3ae..513404cad5 100644 --- a/static/js/user_settings.ts +++ b/static/js/user_settings.ts @@ -31,7 +31,6 @@ export type UserSettings = (StreamNotificationSettings & PmNotificationSettings) escape_navigates_to_default_view: boolean; fluid_layout_width: boolean; high_contrast_mode: boolean; - left_side_userlist: boolean; message_content_in_email_notifications: boolean; notification_sound: string; pm_content_in_desktop_notifications: boolean; diff --git a/static/styles/compose.css b/static/styles/compose.css index 3d00fc824d..e7299042d6 100644 --- a/static/styles/compose.css +++ b/static/styles/compose.css @@ -715,7 +715,6 @@ a.compose_control_button.hide { margin-right: 4px; } -/* This max-width must be synced with message_viewport.is_narrow */ @media (width < $xl_min) { #compose-content { margin-right: 7px; diff --git a/static/styles/left_sidebar.css b/static/styles/left_sidebar.css index 0433804c1f..cd8be55fcf 100644 --- a/static/styles/left_sidebar.css +++ b/static/styles/left_sidebar.css @@ -12,16 +12,6 @@ $topic_resolve_width: 13px; $sections_vertical_gutter: 8px; $bottom_scrolling_buffer: 25px; -#left-sidebar { - #user-list { - padding-left: 8.5px; - - #userlist-header { - padding-left: 3px; - } - } -} - .hashtag { &:empty { &::after { diff --git a/static/styles/right_sidebar.css b/static/styles/right_sidebar.css index cef6f929af..d2eb9ace98 100644 --- a/static/styles/right_sidebar.css +++ b/static/styles/right_sidebar.css @@ -267,7 +267,6 @@ $user_status_emoji_width: 24px; margin-top: 5px; } -/* This max-width must be synced with message_viewport.is_narrow */ @media (width < $xl_min) { #userlist-toggle { display: block; diff --git a/static/styles/typing_notifications.css b/static/styles/typing_notifications.css index b62cbd4d28..cd933c6d1f 100644 --- a/static/styles/typing_notifications.css +++ b/static/styles/typing_notifications.css @@ -10,7 +10,6 @@ margin: 0; } -/* This max-width must be synced with message_viewport.is_narrow */ @media (width < $xl_min) { #typing_notifications { margin-right: 7px; diff --git a/static/styles/zulip.css b/static/styles/zulip.css index a6bcec1477..d67d73bc4d 100644 --- a/static/styles/zulip.css +++ b/static/styles/zulip.css @@ -2846,7 +2846,6 @@ div.topic_edit_spinner .loading_indicator_spinner { } } -/* This max-width must be synced with message_viewport.is_narrow */ @media (width < $xl_min) { .app-main, .header-main { diff --git a/templates/zerver/help/include/sidebar_index.md b/templates/zerver/help/include/sidebar_index.md index 5fcb62666a..51d0c5d8a6 100755 --- a/templates/zerver/help/include/sidebar_index.md +++ b/templates/zerver/help/include/sidebar_index.md @@ -52,7 +52,6 @@ * [Enable emoticon translations](/help/enable-emoticon-translations) * [Configure default view](/help/configure-default-view) * [Enable full width display](/help/enable-full-width-display) -* [Display the buddy list on narrow screens](/help/move-the-users-list-to-the-left-sidebar) * [Manage your uploaded files](/help/manage-your-uploaded-files) ## Sending messages diff --git a/templates/zerver/help/move-the-users-list-to-the-left-sidebar.md b/templates/zerver/help/move-the-users-list-to-the-left-sidebar.md deleted file mode 100644 index 917a16fcc0..0000000000 --- a/templates/zerver/help/move-the-users-list-to-the-left-sidebar.md +++ /dev/null @@ -1,20 +0,0 @@ -# Move the users list to the left sidebar - -By default, the **User list** is displayed in the right sidebar, to the -right of the main message pane. However, in narrower windows (smaller than -1200 pixels) the right sidebar is hidden to save space. - -You can choose to move the **User list** to the left hand side, under the -**Stream list**, in narrower windows. - -### Move the users list - -{start_tabs} - -{settings_tab|display-settings} - -2. Under **Advanced**, select **Show user list on left sidebar in narrow windows**. - -3. Reload the page. - -{end_tabs} diff --git a/zerver/models.py b/zerver/models.py index 5d9e7f3adf..ade0160e95 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1485,10 +1485,13 @@ class UserBaseSettings(models.Model): created after the change. """ - # UI settings + ### Generic UI settings enter_sends = models.BooleanField(default=False) - # display settings + ### Display settings. ### + # left_side_userlist was removed from the UI in Zulip 6.0; the + # database model is being temporarily preserved in case we want to + # restore a version of the setting, preserving who had it enabled. left_side_userlist = models.BooleanField(default=False) default_language = models.CharField(default="en", max_length=MAX_LANGUAGE_ID_LENGTH) # This setting controls which view is rendered first when Zulip loads.