From a78dc4a2bd5ae8735276d6e650bb2ec6de8440c4 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Sat, 20 May 2023 20:00:21 +0530 Subject: [PATCH] css: Scroll on `html` instead of `.app`. --- templates/zerver/app/index.html | 7 ++-- web/src/compose_actions.js | 3 +- web/src/compose_recipient.js | 3 +- web/src/compose_ui.js | 5 +-- web/src/message_edit.js | 4 +-- web/src/message_list_view.js | 10 +++--- web/src/message_scroll.js | 2 +- web/src/message_viewport.js | 25 +++++++------- web/src/narrow.js | 6 ++-- web/src/overlays.ts | 5 ++- web/src/popovers.js | 11 +++--- web/src/reload.js | 4 +-- web/src/resize.js | 11 ++++++ web/src/scroll_bar.ts | 39 ---------------------- web/src/setup.js | 4 +++ web/src/ui_init.js | 2 +- web/styles/zulip.css | 20 +++++++---- web/templates/navbar.hbs | 4 +-- web/tests/compose_ui.test.js | 14 +------- web/tests/narrow_activate.test.js | 2 +- web/tests/popovers.test.js | 1 + web/third/bootstrap-tooltip/tooltip.js | 4 +-- web/third/bootstrap-typeahead/typeahead.js | 2 +- 23 files changed, 80 insertions(+), 108 deletions(-) diff --git a/templates/zerver/app/index.html b/templates/zerver/app/index.html index 6f501c4998..8118c6e256 100644 --- a/templates/zerver/app/index.html +++ b/templates/zerver/app/index.html @@ -104,6 +104,7 @@ text-decoration: underline; } + {% endblock %} {% block content %} @@ -202,8 +203,10 @@ - - +
diff --git a/web/src/compose_actions.js b/web/src/compose_actions.js index 484db7f140..beefdd526a 100644 --- a/web/src/compose_actions.js +++ b/web/src/compose_actions.js @@ -128,7 +128,8 @@ export function maybe_scroll_up_selected_message() { return; } - const cover = $selected_row.offset().top + $selected_row.height() - $("#compose").offset().top; + const cover = + $selected_row.get_offset_to_window().bottom - $("#compose").get_offset_to_window().top; if (cover > 0) { message_viewport.user_initiated_animate_scroll(cover + 20); } diff --git a/web/src/compose_recipient.js b/web/src/compose_recipient.js index 30559ad27e..ec3a84a24e 100644 --- a/web/src/compose_recipient.js +++ b/web/src/compose_recipient.js @@ -286,7 +286,8 @@ function compose_recipient_dropdown_on_show(dropdown) { const window_height = window.innerHeight; const search_box_and_padding_height = 50; // pixels above compose box. - const recipient_input_top = $("#compose_recipient_selection_dropdown").offset().top; + const recipient_input_top = $("#compose_recipient_selection_dropdown").get_offset_to_window() + .top; const top_space = recipient_input_top - top_offset - search_box_and_padding_height; // pixels below compose starting from top of compose box. const bottom_space = window_height - recipient_input_top - search_box_and_padding_height; diff --git a/web/src/compose_ui.js b/web/src/compose_ui.js index a61d0192ce..83292df3c0 100644 --- a/web/src/compose_ui.js +++ b/web/src/compose_ui.js @@ -260,10 +260,7 @@ export function set_compose_box_top(set_top) { // using CSS. If that wasn't the case, we could have somehow // refactored the HTML so as to consider only the space below // below the `#navbar_alerts` as `height: 100%` of `#compose`. - const compose_top = - $("#navbar_alerts_wrapper").height() + - $(".header").height() + - Number.parseInt($(".header").css("paddingBottom"), 10); + const compose_top = $("#navbar-fixed-container").height(); $("#compose").css("top", compose_top + "px"); } else { $("#compose").css("top", ""); diff --git a/web/src/message_edit.js b/web/src/message_edit.js index a4ffb0f5fd..ce556f5491 100644 --- a/web/src/message_edit.js +++ b/web/src/message_edit.js @@ -566,8 +566,8 @@ function edit_message($row, raw_content) { function start_edit_maintaining_scroll($row, content) { edit_message($row, content); - const row_bottom = $row.height() + $row.offset().top; - const composebox_top = $("#compose").offset().top; + const row_bottom = $row.get_offset_to_window().bottom; + const composebox_top = $("#compose").get_offset_to_window().top; if (row_bottom > composebox_top) { message_viewport.scrollTop(message_viewport.scrollTop() + row_bottom - composebox_top); } diff --git a/web/src/message_list_view.js b/web/src/message_list_view.js index a66a800344..a8f56bfa4f 100644 --- a/web/src/message_list_view.js +++ b/web/src/message_list_view.js @@ -778,7 +778,7 @@ export class MessageListView { const save_scroll_position = () => { if (orig_scrolltop_offset === undefined && this.selected_row().length > 0) { - orig_scrolltop_offset = this.selected_row().offset().top; + orig_scrolltop_offset = this.selected_row().get_offset_to_window().top; } }; @@ -977,7 +977,7 @@ export class MessageListView { // it's the max amount that we can scroll down (or "skooch // up" the messages) before knocking the selected message // out of the feed. - const selected_row_top = $selected_row.offset().top; + const selected_row_top = $selected_row.get_offset_to_window().top; let scroll_limit = selected_row_top - viewport_info.visible_top; if (scroll_limit < 0) { @@ -1146,7 +1146,7 @@ export class MessageListView { const $selected_row = this.selected_row(); const selected_in_view = $selected_row.length > 0; if (selected_in_view) { - old_offset = $selected_row.offset().top; + old_offset = $selected_row.get_offset_to_window().top; } if (discard_rendering_state) { // If we know that the existing render is invalid way @@ -1160,7 +1160,9 @@ export class MessageListView { set_message_offset(offset) { const $msg = this.selected_row(); - message_viewport.scrollTop(message_viewport.scrollTop() + $msg.offset().top - offset); + message_viewport.scrollTop( + message_viewport.scrollTop() + $msg.get_offset_to_window().top - offset, + ); } rerender_with_target_scrolltop(selected_row, target_offset) { diff --git a/web/src/message_scroll.js b/web/src/message_scroll.js index 292ff71b6d..191a3fe32d 100644 --- a/web/src/message_scroll.js +++ b/web/src/message_scroll.js @@ -204,7 +204,7 @@ function scroll_finish() { } export function initialize() { - message_viewport.$message_pane.on( + $(document).on( "scroll", _.throttle(() => { unread_ops.process_visible(); diff --git a/web/src/message_viewport.js b/web/src/message_viewport.js index 4fd801ea92..cf8743e34f 100644 --- a/web/src/message_viewport.js +++ b/web/src/message_viewport.js @@ -56,11 +56,10 @@ export function message_viewport_info() { const res = {}; - const $element_just_above_us = $("#navbar-container .header"); + const $element_just_above_us = $("#navbar-fixed-container"); const $element_just_below_us = $("#compose"); - res.visible_top = - $element_just_above_us.offset().top + $element_just_above_us.safeOuterHeight(); + res.visible_top = $element_just_above_us.safeOuterHeight(); const $sticky_header = $(".sticky_header"); if ($sticky_header.length) { @@ -119,7 +118,7 @@ export function offset_from_bottom($last_row) { // A positive return value here means the last row is // below the bottom of the feed (i.e. obscured by the compose // box or even further below the bottom). - const message_bottom = $last_row.offset().top + $last_row.height(); + const message_bottom = $last_row.get_offset_to_window().bottom; const info = message_viewport_info(); return message_bottom - info.visible_bottom; @@ -188,8 +187,8 @@ function add_to_visible( const top_of_feed = new util.CachedValue({ compute_value() { - const $header = $("#navbar-container .header"); - let visible_top = $header.offset().top + $header.safeOuterHeight(); + const $header = $("#navbar-fixed-container"); + let visible_top = $header.safeOuterHeight(); const $sticky_header = $(".sticky_header"); if ($sticky_header.length) { @@ -290,7 +289,7 @@ export function scrollTop(target_scrollTop) { } let $ret = $message_pane.scrollTop(target_scrollTop); const new_scrollTop = $message_pane.scrollTop(); - const space_to_scroll = $("#bottom_whitespace").offset().top - height(); + const space_to_scroll = $("#bottom_whitespace").get_offset_to_window().top - height(); // Check whether our scrollTop didn't move even though one could have scrolled down if ( @@ -362,7 +361,7 @@ export function recenter_view($message, {from_scroll = false, force_center = fal const bottom_threshold = viewport_info.visible_bottom; - const message_top = $message.offset().top; + const message_top = $message.get_offset_to_window().top; const message_height = $message.safeOuterHeight(true); const message_bottom = message_top + message_height; @@ -395,7 +394,7 @@ export function maybe_scroll_to_show_message_top() { // Only applies if the top of the message is out of view above the visible area. const $selected_message = message_lists.current.selected_row(); const viewport_info = message_viewport_info(); - const message_top = $selected_message.offset().top; + const message_top = $selected_message.get_offset_to_window().top; const message_height = $selected_message.safeOuterHeight(true); if (message_top < viewport_info.visible_top) { set_message_position(message_top, message_height, viewport_info, 0); @@ -405,7 +404,7 @@ export function maybe_scroll_to_show_message_top() { export function is_message_below_viewport($message_row) { const info = message_viewport_info(); - const offset = $message_row.offset(); + const offset = $message_row.get_offset_to_window(); return offset.top >= info.visible_bottom; } @@ -432,7 +431,7 @@ export function keep_pointer_in_view() { return true; } - const message_top = $next_row.offset().top; + const message_top = $next_row.get_offset_to_window().top; // If the message starts after the very top of the screen, we just // leave it alone. This avoids bugs like #1608, where overzealousness @@ -453,7 +452,7 @@ export function keep_pointer_in_view() { } function message_is_far_enough_up() { - return at_bottom() || $next_row.offset().top <= bottom_threshold; + return at_bottom() || $next_row.get_offset_to_window().top <= bottom_threshold; } function adjust(in_view, get_next_row) { @@ -481,7 +480,7 @@ export function keep_pointer_in_view() { export function initialize() { $jwindow = $(window); - $message_pane = $(".app"); + $message_pane = $("html"); // This handler must be placed before all resize handlers in our application $jwindow.on("resize", () => { dimensions.height.reset(); diff --git a/web/src/narrow.js b/web/src/narrow.js index 2ba9460d20..164359526e 100644 --- a/web/src/narrow.js +++ b/web/src/narrow.js @@ -64,7 +64,9 @@ export function save_pre_narrow_offset_for_reload() { render_end: message_lists.current.view._render_win_end, }); } - message_lists.current.pre_narrow_offset = message_lists.current.selected_row().offset().top; + message_lists.current.pre_narrow_offset = message_lists.current + .selected_row() + .get_offset_to_window().top; } } @@ -409,7 +411,7 @@ export function activate(raw_operators, opts) { if (opts.then_select_offset === undefined) { const $row = message_lists.current.get_row(opts.then_select_id); if ($row.length > 0) { - opts.then_select_offset = $row.offset().top; + opts.then_select_offset = $row.get_offset_to_window().top; } } } diff --git a/web/src/overlays.ts b/web/src/overlays.ts index 8e5ed601f4..b5f1053ef0 100644 --- a/web/src/overlays.ts +++ b/web/src/overlays.ts @@ -137,7 +137,7 @@ export function open_overlay(opts: OverlayOptions): void { opts.$overlay.addClass("show"); opts.$overlay.attr("aria-hidden", "false"); $(".app").attr("aria-hidden", "true"); - $(".header").attr("aria-hidden", "true"); + $("#navbar-fixed-container").attr("aria-hidden", "true"); } // If conf.autoremove is true, the modal element will be removed from the DOM @@ -240,7 +240,6 @@ export function open_modal( } close_modal(modal_id); }); - Micromodal.show(modal_id, { disableFocus: true, openClass: "modal--opening", @@ -272,7 +271,7 @@ export function close_overlay(name: string): void { active_overlay.$element.attr("aria-hidden", "true"); $(".app").attr("aria-hidden", "false"); - $(".header").attr("aria-hidden", "false"); + $("#navbar-fixed-container").attr("aria-hidden", "false"); active_overlay.close_handler(); } diff --git a/web/src/popovers.js b/web/src/popovers.js index b2538d7314..0278887a24 100644 --- a/web/src/popovers.js +++ b/web/src/popovers.js @@ -163,7 +163,7 @@ function load_medium_avatar(user, $elt) { } function calculate_info_popover_placement(size, $elt) { - const ypos = $elt.offset().top; + const ypos = $elt.get_offset_to_window().top; if (!(ypos + size / 2 < message_viewport.height() && ypos > size / 2)) { if (ypos + size < message_viewport.height()) { @@ -303,10 +303,7 @@ function render_user_info_popover( const $popover_content = $(render_user_info_popover_content(args)); popover_element.popover({ content: $popover_content.get(0), - // TODO: Determine whether `fixed` should be applied - // unconditionally. Right now, we only do it for the user - // sidebar version of the popover. - fixed: template_class === "user_popover", + fixed: true, placement: popover_placement, template: render_no_arrow_popover({class: template_class}), title: render_user_info_popover_title({ @@ -316,7 +313,7 @@ function render_user_info_popover( }), html: true, trigger: "manual", - top_offset: $("#userlist-title").offset().top + 15, + top_offset: $("#userlist-title").get_offset_to_window().top + 15, fix_positions: true, }); popover_element.popover("show"); @@ -709,7 +706,7 @@ export function toggle_playground_link_popover(element, playground_info) { } const $elt = $(element); if ($elt.data("popover") === undefined) { - const ypos = $elt.offset().top; + const ypos = $elt.get_offset_to_window().top; $elt.popover({ // It's unlikely we'll have more than 3-4 playground links // for one language, so it should be OK to hardcode 120 here. diff --git a/web/src/reload.js b/web/src/reload.js index f809ed7ba2..d15310b26c 100644 --- a/web/src/reload.js +++ b/web/src/reload.js @@ -79,7 +79,7 @@ function preserve_state(send_after_reload, save_pointer, save_narrow, save_compo const $row = message_lists.home.selected_row(); if (!narrow_state.active()) { if ($row.length > 0) { - url += "+offset=" + $row.offset().top; + url += "+offset=" + $row.get_offset_to_window().top; } } else { url += "+offset=" + message_lists.home.pre_narrow_offset; @@ -92,7 +92,7 @@ function preserve_state(send_after_reload, save_pointer, save_narrow, save_compo } const $narrow_row = message_lists.current.selected_row(); if ($narrow_row.length > 0) { - url += "+narrow_offset=" + $narrow_row.offset().top; + url += "+narrow_offset=" + $narrow_row.get_offset_to_window().top; } } } diff --git a/web/src/resize.js b/web/src/resize.js index 3e1e8727cc..d5dba39bd9 100644 --- a/web/src/resize.js +++ b/web/src/resize.js @@ -145,8 +145,19 @@ export function resize_sidebars() { return h; } +export function reposition_message_header() { + // Since `navbar_alerts_wrapper`'s height can vary based on text / language, we + // need to adjust at what `top` position do `message-header` becomes `sticky`. + // Best way to do this is via adding custom CSS to the DOM instead of running endless + // javascript queries to find and update them on various re-renders. + const navbar_fixed_height = $("#navbar-fixed-container").safeOuterHeight(true); + const style = document.querySelector("#sticky_message_header_styles"); + style.textContent = `.message_list .message_header { top: ${navbar_fixed_height}px !important; }`; +} + export function resize_page_components() { navbar_alerts.resize_app(); + reposition_message_header(); const h = resize_sidebars(); resize_bottom_whitespace(h); } diff --git a/web/src/scroll_bar.ts b/web/src/scroll_bar.ts index cdcd180185..8d898a49e2 100644 --- a/web/src/scroll_bar.ts +++ b/web/src/scroll_bar.ts @@ -2,37 +2,6 @@ import $ from "jquery"; import {user_settings} from "./user_settings"; -// A few of our width properties in Zulip depend on the width of the -// browser scrollbar that is generated at the far right side of the -// page, which unfortunately varies depending on the browser and -// cannot be detected directly using CSS. As a result, we adjust a -// number of element widths based on the value detected here. -// -// From https://stackoverflow.com/questions/13382516/getting-scroll-bar-width-using-javascript -function getScrollbarWidth(): number { - const outer = document.createElement("div"); - outer.style.visibility = "hidden"; - outer.style.width = "100px"; - - document.body.append(outer); - - const widthNoScroll = outer.offsetWidth; - // force scrollbars - outer.style.overflow = "scroll"; - - // add inner div - const inner = document.createElement("div"); - inner.style.width = "100%"; - outer.append(inner); - - const widthWithScroll = inner.offsetWidth; - - // remove divs - outer.remove(); - - return widthNoScroll - widthWithScroll; -} - export function set_layout_width(): void { if (user_settings.fluid_layout_width) { $(".header-main, .app .app-main, #compose-container").css("max-width", "inherit"); @@ -41,14 +10,6 @@ export function set_layout_width(): void { } } -let sbWidth: number; - export function initialize(): void { - // Workaround for browsers with fixed scrollbars - sbWidth = getScrollbarWidth(); - if (sbWidth > 0) { - // Reduce width of screen-wide parent containers, whose width doesn't vary with scrollbar width, by scrollbar width. - $("#navbar-container .header, #compose").css("width", `calc(100% - ${sbWidth}px)`); - } set_layout_width(); } diff --git a/web/src/setup.js b/web/src/setup.js index 6c1ccd5dab..726deef7b7 100644 --- a/web/src/setup.js +++ b/web/src/setup.js @@ -31,6 +31,10 @@ $(() => { return this.outerHeight(...args) || 0; }; + $.fn.get_offset_to_window = function () { + return this[0].getBoundingClientRect(); + }; + $.fn.safeOuterWidth = function (...args) { return this.outerWidth(...args) || 0; }; diff --git a/web/src/ui_init.js b/web/src/ui_init.js index cf5e685213..e91f420568 100644 --- a/web/src/ui_init.js +++ b/web/src/ui_init.js @@ -221,7 +221,7 @@ function initialize_navbar() { search_pills_enabled: page_params.search_pills_enabled, }); - $("#navbar-container").html(rendered_navbar); + $("#header-container").html(rendered_navbar); } function initialize_compose_box() { diff --git a/web/styles/zulip.css b/web/styles/zulip.css index ca72800abe..c5c53f1585 100644 --- a/web/styles/zulip.css +++ b/web/styles/zulip.css @@ -1,10 +1,13 @@ +html { + overflow-y: scroll; + overflow-x: hidden; + overscroll-behavior-y: none; +} + body, html { width: 100%; height: 100%; - overflow-x: hidden; - overflow-y: hidden; - touch-action: manipulation; } @@ -359,9 +362,14 @@ p.n-margin { opacity: 1; } -.header { +#navbar-fixed-container { position: fixed; - z-index: 102; /* Needs to be higher than .alert-bar-container */ + top: 0; + z-index: 102; + width: 100%; +} + +.header { width: 100%; height: var(--header-height); /* Since the headers are sticky, we need non-transparent background. */ @@ -485,9 +493,7 @@ p.n-margin { .app { width: 100%; height: 100%; - overflow-y: scroll; z-index: 99; - -webkit-overflow-scrolling: touch; } .app-main, diff --git a/web/templates/navbar.hbs b/web/templates/navbar.hbs index 8de0679aa5..49fcdf2bd7 100644 --- a/web/templates/navbar.hbs +++ b/web/templates/navbar.hbs @@ -1,5 +1,5 @@
- {{!-- remove `top_navbar_full_width` wrapper div once the scrollbar is on the `html` element, and then move the styles on `header` to `navbar-container --}} + {{!-- remove `top_navbar_full_width` wrapper div once the scrollbar is on the `html` element, and then move the styles on `header` to `header-container --}}