FIX: improves text selection of posts #33143

jjaffeux · 2025-06-10T09:43:05Z

This commit is applying different techniques to make selecting text of a post less error prone:

ensure we only ever show the toolbar when the common ancestor of a range is cooked
ensures the menu is not interfering with text selection
do not compute the menu while selection is changing, only when pointer is released

The situation was very bad on android but it should also improve other situations.

jjaffeux · 2025-06-10T09:43:47Z

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

-    // ensure we selected content inside 1 post *only*
-    let postId;
-    for (let r = 0; r < selection.rangeCount; r++) {
-      const range = selection.getRangeAt(r);
-      const selectionStart = getElement(range.startContainer);
-      const ancestor = getElement(range.commonAncestorContainer);
-
-      if (!selectionStart.closest(".cooked")) {
-        return await this.hideToolbar();
-      }
-
-      postId ||= ancestor.closest(".boxed, .reply")?.dataset?.postId;
-
-      if (!ancestor.closest(".contents") || !postId) {
-        return await this.hideToolbar();
-      }


we now receive the cooked from the onSelectionChanged event

jjaffeux · 2025-06-10T09:43:58Z

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

-    const _selectedElement = getElement(selectedNode());
-    const cooked =
-      _selectedElement.querySelector(".cooked") ||
-      _selectedElement.closest(".cooked");
-


cooked is received from onSelectionChange event

jjaffeux · 2025-06-10T09:44:14Z

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

@@ -220,7 +232,8 @@ export default class PostTextSelection extends Component {
        //   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 { isAndroid } = this.capabilities;
+        offset = isAndroid ? 90 : 70;


THe menu needs more spacing on android

Should we just increase it everywhere for consistency?

It's just the native toolbar on android is bigger, but we can try the same if you think that's better

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

app/assets/stylesheets/common/base/topic-post.scss

davidtaylorhq · 2025-06-10T14:16:46Z

app/assets/stylesheets/common/base/topic-post.scss

+  &:not(.ios-device) body.-disable-global-text-selection {
+    // prevents hovering toolbar menu to confuse text selection on android


Can we just do the same thing on iOS for consistency? Or does it cause issues there?

it does nothing on iOS I think so yes I can probably just add it too

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

app/assets/stylesheets/common/base/topic-post.scss

This commit is applying different techniques to make selecting text of a post less error prone: - disables text selection of all the page BUT the current post - ensures the selection menu is not interfering with text selection - ensures the d-header is not interfering with text selection The situation was very bad on android but it should also improve other situations.

jjaffeux · 2025-06-11T16:35:02Z

app/assets/javascripts/discourse/app/lib/utilities.js

-    // ensure we never quote text in the post menu area
-    const postMenuArea = ancestor.querySelector(".post-menu-area");
-    if (postMenuArea) {
-      range.setEndBefore(postMenuArea);
-    }
-


I think we should decide this outside of this helper, setEndBefore is messing with browser behavior and can end up creating weird behaviors where the text selection is changing after cursor release, which is a very janky feeling.

jjaffeux · 2025-06-11T16:36:06Z

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

+      const parent =
+        range.commonAncestorContainer.nodeType === Node.TEXT_NODE
+          ? range.commonAncestorContainer.parentNode
+          : range.commonAncestorContainer;
+
+      const cooked = parent.closest(".cooked");


This is mostly my solution to ensure we don't show the toolbar when selecting outside of one post, if the common ancestor to a range selection is not a cooked, it means we are selecting outside of a post, or two posts, or a post and something else...

This is needed for discourse/discourse#33143 as we now rely on this pointerup event.

jjaffeux · 2025-06-11T18:54:15Z

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


-    const { isIOS, isWinphone, isAndroid } = this.capabilities;
-    const wait = isIOS || isWinphone || isAndroid ? INPUT_DELAY : 25;
-    this.selectionChangeHandler = discourseDebounce(


now that we rely on pointerup and contextmenu and not selectionchange, we don't need debouncing anymore.

This reverts commit 51c9bb4.

davidtaylorhq · 2025-06-11T21:23:15Z

app/assets/stylesheets/mobile/topic-post.scss

+}
+
+// most importantly for android to prevent the quote menu to interfere with the text selection
+[data-identifier="post-text-selection-toolbar"].-disable-pointer-events,


Would it be easy to make a class for this element? Selecting by a data attribute feels a little weird

davidtaylorhq · 2025-06-11T21:27:30Z

app/assets/stylesheets/mobile/topic-post.scss

+
+// most importantly for android to prevent the quote menu to interfere with the text selection
+[data-identifier="post-text-selection-toolbar"].-disable-pointer-events,
+[data-identifier="post-text-selection-toolbar"].-disable-pointer-events * {


Does it work if we do this?

Suggested change

[data-identifier="post-text-selection-toolbar"].-disable-pointer-events * {

[data-identifier="post-text-selection-toolbar"].-disable-pointer-events {

If I'm understanding MDN correctly, pointer-events:none applies to children as long as they don't explicitly set pointer-events: auto 👀

davidtaylorhq · 2025-06-11T21:29:21Z

app/assets/stylesheets/mobile/topic-post.scss

@@ -323,6 +323,16 @@ blockquote {
  margin-right: 0;
 }

+.d-header-wrap * {


Does it work without *? MDN page on user-select says:

the initial auto value makes it behave like it is inherited most of the time

This is needed for discourse/discourse#33143 as we now rely on this pointerup event.

This is needed for discourse/discourse#33143 Ideally we should move all these helpers to core, or improve the ones in core.

discoursebot · 2025-06-12T16:43:03Z

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/i-can-only-quote-one-word/369911/1

jjaffeux commented Jun 10, 2025

View reviewed changes

app/assets/javascripts/discourse/app/components/post-text-selection.gjs Outdated Show resolved Hide resolved

davidtaylorhq reviewed Jun 10, 2025

View reviewed changes

app/assets/stylesheets/common/base/topic-post.scss Outdated Show resolved Hide resolved

davidtaylorhq reviewed Jun 10, 2025

View reviewed changes

app/assets/javascripts/discourse/app/components/post-text-selection.gjs Outdated Show resolved Hide resolved

davidtaylorhq reviewed Jun 10, 2025

View reviewed changes

app/assets/stylesheets/common/base/topic-post.scss Outdated Show resolved Hide resolved

jjaffeux force-pushed the better-text-selection-android branch from fe2750f to a929410 Compare June 11, 2025 16:25

linting

e4a3f75

jjaffeux commented Jun 11, 2025

View reviewed changes

jjaffeux added 3 commits June 11, 2025 20:24

simpler code

122d883

clear quote state

3739b93

no debounce

51c9bb4

jjaffeux added a commit to discourse/discourse-ai that referenced this pull request Jun 11, 2025

DEV: a real selection change has a pointerup event

3ba7062

This is needed for discourse/discourse#33143 as we now rely on this pointerup event.

jjaffeux mentioned this pull request Jun 11, 2025

DEV: a real selection change has a pointerup event discourse/discourse-ai#1427

Merged

jjaffeux commented Jun 11, 2025

View reviewed changes

Revert "no debounce"

09ff69f

This reverts commit 51c9bb4.

davidtaylorhq reviewed Jun 11, 2025

View reviewed changes

jjaffeux added 2 commits June 12, 2025 00:23

simplify selectors

5e3b018

fix specs

c7894f4

jjaffeux added a commit to discourse/discourse-ai that referenced this pull request Jun 11, 2025

DEV: a real selection change has a pointerup event (#1427)

26217e5

This is needed for discourse/discourse#33143 as we now rely on this pointerup event.

jjaffeux added a commit to discourse/discourse-ai that referenced this pull request Jun 11, 2025

DEV: adds pointerup event to our select text helper

7515994

This is needed for discourse/discourse#33143 Ideally we should move all these helpers to core, or improve the ones in core.

jjaffeux mentioned this pull request Jun 11, 2025

DEV: adds pointerup event to our select text helper discourse/discourse-ai#1429

Merged

jjaffeux added a commit to discourse/discourse-ai that referenced this pull request Jun 12, 2025

DEV: adds pointerup event to our select text helper (#1429)

b5a2ee3

This is needed for discourse/discourse#33143 Ideally we should move all these helpers to core, or improve the ones in core.

davidtaylorhq approved these changes Jun 12, 2025

View reviewed changes

jjaffeux added 2 commits June 12, 2025 11:13

not used

66007f2

not used

96480a9

jjaffeux merged commit 876dbe6 into main Jun 12, 2025
28 of 29 checks passed

jjaffeux deleted the better-text-selection-android branch June 12, 2025 13:06

		&:not(.ios-device) body.-disable-global-text-selection {
		// prevents hovering toolbar menu to confuse text selection on android

	[data-identifier="post-text-selection-toolbar"].-disable-pointer-events * {
	[data-identifier="post-text-selection-toolbar"].-disable-pointer-events {

FIX: improves text selection of posts #33143

FIX: improves text selection of posts #33143

Uh oh!

Conversation

jjaffeux commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

discoursebot commented Jun 12, 2025

Uh oh!

Uh oh!

jjaffeux commented Jun 10, 2025 •

edited

Loading