[$1000] Clicking outside of the comment text won’t unselect the highlighted text

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open web
2. Go to any chat
3. Highlight range of text
4. Click on empty space on the left

Expected Result:

The text selected range should be reset / unselected

Actual Result:

The text selection range stays highlighted

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.56-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Recording.1327.mp4

Screen.Recording.2023-01-18.at.10.55.11.AM.1.mov

Expensify/Expensify Issue URL:
Issue reported by: @getusha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1674068409564519

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e5387a24edfdca63
• Upwork Job ID: 1616243842386743296
• Last Price Increase: 2023-01-20

[melvin-bot]

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[johncschuster]

I was able to reproduce this by following the reproduction steps above.

2023-01-19_08-30-57.mp4

A few things to note, it seems that when you are clicking into the "user details" section of the chat (highlighted in orange in the screenshot below), it seems the highlighted text remains highlighted, but when you click into the "text" section of the chat (highlighted in pink in the screenshot below), the highlight goes away.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01e5387a24edfdca63

[melvin-bot]

Current assignee @johncschuster is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (External)

[melvin-bot]

Current assignee @cead22 is eligible for the External assigner, not assigning anyone new.

[getusha]

Proposal

index 581012838..b13c27a4c 100644
--- a/src/App.js
+++ b/src/App.js
@@ -16,6 +16,7 @@ import SafeArea from './components/SafeArea';
 import * as Environment from './libs/Environment/Environment';
 import {WindowDimensionsProvider} from './components/withWindowDimensions';
 import {KeyboardStateProvider} from './components/withKeyboardState';
