From 8a4ab79be24f2575a00ad885394797fd6c23ebf4 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Fri, 15 Dec 2023 09:29:17 -0500 Subject: [PATCH] DEV: Improve header offset calculation (#24910) --- .../discourse/app/components/site-header.js | 28 +++++++++++++------ .../discourse/components/navbar/index.gjs | 8 +----- .../stylesheets/common/chat-navbar.scss | 1 + 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js index 3794aafae0a..18ed8387322 100644 --- a/app/assets/javascripts/discourse/app/components/site-header.js +++ b/app/assets/javascripts/discourse/app/components/site-header.js @@ -448,27 +448,37 @@ export default SiteHeaderComponent.extend({ @bind updateHeaderOffset() { - let headerWrapTop = this.headerWrap.getBoundingClientRect().top; + // Safari likes overscolling the page (on both iOS and macOS). + // This shows up as a negative value in window.scrollY. + // We can use this to offset the headerWrap's top offset to avoid + // jitteriness and bad positioning. + const windowOverscroll = Math.min(0, window.scrollY); - if (headerWrapTop !== 0) { - headerWrapTop -= Math.max(0, document.body.getBoundingClientRect().top); - } + // The headerWrap's top offset can also be a negative value on Safari, + // because of the changing height of the viewport (due to the URL bar). + // For our use case, it's best to ensure this is clamped to 0. + const headerWrapTop = Math.max( + 0, + Math.floor(this.headerWrap.getBoundingClientRect().top) + ); + let offsetTop = headerWrapTop + windowOverscroll; if (DEBUG && isTesting()) { - headerWrapTop -= document + offsetTop -= document .getElementById("ember-testing-container") .getBoundingClientRect().top; - headerWrapTop -= 1; // For 1px border on testing container + offsetTop -= 1; // For 1px border on testing container } const documentStyle = document.documentElement.style; - const currentValue = documentStyle.getPropertyValue("--header-offset"); - const newValue = `${this.headerWrap.offsetHeight + headerWrapTop}px`; + const currentValue = + parseInt(documentStyle.getPropertyValue("--header-offset"), 10) || 0; + const newValue = this.headerWrap.offsetHeight + offsetTop; if (currentValue !== newValue) { - documentStyle.setProperty("--header-offset", newValue); + documentStyle.setProperty("--header-offset", `${newValue}px`); } }, diff --git a/plugins/chat/assets/javascripts/discourse/components/navbar/index.gjs b/plugins/chat/assets/javascripts/discourse/components/navbar/index.gjs index 1f1a90b7ec9..b342d3cee58 100644 --- a/plugins/chat/assets/javascripts/discourse/components/navbar/index.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/navbar/index.gjs @@ -1,18 +1,12 @@ import Component from "@glimmer/component"; import { action } from "@ember/object"; import { inject as service } from "@ember/service"; -import { htmlSafe } from "@ember/template"; import DButton from "discourse/components/d-button"; -import { headerOffset } from "discourse/lib/offset-calculator"; import DiscourseURL from "discourse/lib/url"; export default class ChatNavbar extends Component { @service chatStateManager; - get topStyle() { - return htmlSafe(`top: ${headerOffset()}px`); - } - @action async closeFullScreen() { this.chatStateManager.prefersDrawer(); @@ -26,7 +20,7 @@ export default class ChatNavbar extends Component { }