[$1000] Web - Profile - The text on the modal Drag, zoom, and rotate your image to your preferred is highlighted when rotate the picture the picture

[kbecciv]

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

---

HOLD on #16660

Issue found when executing PR #14115

Action Performed:

1. Login to any account
2. Open the settings page
3. Go to the profile page by clicking on the profile picture or the account email
4. Click on the profile picture and click on the upload photo button
5. Select an image from your device and the upload photo modal will open
6. Select the text on the modal Drag, zoom, and rotate your image to your preferred specifications by double click the text
7. Start Rotate the picture

Expected Result:

The text on the modal Drag, zoom, and rotate your image to your preferred is not highlighted when rotate the picture

Actual Result:

The text on the modal Drag, zoom, and rotate your image to your preferred is highlighted when rotate the picture

Workaround:

Unknown

Platforms:

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

• Web / Chrome

Version Number: 1.2.50.14

Reproducible in staging?: Yes

Reproducible in production?: Yes

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: Any additional supporting documentation

Recording.1971.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012de3dfcd27f5f324
• Upwork Job ID: 1613145679138578432
• Last Price Increase: 2023-03-27

[twisterdotcom]

My first few of these, so taking the template from StackOverflowTeams: https://stackoverflowteams.com/c/expensify/questions/14418

• The "bug" is actually a bug.
• The bug is not a duplicate report of an existing GH
• The bug is reproducible, following the reproduction steps.
• The GH template is filled out as fully as possible
• The GH was created by an Expensify employee or a QA tester.
• The bug is an /App issue

---

I don't really understand this issue. If I highlight some text, and then click somewhere else in the app, the text should be unhighlighted shouldn't it? The focus has moved, in this case, to the rotate option.

[twisterdotcom]

Ah wait, I see now. It just randomly highlights it whenever you click rotate:

2023-01-11_12-06-22.mp4