+import DomUtils from "./libs/DomUtils";

 // For easier debugging and development, when we are in web we expose Onyx to the window, so you can more easily set data into Onyx
 if (window && Environment.isDevelopment()) {
@@ -31,8 +32,17 @@ LogBox.ignoreLogs([

 const fill = {flex: 1};

+/**
+ * We want to reset highlighted text range when user clicks outside of the text
+ * More details here: https://github.com/Expensify/App/issues/14402
+ * @param {Event} event
+ */
+const handleOnMouseDown = (event)=>{
+    DomUtils.removeSelectionRange(event)
+}
+
 const App = () => (
-    <GestureHandlerRootView style={fill}>
+    <GestureHandlerRootView onMouseDown={handleOnMouseDown} style={fill}>
         <ComposeProviders
             components={[
                 OnyxProvider,

index 4e58ea3a0..08ce80b69 100644
--- a/src/libs/DomUtils/index.js
+++ b/src/libs/DomUtils/index.js
@@ -2,6 +2,12 @@ function blurActiveElement() {
     document.activeElement.blur();
 }

+function removeSelectionRange(e) {
+    if(e.button !== 0) return;
+    document.getSelection().removeAllRanges();
+}
+
 export default {
     blurActiveElement,
+    removeSelectionRange
 };

index 0e796bc40..9fdab5f52 100644
--- a/src/libs/DomUtils/index.native.js
+++ b/src/libs/DomUtils/index.native.js
@@ -1,5 +1,7 @@
 function blurActiveElement() {}
+function removeSelectionRange() {}

 export default {
     blurActiveElement,
+    removeSelectionRange
 };

Result:

Screen.Recording.2023-01-19.at.9.32.00.PM.mov

By utilizing the onMouseDown event, we can implement a function to remove the selected range whenever the user initiates a click action on highlighted text, effectively deselecting it.

there are multiple ways that we can implement my solution in

1. using onMouseDown on GestureHandlerRootView (strongly recommended)
2. using event listener document.addEventListener("mousedown")
3. using inline event document.onmousedown = ()=>{}

We can use all of these with the same output but i strongly recommend using onMouseDown on GestureHandlerRootView.

It is strongly recommended to utilize the onMouseDown event within the GestureHandlerRootView, as it provides improved functionality and efficient output.

[richard-here]

Content.

• Root Cause
• Initial Solutions with Drawbacks
• Proposal (Final Solution)

Root Cause

In the case explained in the issue above, the selected portion of text is still highlighted (i.e. selected) although elements outside the text are clicked. There are two screen recordings:

1. the first case shows clicking outside the message box (e.g. the chatroom)
2. the second case shows clicking inside the message box beside the chat message inside the message box.

In the first case, the exact reason for the text still being highlighted is unknown. However, I suspect it has something to do with this explanation: https://developer.mozilla.org/en-US/docs/Web/API/Selection#behavior_of_selection_api_in_terms_of_editing_host_focus_changes. I'm assuming it is similar to the behavior of how text is still highlighted when we click on images or buttons in regular HTML. What this means is that there might be a need of altering possibly a lot of components in the already created components.

In the second case, the exact reason for the text still being highlighted is exactly because of the same exact behavior of how text is still highlighted when we click on images or buttons in regular HTML. This is because the message box itself is a Pressable component.

Initial Solutions with Drawbacks

Solution 1

Create a global mousedown event with a handler of removing selection using window.getSelection().removeAllRanges(). This doesn't work as well as expected because it causes not able to drag and drop the highlighted text. Furthermore, as it removes selection range on mousedown, typing on the textarea is also not possible because the cursor is immediately removed after mousedown on the textarea.

Solution 2

Let the sent text consume the press event, then make the Pressable message box to also have a handler in onPressOut to window.getSelection().removeAllRanges(). This works well, but the function of removeAllRanges must be used on other components that also make the text deselected, which means having to keep track which components need to have this handler. This is tedious and error-prone.

Proposal

In light of the previous solutions with drawbacks, I am proposing to create a wrapper for the text component. The wrapper will contain a handler for outside clicks deselecting text. This way, the selectable text component is standalone and the functionality of removing text selection is connected to the selectable text itself.

[cead22]

@getusha @richard-here thank you both for the proposals, and @richard-here thanks for the explanation. Correct me if I'm wrong but it sounds like the text should be deselected whenever we click outside of it, and so long as we're not clicking a Pressable component, is that right, or is it more nuanced than that?

In the example below, that seems to work well when I click on someone's avatar, but not so much when I click the X to close the dialog -- which is inside a Pressable component, or the semi-transparent overlay, which is also inside a Pressable component.

That said, it looks like GH isn't very consistent with this either

I think we probably want to define what we want our behavior to be before writing up the solution -- though one valid approach is to "deselect when you click/tap a non-Pressable component outside of the selected element"

I'm going raise this in #expensify-open-source

[getusha]

@cead22 interesting! but you can check that behavior doesn't exist on safari, so it shouldn't concern us?

Screen.Recording.2023-01-20.at.1.32.35.PM.mov

[cead22]

It doesn't look consistent in Safari either, check this out

[getusha]

@cead22 i see, it's pretty inconsistent, FWIW keeping it focused on Pressable press?

[rushatgabhane]

Hello 👋, I'm not a huge fan of any proposals here.

Handling mouse or press events seems like a hack.
I feel that we should let the browser do the unselecting on clicking outside.

We could find what's preventing the text being unselected, and fix that instead.

What do you think?

[rushatgabhane]

@richard-here you might be onto something with the root cause #14402 (comment)

[s77rt]

Proposal

diff --git a/src/styles/styles.js b/src/styles/styles.js
index 71fed02015..cf29954d03 100644
--- a/src/styles/styles.js
+++ b/src/styles/styles.js
@@ -1238,10 +1238,10 @@ const styles = {
         overflow: 'hidden',
 
         // Starting version 6.3.2 @react-navigation/drawer adds "user-select: none;" to its container.
-        // We add user-select-auto to the inner component to prevent incorrect triple-click text selection.
-        // For further explanation see - https://github.com/Expensify/App/pull/12730/files#r1022883823
-        userSelect: 'auto',
-        WebkitUserSelect: 'auto',
+        // We add "user-select: text;" to the inner component to prevent incorrect triple-click text selection.
+        // For further explanation see - https://github.com/Expensify/App/pull/12730/files#r1022883823 and https://github.com/Expensify/App/issues/14402
+        userSelect: 'text',
+        WebkitUserSelect: 'text',
     },
 
     appContentHeader: {

RCA

Browsers deselect the current text only if the mouse down target is an element that can be selected (user-select is not none).

If you look into the source code you will see one of the main wrapper is using user-select: none this style is causing the issue but it's not from our App. It's from react-navigation and I think it's actually from react-gesture-handler which react-navigation uses.

The thing is apparently we are aware of this as I have seen some comments confirming this, and to cover that issue we used user-select: auto on that main wrapper children, thus it's applied in our appContent style rule.

The problem is that using user-select: auto is not really doing it's job as you'd expect based on MDN:

Note: user-select is not an inherited property, though the initial auto value makes it behave like it is inherited most of the time. WebKit/Chromium-based browsers do implement the property as inherited, which violates the behavior described in the spec, and this will bring some issues. Until now, Chromium has chosen to fix the issues to make the final behavior meet the specifications.

I think the best we can do and the easiest fix is to use user-select: text instead of user-select: auto.

Results

Kooha-2023-01-21-00-40-08.mp4

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[richard-here]

Proposal

@getusha @richard-here thank you both for the proposals, and @richard-here thanks for the explanation. Correct me if I'm wrong but it sounds like the text should be deselected whenever we click outside of it, and so long as we're not clicking a Pressable component, is that right, or is it more nuanced than that?

In the example below, that seems to work well when I click on someone's avatar, but not so much when I click the X to close the dialog -- which is inside a Pressable component, or the semi-transparent overlay, which is also inside a Pressable component.

That said, it looks like GH isn't very consistent with this either

I think we probably want to define what we want our behavior to be before writing up the solution -- though one valid approach is to "deselect when you click/tap a non-Pressable component outside of the selected element"

I'm going raise this in #expensify-open-source

Additional Explanation on Root Case

I have been doing further reading because it is interesting and I want to shed some light regarding the root cause. Here is what I have found.

• This behavior is related to the Selection API. You can read what MDN specifies about it here and what W3 specifies about it here.

• In the MDN API explanation, a Selection is defined as the following.

  A Selection object represents the range of text selected by the user or the current position of the caret.

  Besides that, it is important to note that W3 defines that there must be only one unique Selection object with one range or caret in a document. This is mentioned in MDN's Selection API as well, quoted below.

  As the Selection API specification notes, the Selection API was initially created by Netscape and allowed multiple ranges (for instance, to allow the user to select a column from a table). However, browsers other than Gecko did not implement multiple ranges, and the specification also requires the selection to always have a single range.

  As can be seen, it seems that initially, there can be multiple ranges in a Selection, but as of now, there can only be a single range or caret for the Selection object. A more detailed explanation is available in W3 here. Basically, it defines what a Selection is supposed to be. To paraphrase it, it's basically a singleton unique object that can be empty (when nothing is selected or no caret) or that it can be a single range.

  Now all this explains the inconsistency in GH that you mentioned. It is actually not inconsistent. It is working as expected. When you are clicking the hyperlink or any clickable components, the selection does not disappear or the range is still the same. However, when you clicked a component on GH that immediately focuses on a text input field, the Selection's range is now actually the caret in the input field, and as there can only be one range or caret for the Selection object, the previous range is naturally no longer selected. Instead, the Selection object (you can get this by using window.getSelection()) will now be of type Caret, which means that there is a Selection object, but it is collapsed, i.e. no range is selected, but caret is placed on some text (read more here). This explains the Github behavior you sent in the video. Let's move on to the next behavior.

• After clicking on someone's avatar, then closing the dialog by clicking on the X button, the Selection range is no longer the text selected before closing the dialog. However, when opening the dialog (i.e. clicking on someone's avatar), the selection range stays the same. This is also expected and consistent with the behavior I mentioned. This is because the Selection becomes of type Caret and the anchorNode is now the text input field in the app to send messages. I reckon this has something to do with the app focusing on the text field immediately (for UX purposes probably) when the page opens. Let me send a video that shows this.

  screen-capture.1.mov

• After investigating more from W3, I have finally been able to find an explanation in regards to why Selection is not collapsed into type Caret when clicking on clickable components (in regular HTML or in the app). It is because those clickable components are not selectable text nodes, and according to W3 specs, once a Selection is not empty, the user agent must not set the Selection as empty anymore. That is why the selected range stays the same when clicking on unselectable components.

  The user agent must not make a selection empty if it was not already empty in response to any user actions (e.g. clicking on a non-editable region).

This explains the root cause of the behavior we are seeing.

Better Proposal

Behavior Changes

As mentioned here: #14402 (comment), altering the window's behavior on click is hack-ish. I think the best way to make it not hack-ish (i.e. altering the default browser's behavior when clicking), we should instead make all components selectable in the whole app, including Pressable components. I still think it's good UX. This also means that Text in the React Native component without the selectable={true} property will still be able to be selected even without the selectable={true} property.

Solution

To make a component selectable, we can use the CSS property user-select and set its value to text. Unfortunately, there is a parent element with user-select: none and this is inherited by the children using user-select: auto. This inheritance mechanism of children components using user-select: auto is explained in MDN as well.

The used value of auto is determined as follows:
On the ::before and ::after pseudo elements, the used value is none
If the element is an editable element, the used value is contain
Otherwise, if the used value of user-select on the parent of this element is all, the used value is all
Otherwise, if the used value of user-select on the parent of this element is none, the used value is none
Otherwise, the used value is text

Here is a screenshot of what I meant by the parent's user-select being none.

If we change the parent's user-select to text, all components will have a used value of text, solving this issue. Note that I mentioned that all text will now behave like selectable={true}. However, if we want to make some text not selectable, we can always set the Text's selectable to false. This works because React actually will set the component's user-select to none, making it not selectable.

With this, clicking on anywhere should collapse the existing range and make the Selection type of Caret to another text node as long as React Native does not have some CSS styling with higher specificity that makes some components' user-select's value to none, thus making the component not selectable. Here's the solution in action.

screen-capture.2.mov

[getusha]

@rushatgabhane the root cause is very complicated and wired and i tried to understand it fully and i present it as follows 😅

There was an issue #7462 Long press on image doesn't open the context menu reported by you.

• in order to fix that this PR was raised Fix: Long press on image doesn't open the context menu #8018, in that PR LongPressGestureHandler was added for the web too even though it's not needed since we have contextmenu event listener to open the context menu. (introduced this issue for the first time)
• • then this PR introduced a great regression here [HOLD for payment 2023-01-23] [$4000] mWeb - Copy to email/URL is not working correctly - reported by @mateusbra #8311 ,
• • • and this PR also introduced [HOLD for payment 2023-02-10] [$2000] Showing pointer cursor on code-block chat message instead of I-beam cursor #14301 (comment) [HOLD for payment 2023-02-10] [$2000] Showing pointer cursor on code-block chat message instead of I-beam cursor #14301 (comment)
• I think we should Hold that until this gets fixed (seems related to me)

I just wanted to comment this Just help prevent future issues related to this, because this looks related

Proposal

Interaction/index.js
index 7b971e7268..3881125e95 100644
--- a/src/components/PressableWithSecondaryInteraction/index.js
+++ b/src/components/PressableWithSecondaryInteraction/index.js
@@ -43,7 +43,7 @@ class PressableWithSecondaryInteraction extends Component {
         // On Web, Text does not support LongPress events thus manage inline mode with styling instead of using Text.
         return (
             <Pressable
-                style={this.props.inline && styles.dInline}
+                style={[this.props.inline && styles.dInline, styles.userSelectText]}
                 onPressIn={this.props.onPressIn}
                 onLongPress={(e) => {
                     if (DeviceCapabilities.hasHoverSupport()) {

Result

Screen.Recording.2023-01-21.at.1.56.28.PM.mov

[rushatgabhane]

discussion on slack

[cead22]

Still discussing here https://expensify.slack.com/archives/C01GTK53T8Q/p1674580475813759?thread_ts=1674249705.438009&cid=C01GTK53T8Q, but I'm proposing we close this issue. I'm going to leave it open for now to let other chime in

[cead22]

Closing per slack discussion