From 80d2afa31627c6851f4d99a47563f5705a385fc8 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 19 Mar 2024 10:53:51 +0100 Subject: [PATCH] FIX: refreshes post toolbar on topic scroll (#26228) This commit now ensures we will properly attempt to refresh the toolbar position after a scroll and consider it as a selection change. Tangential to this fix we improved the positioning on mobile to better account for the native menu position and avoid a situation where the toolbar is always behind the native menu and can't be used. --- .../app/components/post-text-selection.gjs | 52 ++++++++++++++++--- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/post-text-selection.gjs b/app/assets/javascripts/discourse/app/components/post-text-selection.gjs index 5bfb62e8a83..773e85bfcf5 100644 --- a/app/assets/javascripts/discourse/app/components/post-text-selection.gjs +++ b/app/assets/javascripts/discourse/app/components/post-text-selection.gjs @@ -1,10 +1,11 @@ import Component from "@glimmer/component"; import { cached, tracked } from "@glimmer/tracking"; import { action } from "@ember/object"; -import { cancel } from "@ember/runloop"; +import { cancel, debounce } from "@ember/runloop"; import { service } from "@ember/service"; import { modifier } from "ember-modifier"; import PostTextSelectionToolbar from "discourse/components/post-text-selection-toolbar"; +import isElementInViewport from "discourse/lib/is-element-in-viewport"; import toMarkdown from "discourse/lib/to-markdown"; import { selectedNode, @@ -62,9 +63,15 @@ export default class PostTextSelection extends Component { }); appEventsListeners = modifier(() => { + this.appEvents.on("topic:current-post-scrolled", this, "handleTopicScroll"); this.appEvents.on("quote-button:quote", this, "insertQuote"); return () => { + this.appEvents.off( + "topic:current-post-scrolled", + this, + "handleTopicScroll" + ); this.appEvents.off("quote-button:quote", this, "insertQuote"); }; }); @@ -72,6 +79,7 @@ export default class PostTextSelection extends Component { willDestroy() { super.willDestroy(...arguments); + cancel(this.debouncedSelectionChanged); this.menuInstance?.destroy(); } @@ -81,8 +89,7 @@ export default class PostTextSelection extends Component { await this.menuInstance?.close(); } - @bind - async selectionChanged() { + async selectionChanged(options = {}) { if (this.isSelecting) { return; } @@ -102,6 +109,7 @@ export default class PostTextSelection extends Component { // it's also generally unecessary work to go // through this if the selection hasn't changed if ( + !options.force && this.menuInstance?.expanded && this.prevSelectedText === _selectedText ) { @@ -184,7 +192,26 @@ export default class PostTextSelection extends Component { } } - const options = { + let offset = 3; + if (this.shouldRenderUnder) { + // on mobile, we ideally want to show the toolbar at the end of the selection + offset = 20; + + if ( + !isElementInViewport(selectedRange().startContainer.parentNode) || + !isElementInViewport(selectedRange().endContainer.parentNode) + ) { + // we force a higher offset in two cases: + // - the start of the selection is not in viewport, in this case on iOS for example + // the native menu will be shown at the bottom of the screen, right after text selection + // so we need more space + // - the end of the selection is not in viewport, in this case our menu will be shown at the top + // of the screen, so we need more space to avoid overlapping with the native menu + offset = 70; + } + } + + const menuOptions = { identifier: "post-text-selection-toolbar", component: PostTextSelectionToolbar, inline: true, @@ -192,7 +219,7 @@ export default class PostTextSelection extends Component { fallbackPlacements: this.shouldRenderUnder ? ["bottom-end", "top-start"] : ["bottom-start"], - offset: this.shouldRenderUnder ? 25 : 3, + offset, trapTab: false, closeOnScroll: false, data: { @@ -212,7 +239,7 @@ export default class PostTextSelection extends Component { this.menuInstance = await this.menu.show( virtualElementFromTextRange(), - options + menuOptions ); } @@ -267,6 +294,19 @@ export default class PostTextSelection extends Component { return this.site.isMobileDevice || isIOS || isAndroid || isOpera; } + @action + handleTopicScroll() { + if (this.site.mobileView) { + this.debouncedSelectionChanged = debounce( + this, + this.selectionChanged, + { force: true }, + 250, + false + ); + } + } + @action async insertQuote() { await this.args.selectText();