From f67f4b6dfcb724c362909a573c418b0df78d402e Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Thu, 12 Dec 2024 06:33:26 -0500 Subject: [PATCH] DEV: Address composer mobile positioning issues --- .../discourse/app/components/composer-body.js | 10 -- .../app/components/d-virtual-height.gjs | 24 ++-- .../app/components/scrolling-post-stream.js | 7 +- .../app/components/topic-progress.js | 107 ++---------------- .../discourse/app/lib/put-cursor-at-end.js | 11 +- .../stylesheets/common/base/compose.scss | 62 +++++----- app/assets/stylesheets/common/base/modal.scss | 4 - .../stylesheets/common/base/topic-footer.scss | 16 --- app/assets/stylesheets/common/base/topic.scss | 6 + .../common/components/footer-nav.scss | 5 +- app/assets/stylesheets/mobile/compose.scss | 21 ++-- 11 files changed, 77 insertions(+), 196 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/composer-body.js b/app/assets/javascripts/discourse/app/components/composer-body.js index af2bca42844..24cb6017a00 100644 --- a/app/assets/javascripts/discourse/app/components/composer-body.js +++ b/app/assets/javascripts/discourse/app/components/composer-body.js @@ -3,7 +3,6 @@ import { cancel, schedule, throttle } from "@ember/runloop"; import { classNameBindings } from "@ember-decorators/component"; import { observes } from "@ember-decorators/object"; import { headerOffset } from "discourse/lib/offset-calculator"; -import positioningWorkaround from "discourse/lib/safari-hacks"; import { isiPad } from "discourse/lib/utilities"; import Composer from "discourse/models/composer"; import discourseDebounce from "discourse-common/lib/debounce"; @@ -68,13 +67,6 @@ export default class ComposerBody extends Component { }, 1000); } - @observes("composeState") - disableFullscreen() { - if (this.composeState !== Composer.OPEN && positioningWorkaround.blur) { - positioningWorkaround.blur(); - } - } - setupComposerResizeEvents() { this.origComposerSize = 0; this.lastMousePos = 0; @@ -184,8 +176,6 @@ export default class ComposerBody extends Component { triggerOpen(); } }); - - positioningWorkaround(this.element); } willDestroyElement() { diff --git a/app/assets/javascripts/discourse/app/components/d-virtual-height.gjs b/app/assets/javascripts/discourse/app/components/d-virtual-height.gjs index 4ab506df947..c33f1aa426f 100644 --- a/app/assets/javascripts/discourse/app/components/d-virtual-height.gjs +++ b/app/assets/javascripts/discourse/app/components/d-virtual-height.gjs @@ -1,12 +1,12 @@ import Component from "@glimmer/component"; import { cancel, scheduleOnce } from "@ember/runloop"; import { service } from "@ember/service"; -import { clearAllBodyScrollLocks } from "discourse/lib/body-scroll-lock"; +import { clearAllBodyScrollLocks, disableBodyScroll } from "discourse/lib/body-scroll-lock"; import isZoomed from "discourse/lib/zoom-check"; import discourseDebounce from "discourse-common/lib/debounce"; import { bind } from "discourse-common/utils/decorators"; -const KEYBOARD_DETECT_THRESHOLD = 150; +const FF_KEYBOARD_DETECT_THRESHOLD = 150; export default class DVirtualHeight extends Component { @service site; @@ -95,6 +95,7 @@ export default class DVirtualHeight extends Component { @bind onViewportResize() { this.setVH(); + const docEl = document.documentElement; let keyboardVisible = false; if ("virtualKeyboard" in navigator) { @@ -106,7 +107,7 @@ export default class DVirtualHeight extends Component { Math.abs( this.windowInnerHeight - Math.min(window.innerHeight, window.visualViewport.height) - ) > KEYBOARD_DETECT_THRESHOLD + ) > FF_KEYBOARD_DETECT_THRESHOLD ) { keyboardVisible = true; } @@ -121,7 +122,7 @@ export default class DVirtualHeight extends Component { // adds bottom padding when using a hardware keyboard and the accessory bar is visible // accessory bar height is 55px, using 75 allows a small buffer if (this.capabilities.isIpadOS) { - document.documentElement.style.setProperty( + docEl.style.setProperty( "--composer-ipad-padding", `${viewportWindowDiff < 75 ? viewportWindowDiff : 0}px` ); @@ -130,11 +131,20 @@ export default class DVirtualHeight extends Component { this.appEvents.trigger("keyboard-visibility-change", keyboardVisible); - keyboardVisible - ? document.documentElement.classList.add("keyboard-visible") - : document.documentElement.classList.remove("keyboard-visible"); + if (keyboardVisible) { + docEl.classList.add("keyboard-visible"); + + // disable body scroll in mobile composer + // we have to do this because we're positioning the composer with + // position: fixed and top: 0 and scrolling would move the composer halfway out of the viewport + // we can't use bottom: 0, it is very unreliable with keyboard visible + if (docEl.classList.contains("composer-open")) { + disableBodyScroll(document.querySelector("#reply-control")); + } + } if (!keyboardVisible) { + docEl.classList.remove("keyboard-visible"); clearAllBodyScrollLocks(); } } diff --git a/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js b/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js index 29063e90623..6a67001b634 100644 --- a/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js +++ b/app/assets/javascripts/discourse/app/components/scrolling-post-stream.js @@ -2,7 +2,6 @@ import { schedule, scheduleOnce } from "@ember/runloop"; import { service } from "@ember/service"; import MountWidget from "discourse/components/mount-widget"; import offsetCalculator from "discourse/lib/offset-calculator"; -import { isWorkaroundActive } from "discourse/lib/safari-hacks"; import DiscourseURL from "discourse/lib/url"; import { cloak, uncloak } from "discourse/widgets/post-stream"; import discourseDebounce from "discourse-common/lib/debounce"; @@ -64,11 +63,7 @@ export default class ScrollingPostStream extends MountWidget { return; } - if ( - isWorkaroundActive() || - document.webkitFullscreenElement || - document.fullscreenElement - ) { + if (document.webkitFullscreenElement || document.fullscreenElement) { return; } diff --git a/app/assets/javascripts/discourse/app/components/topic-progress.js b/app/assets/javascripts/discourse/app/components/topic-progress.js index e78ece26a5e..5f9fddfc002 100644 --- a/app/assets/javascripts/discourse/app/components/topic-progress.js +++ b/app/assets/javascripts/discourse/app/components/topic-progress.js @@ -3,17 +3,12 @@ import { action } from "@ember/object"; import { alias } from "@ember/object/computed"; import { scheduleOnce } from "@ember/runloop"; import { classNameBindings } from "@ember-decorators/component"; -import { isTesting } from "discourse-common/config/environment"; -import discourseLater from "discourse-common/lib/later"; -import discourseComputed, { bind } from "discourse-common/utils/decorators"; +import discourseComputed from "discourse-common/utils/decorators"; -const CSS_TRANSITION_DELAY = isTesting() ? 0 : 500; - -@classNameBindings("docked", "withTransitions") +@classNameBindings("docked") export default class TopicProgress extends Component { elementId = "topic-progress-wrapper"; docked = false; - withTransitions = null; progressPosition = null; @alias("topic.postStream") postStream; @@ -70,106 +65,20 @@ export default class TopicProgress extends Component { didInsertElement() { super.didInsertElement(...arguments); - this.appEvents - .on("composer:resized", this, this._composerEvent) - .on("topic:current-post-scrolled", this, this._topicScrolled); + this.appEvents.on("topic:current-post-scrolled", this, this._topicScrolled); if (this.prevEvent) { scheduleOnce("afterRender", this, this._topicScrolled, this.prevEvent); } - scheduleOnce("afterRender", this, this._startObserver); - - // start CSS transitions a tiny bit later - // to avoid jumpiness on initial topic load - discourseLater(this._addCssTransitions, CSS_TRANSITION_DELAY); } willDestroyElement() { super.willDestroyElement(...arguments); - this._topicBottomObserver?.disconnect(); - this.appEvents - .off("composer:resized", this, this._composerEvent) - .off("topic:current-post-scrolled", this, this._topicScrolled); - } - - @bind - _addCssTransitions() { - if (this.isDestroying || this.isDestroyed) { - return; - } - this.set("withTransitions", true); - } - - _startObserver() { - if ("IntersectionObserver" in window) { - this._topicBottomObserver = this._setupObserver(); - this._topicBottomObserver.observe( - document.querySelector("#topic-bottom") - ); - } - } - - _setupObserver() { - // minimum 50px here ensures element is not docked when - // scrolling down quickly, it causes post stream refresh loop - // on Android - const bottomIntersectionMargin = - document.querySelector("#reply-control")?.clientHeight || 50; - - return new IntersectionObserver(this._intersectionHandler, { - threshold: 1, - rootMargin: `0px 0px -${bottomIntersectionMargin}px 0px`, - }); - } - - _composerEvent() { - // reinitializing needed to account for composer height - // might be no longer necessary if IntersectionObserver API supports dynamic rootMargin - // see https://github.com/w3c/IntersectionObserver/issues/428 - if ("IntersectionObserver" in window) { - this._topicBottomObserver?.disconnect(); - this._startObserver(); - } - } - - @bind - _intersectionHandler(entries) { - if (!this.element || this.isDestroying || this.isDestroyed) { - return; - } - - const composerH = - document.querySelector("#reply-control")?.clientHeight || 0; - - // on desktop, pin this element to the composer - // otherwise the grid layout will change too much when toggling the composer - // and jitter when the viewport is near the topic bottom - if (this.site.desktopView && composerH) { - this.set("docked", false); - this.element.style.setProperty("bottom", `${composerH}px`); - return; - } - - if (entries[0].isIntersecting === true) { - this.set("docked", true); - this.element.style.removeProperty("bottom"); - } else { - if (entries[0].boundingClientRect.top > 0) { - this.set("docked", false); - if (composerH === 0) { - const filteredPostsHeight = - document.querySelector(".posts-filtered-notice")?.clientHeight || 0; - filteredPostsHeight === 0 - ? this.element.style.removeProperty("bottom") - : this.element.style.setProperty( - "bottom", - `${filteredPostsHeight}px` - ); - } else { - this.element.style.setProperty("bottom", `${composerH}px`); - } - } - } + this.appEvents.off( + "topic:current-post-scrolled", + this, + this._topicScrolled + ); } click(e) { diff --git a/app/assets/javascripts/discourse/app/lib/put-cursor-at-end.js b/app/assets/javascripts/discourse/app/lib/put-cursor-at-end.js index 3c5ade9f84c..3fff926badc 100644 --- a/app/assets/javascripts/discourse/app/lib/put-cursor-at-end.js +++ b/app/assets/javascripts/discourse/app/lib/put-cursor-at-end.js @@ -1,14 +1,5 @@ -import positioningWorkaround from "discourse/lib/safari-hacks"; -import { helperContext } from "discourse-common/lib/helpers"; - export default function (element) { - const caps = helperContext().capabilities; - - if (caps.isApple && positioningWorkaround.touchstartEvent) { - positioningWorkaround.touchstartEvent(element); - } else { - element.focus(); - } + element.focus(); const len = element.value.length; element.setSelectionRange(len, len); diff --git a/app/assets/stylesheets/common/base/compose.scss b/app/assets/stylesheets/common/base/compose.scss index 9bfae4b16d8..78c53985b30 100644 --- a/app/assets/stylesheets/common/base/compose.scss +++ b/app/assets/stylesheets/common/base/compose.scss @@ -1,9 +1,10 @@ -html.composer-open { +html.composer-open.not-mobile-device { #main-outlet { padding-bottom: var(--composer-height); transition: padding-bottom 250ms ease; } } + #reply-control { position: fixed; display: flex; @@ -71,10 +72,6 @@ html.composer-open { height: var(--composer-height); } - &.closed { - height: 0 !important; - } - &.draft, &.saving { height: 40px !important; @@ -613,36 +610,6 @@ div.ac-wrap { } } -body.ios-safari-composer-hacks { - #main-outlet, - header, - .grippie, - html:not(.fullscreen-composer) & .toggle-fullscreen { - display: none; - } - - #reply-control { - top: 0px; - &.open { - height: calc(var(--composer-vh, 1vh) * 100); - } - } -} - -body:not(.ios-safari-composer-hacks) { - #reply-control.open { - --min-height: 255px; - min-height: var(--min-height); - max-height: calc(100vh - var(--header-offset, 4em)); - &.composer-action-reply { - // we can let the reply composer get a little shorter - min-height: calc(var(--min-height) - 4em); - } - padding-bottom: var(--composer-ipad-padding); - box-sizing: border-box; - } -} - .toggle-preview { margin-left: auto; transition: all 0.33s ease-out; @@ -659,3 +626,28 @@ body:not(.ios-safari-composer-hacks) { .draft-error { color: var(--danger); } + +@keyframes blink_input_opacity_to_prevent_scrolling_when_focus { + 0% { + opacity: 0; + } + 100% { + opacity: 1; + } +} + +// When an element (input, textearea) gets focus, iOS Safari tries to put it in the center +// by scrolling and zooming. We handle zooming with a meta tag. We used to handle scrolling +// using a complicated JS hack. +// +// However, iOS Safari doesn't scroll when the input has opacity of 0 or is clipped. +// We use this quirk and temporarily hide the element on scroll and quickly show it again +// +// Source https://gist.github.com/kiding/72721a0553fa93198ae2bb6eefaa3299 + +.ios-device #reply-control { + input:focus, + textarea:focus { + animation: blink_input_opacity_to_prevent_scrolling_when_focus 0.01s; + } +} diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss index e1860380856..1ad8a3d848f 100644 --- a/app/assets/stylesheets/common/base/modal.scss +++ b/app/assets/stylesheets/common/base/modal.scss @@ -16,10 +16,6 @@ position: relative; } - html.keyboard-visible body.ios-safari-composer-hacks & { - height: calc(var(--composer-vh, 1vh) * 100); - } - &__container { display: flex; flex-direction: column; diff --git a/app/assets/stylesheets/common/base/topic-footer.scss b/app/assets/stylesheets/common/base/topic-footer.scss index abb6e7e0d97..c949a488c27 100644 --- a/app/assets/stylesheets/common/base/topic-footer.scss +++ b/app/assets/stylesheets/common/base/topic-footer.scss @@ -41,20 +41,12 @@ } #topic-progress-wrapper { - position: fixed; &.docked { - position: initial; - .toggle-admin-menu { display: none; } } - bottom: 0; - html:not(.footer-nav-visible) & { - bottom: env(safe-area-inset-bottom); - } - right: 10px; z-index: z("timeline"); display: flex; @@ -67,14 +59,6 @@ border: 0; } - &.with-transitions { - transition: bottom 0.2s, margin-bottom 0.2s; - - #topic-progress .bg { - transition: width 0.5s; - } - } - &:not(.docked) { @media screen and (min-width: $reply-area-max-width) { right: calc(50%); // right side of composer diff --git a/app/assets/stylesheets/common/base/topic.scss b/app/assets/stylesheets/common/base/topic.scss index 3daa7244031..756d863aef4 100644 --- a/app/assets/stylesheets/common/base/topic.scss +++ b/app/assets/stylesheets/common/base/topic.scss @@ -117,6 +117,12 @@ } } +.with-topic-progress { + position: sticky; + bottom: 0px; + z-index: z("timeline"); +} + .topic-status-info, .topic-timer-info { border-top: 1px solid var(--primary-low); diff --git a/app/assets/stylesheets/common/components/footer-nav.scss b/app/assets/stylesheets/common/components/footer-nav.scss index ad06d497166..2c42e74bf60 100644 --- a/app/assets/stylesheets/common/components/footer-nav.scss +++ b/app/assets/stylesheets/common/components/footer-nav.scss @@ -19,9 +19,10 @@ html.footer-nav-visible { padding-bottom: 0px; } - #topic-progress-wrapper:not(.docked) { - margin-bottom: calc(var(--footer-nav-height) + env(safe-area-inset-bottom)); + .with-topic-progress { + bottom: var(--footer-nav-height); } + .posts-filtered-notice { transition: all linear 0.1s; bottom: calc(var(--footer-nav-height) + env(safe-area-inset-bottom)); diff --git a/app/assets/stylesheets/mobile/compose.scss b/app/assets/stylesheets/mobile/compose.scss index dcb7b0228fb..d2c571a838f 100644 --- a/app/assets/stylesheets/mobile/compose.scss +++ b/app/assets/stylesheets/mobile/compose.scss @@ -7,7 +7,16 @@ } #reply-control { - z-index: z("mobile-composer"); + // We override values set in base/compose.scss so that on mobile, the composer is top-anchored. + // This is not ideal, but it is much better than bottom-anchoring. + // Mobile browsers (especially Safari and Firefox/Android) do not handle well + // a bottom/fixed-positioned element, especially while the software keyboard is visible. + top: 0; + // With height and z-index: -1, the composer jitters a lot less + height: 100%; + z-index: -1; + max-height: unset; + min-height: unset; .reply-area { padding: 6px; @@ -16,15 +25,12 @@ } &.open { - height: 250px; - &.edit-title { - height: 100%; - } + display: flex; + z-index: z("mobile-composer"); + height: calc(var(--composer-vh, 1vh) * 100); } .keyboard-visible &.open { - top: 0px; - height: calc(var(--composer-vh, 1vh) * 100); .reply-area { padding-bottom: 6px; } @@ -64,6 +70,7 @@ } &.draft { + top: calc(100% - 40px); z-index: z("footer-nav") + 1; padding-bottom: env(safe-area-inset-bottom);