https://expensify.slack.com/archives/C01GTK53T8Q/p1673438611054609 - making this External

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~012de3dfcd27f5f324

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @danieldoglas (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[tienifr]

Proposal

Problem

The clicking action was triggered for the wrapper div component, causing the text in the div to be highlighted

Solution

Disable user select in the text and the menu

diff --git a/src/components/AvatarCropModal/AvatarCropModal.js b/src/components/AvatarCropModal/AvatarCropModal.js
index 1d9922ead4..a024e3b079 100644
--- a/src/components/AvatarCropModal/AvatarCropModal.js
+++ b/src/components/AvatarCropModal/AvatarCropModal.js
@@ -316,7 +316,7 @@ const AvatarCropModal = (props) => {
                 title={props.translate('avatarCropModal.title')}
                 onCloseButtonPress={props.onClose}
             />
-            <Text style={[styles.mh5]}>{props.translate('avatarCropModal.description')}</Text>
+            <Text style={[styles.mh5, styles.userSelectNone]}>{props.translate('avatarCropModal.description')}</Text>
             <GestureHandlerRootView onLayout={initializeImageContainer} style={[styles.alignSelfStretch, styles.m5, styles.flex1, styles.alignItemsCenter]}>
 
                 {/* To avoid layout shift we should hide this component until the image container & image is initialized */}
diff --git a/src/components/HeaderWithCloseButton.js b/src/components/HeaderWithCloseButton.js
index ee1970bca3..9df1cbb204 100755
--- a/src/components/HeaderWithCloseButton.js
+++ b/src/components/HeaderWithCloseButton.js
@@ -136,6 +136,7 @@ class HeaderWithCloseButton extends Component {
                     styles.flexGrow1,
                     styles.justifyContentBetween,
                     styles.overflowHidden,
+                    styles.userSelectNone
                 ]}
                 >
                     {this.props.shouldShowBackButton && (

recordfix_100123_selectext.mp4

[jatinsonijs]

@tienifr we cannot use userSelectNone as it will create following regression:
#14121

This issue can be fixed by same solution as posted for another issue:
Issue: #13688
Solution: #13688 (comment)

[mollfpr]

I agree the text should be selectable.

@jatinsonijs I feel your proposal to complicated. This issue can be fixed with a slight change.

[jatinsonijs]

@mollfpr understood your concern,
we can fix this by just pass userSelectNone to button

diff --git a/src/components/AvatarCropModal/AvatarCropModal.js b/src/components/AvatarCropModal/AvatarCropModal.js
index 1d9922ead4..a6ad8309c9 100644
--- a/src/components/AvatarCropModal/AvatarCropModal.js
+++ b/src/components/AvatarCropModal/AvatarCropModal.js
@@ -348,7 +348,7 @@ const AvatarCropModal = (props) => {
                                     icon={Expensicons.Rotate}
                                     iconFill={themeColors.inverse}
                                     iconStyles={[styles.mr0]}
-                                    style={[styles.imageCropRotateButton]}
+                                    style={[styles.imageCropRotateButton, styles.userSelectNone]}
                                     onPress={rotateImage}
                                 />
                             </View>

[mollfpr]

@jatinsonijs Could you explain why the issue happen and why your solution is to fix the issue?

[jatinsonijs]

@mollfpr it is default behaviour of browsers whenever we do double click text get selected. With userSelectNone we are preventing this behaviour on particular div. as in our app we are using custom button which is actually a div internally. On div its default behaviour of browsers. as per browser implementation it is prevented internally at browser level like on image db-press, native web buttons, anchor tag and some other elements.

[Ollyws]

Proposal

I believe this is because of Icon, as it is using an SVG image it exhibits similar behaviour to text, allowing the user to select on click.

We can fix this by adding styles.userSelectNone to the iconStyles.

It might be worth fixing this by adding styles.userSelectNone to Button so we don't see this kind of behaviour again elsewhere.

[Puneet-here]

Duplicate of #10515. It was closed because it's not a valuable fix
cc: @iwiznia

[mollfpr]

Thanks @jatinsonijs!

---

I believe this is because of Icon, as it is using an SVG image it exhibits similar behaviour to text, allowing the user to select on click.

It is only reproducible on the button using an icon.

It might be worth fixing this by adding styles.userSelectNone to Button so we don't see this kind of behaviour again elsewhere.

I agree with this, can you confirm that adding styles.userSelectNone to the button will not make a regression? Even though the text on a button is not selectable. Also, could you send a snippet code for this solution? Thanks!

---

@twisterdotcom Do you think this should be closed to like @Puneet-here attached?

[s77rt]

Proposal

diff --git a/src/components/Button.js b/src/components/Button.js
index 6e6fbade77..e7689a7066 100644
--- a/src/components/Button.js
+++ b/src/components/Button.js
@@ -181,7 +181,6 @@ class Button extends Component {
 
         const textComponent = (
             <Text
-                selectable={false}
                 style={[
                     this.props.isLoading && styles.opacity0,
                     styles.pointerEventsNone,
@@ -255,6 +254,7 @@ class Button extends Component {
                 disabled={this.props.isLoading || this.props.isDisabled}
                 style={[
                     this.props.isDisabled ? {...styles.cursorDisabled, ...styles.noSelect} : {},
+                    styles.userSelectNone,
                     styles.buttonContainer,
                     this.props.shouldRemoveRightBorderRadius ? styles.noRightBorderRadius : undefined,
                     this.props.shouldRemoveLeftBorderRadius ? styles.noLeftBorderRadius : undefined,

Instead of adding the styles.userSelectNone to only the current effected button (@jatinsonijs) or to only the icon (@Ollyws). I think it may be better to apply the style in the <Pressable /> directly and avoid multiple styles declaration.

[mollfpr]

@s77rt Removing selectable={false} will make the text on the web selectable because the default value is true on RNW.

[jatinsonijs]

@mollfpr we can do it at low level like in Button as suggested by @Ollyws,but its only happen on double press I haven't found any other area where we are going to do double tap. Here on rotate button we are un-intensionally doing double tap (click) to rotate image.

[mollfpr]

@jatinsonijs Yes, but since it's a reusable component we can prevent the same issue occur in the future. Toggled button with an icon will not only be for the avatar modal but could be anywhere.

[twisterdotcom]

Still HELD

[twisterdotcom]

Held, but not for long!

[twisterdotcom]

@mollfpr can we unhold this, or do we also need to wait for #31381?

[twisterdotcom]

Still held on #31381

[roryabraham]

Looks like this can come off HOLD

[parasharrajat]

Here is a solution #14139 (comment) which can be implemented now.

[twisterdotcom]

@parasharrajat you wanna be assigned? @mollfpr if you agree and Rajat is down, can we just assign him?

[parasharrajat]

IMO, It was already added in a lot of places. Can we retest this issue?

[mollfpr]

I'll retest the issue 👍

[mollfpr]

I think this issue was already fixed!

Screen.Recording.2024-01-26.at.00.15.29.mp4

Although, the highlight remains, but I think it's expected. I do the same with a new project; it keeps the highlight on the text after pressing a button.

https://codesandbox.io/p/sandbox/stupefied-matan-2y8w2z

[parasharrajat]

That seems like a different thing. We might be blocking the default click behavior causing the static selection.

This does not seem like a major thing. We should be good to close this issue. @twisterdotcom