FIX: improves reliability of post-text-selection bar (#24093)

The main change made is to use `pointerdown` and `touchstart` for detecting click outside in `FloatKit`, the problem of using `click` is that it will trigger on `mouseup` which is not working well with `FloatKit` shown using `mousedown` (when we change selection with the `mousedown` for example) as the release will be interpreted as a click outside and close the menu. To solve this issue the previous code in `post-text-selection` was going through various hacks for detecting state of mouse which are not always very reliable.

The second fix is to exit earlier when selection didn't change.

This has been tested on chrome/firefox and safari (mobile) and seems to work reliably.

<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->

app/assets/javascripts/discourse/app/components/post-text-selection.gjs

@@ -13,7 +13,6 @@ import {
 import virtualElementFromTextRange from "discourse/lib/virtual-element-from-text-range";
 import { INPUT_DELAY } from "discourse-common/config/environment";
 import discourseDebounce from "discourse-common/lib/debounce";
-import discourseLater from "discourse-common/lib/later";
 import { bind } from "discourse-common/utils/decorators";
 import escapeRegExp from "discourse-common/utils/escape-regexp";
 
@@ -50,19 +49,18 @@ export default class PostTextSelection extends Component {
   runLoopHandlers = modifier(() => {
     return () => {
       cancel(this.selectionChangeHandler);
-      cancel(this.holdingMouseDownHandle);
     };
   });
 
   documentListeners = modifier(() => {
     document.addEventListener("mousedown", this.mousedown, { passive: true });
     document.addEventListener("mouseup", this.mouseup, { passive: true });
-    document.addEventListener("selectionchange", this.selectionchange);
+    document.addEventListener("selectionchange", this.onSelectionChanged);
 
     return () => {
       document.removeEventListener("mousedown", this.mousedown);
       document.removeEventListener("mouseup", this.mouseup);
-      document.removeEventListener("selectionchange", this.selectionchange);
+      document.removeEventListener("selectionchange", this.onSelectionChanged);
     };
   });
 
@@ -78,7 +76,6 @@ export default class PostTextSelection extends Component {
     super.willDestroy(...arguments);
 
     this.menuInstance?.destroy();
-    cancel(this.selectionChangedHandler);
   }
 
   @bind
@@ -89,9 +86,19 @@ export default class PostTextSelection extends Component {
 
   @bind
   async selectionChanged() {
-    let supportsFastEdit = this.canEditPost;
-    const selection = window.getSelection();
+    const _selectedText = selectedText();
 
+    // avoid hard loops in quote selection unconditionally
+    // this can happen if you triple click text in firefox
+    // it's also generally unecessary work to go
+    // through this if the selection hasn't changed
+    if (this.menuInstance?.expanded && this.prevSelection === _selectedText) {
+      return;
+    }
+
+    this.prevSelection = _selectedText;
+
+    const selection = window.getSelection();
     if (selection.isCollapsed) {
       if (!this.menuInstance?.expanded) {
         this.args.quoteState.clear();
@@ -127,7 +134,6 @@ export default class PostTextSelection extends Component {
       selectedNode().nodeType === Node.ELEMENT_NODE
         ? selectedNode()
         : selectedNode().parentElement;
-    const _selectedText = selectedText();
     const cooked =
       _selectedElement.querySelector(".cooked") ||
       _selectedElement.closest(".cooked");
@@ -157,6 +163,7 @@ export default class PostTextSelection extends Component {
     const quoteState = this.args.quoteState;
     quoteState.selected(postId, _selectedText, opts);
 
+    let supportsFastEdit = this.canEditPost;
     if (this.canEditPost) {
       const regexp = new RegExp(escapeRegExp(quoteState.buffer), "gi");
       const matches = cooked.innerHTML.match(regexp);
@@ -175,14 +182,6 @@ export default class PostTextSelection extends Component {
       }
     }
 
-    // avoid hard loops in quote selection unconditionally
-    // this can happen if you triple click text in firefox
-    if (this.menuInstance?.expanded && this.prevSelection === _selectedText) {
-      return;
-    }
-
-    this.prevSelection = _selectedText;
-
     // on Desktop, shows the button at the beginning of the selection
     // on Mobile, shows the button at the end of the selection
     const { isIOS, isAndroid, isOpera } = this.capabilities;
@@ -219,7 +218,7 @@ export default class PostTextSelection extends Component {
   onSelectionChanged() {
     const { isIOS, isWinphone, isAndroid } = this.capabilities;
     const wait = isIOS || isWinphone || isAndroid ? INPUT_DELAY : 100;
-    this.selectionChangedHandler = discourseDebounce(
+    this.selectionChangeHandler = discourseDebounce(
       this,
       this.selectionChanged,
       wait
@@ -228,36 +227,21 @@ export default class PostTextSelection extends Component {
 
   @bind
   mousedown(event) {
-    this.holdingMouseDown = false;
+    this.validMouseDown = true;
 
     if (!event.target.closest(".cooked")) {
+      this.validMouseDown = false;
+      return;
+    }
+  }
+
+  @bind
+  mouseup() {
+    if (!this.validMouseDown) {
       return;
     }
 
-    this.isMousedown = true;
-    this.holdingMouseDownHandler = discourseLater(() => {
-      this.holdingMouseDown = true;
-    }, 100);
-  }
-
-  @bind
-  async mouseup() {
-    this.prevSelection = null;
-    this.isMousedown = false;
-
-    if (this.holdingMouseDown) {
-      this.onSelectionChanged();
-    }
-  }
-
-  @bind
-  selectionchange() {
-    cancel(this.selectionChangeHandler);
-    this.selectionChangeHandler = discourseLater(() => {
-      if (!this.isMousedown) {
-        this.onSelectionChanged();
-      }
-    }, 100);
+    this.onSelectionChanged();
   }
 
   get post() {

app/assets/javascripts/discourse/tests/integration/components/float-kit/d-menu-test.js

@@ -152,7 +152,7 @@ module("Integration | Component | FloatKit | d-menu", function (hooks) {
       hbs`<span class="test">test</span><DMenu @inline={{true}} @label="label" @closeOnClickOutside={{true}}  />`
     );
     await open();
-    await click(".test");
+    await triggerEvent(".test", "pointerdown");
 
     assert.dom(".fk-d-menu").doesNotExist();
 
@@ -160,7 +160,7 @@ module("Integration | Component | FloatKit | d-menu", function (hooks) {
       hbs`<span class="test">test</span><DMenu @inline={{true}} @label="label" @closeOnClickOutside={{false}}  />`
     );
     await open();
-    await click(".test");
+    await triggerEvent(".test", "pointerdown");
 
     assert.dom(".fk-d-menu").exists();
   });

app/assets/javascripts/discourse/tests/integration/components/float-kit/d-tooltip-test.js

@@ -158,7 +158,7 @@ module("Integration | Component | FloatKit | d-tooltip", function (hooks) {
       hbs`<span class="test">test</span><DTooltip @inline={{true}} @label="label" @closeOnClickOutside={{true}} />`
     );
     await hover();
-    await click(".test");
+    await triggerEvent(".test", "pointerdown");
 
     assert.dom(".fk-d-tooltip").doesNotExist();
 
@@ -166,7 +166,7 @@ module("Integration | Component | FloatKit | d-tooltip", function (hooks) {
       hbs`<span class="test">test</span><DTooltip @inline={{true}} @label="label" @closeOnClickOutside={{false}} />`
     );
     await hover();
-    await click(".test");
+    await triggerEvent(".test", "pointerdown");
 
     assert.dom(".fk-d-tooltip").exists();
   });

app/assets/javascripts/discourse/tests/integration/components/widgets/post-test.js

@@ -1,6 +1,6 @@
 import { getOwner } from "@ember/application";
 import EmberObject from "@ember/object";
-import { click, render } from "@ember/test-helpers";
+import { click, render, triggerEvent } from "@ember/test-helpers";
 import { hbs } from "ember-cli-htmlbars";
 import { module, test } from "qunit";
 import { setupRenderingTest } from "discourse/tests/helpers/component-test";
@@ -574,7 +574,7 @@ module("Integration | Component | Widget | post", function (hooks) {
 
     assert.dom("[data-content][data-identifier='admin-post-menu']").exists();
 
-    await click(".post-menu-area");
+    await triggerEvent(".post-menu-area", "pointerdown");
     assert
       .dom("[data-content][data-identifier='admin-post-menu']")
       .doesNotExist("clicking outside clears the popup");

app/assets/javascripts/float-kit/addon/modifiers/close-on-click-outside.js

@@ -12,7 +12,8 @@ export default class FloatKitCloseOnClickOutside extends Modifier {
     this.closeFn = closeFn;
     this.trigger = trigger;
     this.element = element;
-    document.addEventListener("click", this.check, {
+
+    document.addEventListener("pointerdown", this.check, {
       passive: true,
     });
   }
@@ -22,6 +23,7 @@ export default class FloatKitCloseOnClickOutside extends Modifier {
     if (this.element.contains(event.target)) {
       return;
     }
+
     if (
       this.trigger instanceof HTMLElement &&
       this.trigger.contains(event.target)
@@ -33,6 +35,6 @@ export default class FloatKitCloseOnClickOutside extends Modifier {
   }
 
   cleanup() {
-    document.removeEventListener("click", this.check);
+    document.removeEventListener("pointerdown", this.check);
   }
 }