[HOLD for payment 2024-04-25] [$500] [LOW] Request money - When amount is highlighted, clicking on the page does not deselect the text

[izarutskaya]

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

---

Found when validating PR : #36463

Version Number: v1.4.43-0
Reproducible in staging?: Y
Reproducible in production?: N
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation:

Action Performed:

Pre-requisite: the user should be logged in.

1. Initiate a manual money request.
2. Enter the amount.
3. Do a text selection.
4. Click on any part of the request page.
5. Verify the text is not deselected.

Expected Result:

The text should be deselected after clicking on the page.

Actual Result:

The text is not deselected, remains highlighted.

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6385343_1708407498195.bandicam_2024-02-19_23-02-52-094.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01626cde7ff61398d1
• Upwork Job ID: 1760234439347433472
• Last Price Increase: 2024-03-13

Issue Owner

Current Issue Owner: @laurenreidexpensify

[melvin-bot]

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

[izarutskaya]

I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

We think that this bug might be related to #vip-bills
CC @davidcardoza

[Krishna2323]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Request money - When amount is highlighted, clicking on the page does not deselect the text

What is the root cause of that problem?

We call event.preventDefault(); in onMouseDown callback function, and due to this the text doesn't get deselected.

App/src/pages/iou/steps/MoneyRequestAmountForm.js

Lines 107 to 112 in c261148

	const onMouseDown = (event, ids) => {
	const relatedTargetId = lodashGet(event, 'nativeEvent.target.id');
	if (!_.contains(ids, relatedTargetId)) {
	return;
	}
	event.preventDefault();

What changes do you think we should make in order to solve the problem?

Initial Proposal

We should use `removeAllRanges` after calling `event.preventDefault();` in `onMouseDown`.

This can be done using code below:

        if (window.getSelection) {
            window.getSelection().removeAllRanges();
        } else if (document.selection) {
            document.selection.empty();
        }

PS: We shouldn't use setSelection when we use onMouseDown for following reasons:

1. The default behaviour of the app/web is to blur the input when pressing outside and updating selection will focus instead of blurring the input.
2. If half of the input value is selected then tapping outside will set selection will set the cursor to last text/number
3. Unnecessary re-renders
4. Inconsistency with the time input, in time input page we do blur the input and remove selection when pressing outside.

Result

text_selection_amount_form.mp4

Alternative

Remove event.preventDefault(); in onMouseDown.

Optional

The text selection is also not removed when we move to currency selection page and go back. For that we can use isFocused state and reset the selection to:

    useEffect(() => {
        if (!isFocused) {
            return;
        }

        setSelection({
            start: currentAmount.length,
            end: currentAmount.length,
        });
    }, [isFocused, setSelection]);

Bug Video:

bug_selection_2.mp4

Alternative 2

We can set selection start and end to selection.end after we prevent default.

        setSelection({
            start: selection.end,
            end: selection.end,
        });

If we also want to fix the bug I mentioned in the optional section, we can do that like:

    useEffect(() => {
        if (!isFocused || wasFocused) {
            return;
        }

        setSelection({
            start: selection.end,
            end: selection.end,
        });
    }, [selection.end, isFocused, selection, wasFocused]);

If we want the same behaviour on native devices also we need to wrap View with PressableWithoutFeedback execute the same functionality like onMouseDown when onPress in triggered.

Demo Code

            <PressableWithoutFeedback
                onMouseDown={(event) => {
                    onMouseDown(event, [AMOUNT_VIEW_ID]);
                }}
                style={[styles.moneyRequestAmountContainer, styles.flex1, styles.w100, styles.cursorDefault]}
                accessibilityLabel="asda"
                role={CONST.ROLE.NONE}
                onPress={(event) => {
                    event?.stopPropagation();
                    event?.preventDefault();
                    if (selection.end === selection.start || !textInput.current?.isFocused()) {
                        return;
                    }
                    setSelection({
                        start: selection.end,
                        end: selection.end,
                    });

                    if (!textInput.current) {
                        return;
                    }
                    if (!textInput.current.isFocused()) {
                        textInput.current.focus();
                    }
                }}
            >
                <View
                    id={AMOUNT_VIEW_ID}
                    // style={[styles.w100, styles.justifyContentEnd, styles.pageWrapper, styles.pt0]}
                    style={[styles.moneyRequestAmountContainer, styles.flex1, styles.flexRow, styles.w100, styles.alignItemsCenter, styles.justifyContentCenter]}
                >
                    <TextInputWithCurrencySymbol
                        formattedAmount={formattedAmount}
                        onChangeAmount={setNewAmount}
                        onCurrencyButtonPress={onCurrencyButtonPress}
                        placeholder={numberFormat(0)}
                        ref={(ref) => {
                            if (typeof forwardedRef === 'function') {
                                forwardedRef(ref);
                            } else if (forwardedRef?.current) {
                                // eslint-disable-next-line no-param-reassign
                                forwardedRef.current = ref;
                            }
                            textInput.current = ref;
                        }}
                        selectedCurrencyCode={currency}
                        selection={selection}
                        onSelectionChange={(e: NativeSyntheticEvent<TextInputSelectionChangeEventData>) => {
                            if (!shouldUpdateSelection) {
                                return;
                            }
                            const maxSelection = formattedAmount.length;
                            const start = Math.min(e.nativeEvent.selection.start, maxSelection);
                            const end = Math.min(e.nativeEvent.selection.end, maxSelection);
                            setSelection({start, end});
                        }}
                        onKeyPress={textInputKeyPress}
                        isCurrencyPressable
                    />
                    {!!formError && (
                        <FormHelpMessage
                            style={[styles.pAbsolute, styles.b0, styles.mb0, styles.ph5, styles.w100]}
                            isError
                            message={formError}
                        />
                    )}
                </View>
            </PressableWithoutFeedback>

Result

fix_amount_selection.mp4

[davidcardoza]

Adding to the vip bill pay project board as a LOW issue

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01626cde7ff61398d1

[melvin-bot]

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

[dukenv0307]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The text is not deselected, and remains highlighted.

What is the root cause of that problem?

The amount input takes up all remaining height. when we click on this space onMouseDown is triggered and we prevent the default behavior here if the relatedTargetId is in the space of the input. So the amount is still highlighted.

App/src/pages/iou/steps/MoneyRequestAmountForm.js

Lines 108 to 114 in c261148

	const relatedTargetId = lodashGet(event, 'nativeEvent.target.id');
	if (!_.contains(ids, relatedTargetId)) {
	return;
	}
	event.preventDefault();
	if (!textInput.current) {
	return;

What changes do you think we should make in order to solve the problem?

We can update the selection state when we click on the space of the input wrapper afte we call prevent default

setSelection({
    start: currentAmount.length,
    end: currentAmount.length,
});

App/src/pages/iou/steps/MoneyRequestAmountForm.js

Lines 108 to 114 in c261148

	const relatedTargetId = lodashGet(event, 'nativeEvent.target.id');
	if (!_.contains(ids, relatedTargetId)) {
	return;
	}
	event.preventDefault();
	if (!textInput.current) {
	return;

What alternative solutions did you explore? (Optional)

NA

[Krishna2323]

Proposal Updated

• Added explanation why we shouldn't use setSelection in onMouseDown event.
• Added fix for another bug in the same page.
• Screenshots added

[thesahindia]

Planning to go OOO! Please reassign @laurenreidexpensify.

[situchan]

I can take over.

I am not able to select text on main so stuck on Step 3. Looks like another regression

Screen.Recording.2024-02-21.at.10.41.51.PM.mov

[Krishna2323]

Regressions from #35774

cc: @ishpaul777 @neonbhai

[ishpaul777]

@neonbhai tag me when you have fix ready for review, ideally before this hit staging

[laurenreidexpensify]

@situchan assigned 👍

[neonbhai]

PR fixing selection issue has been merged 🚀

[situchan]

ok so we can continue this issue

[situchan]

Version Number: v1.4.43-0
Reproducible in staging?: Y
Reproducible in production?: N

This was deploy blocker.
@Krishna2323 @dukenv0307 are you able to find offending PR?

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.62-17 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: When amount is highlighted, clicking on the page does not deselect the text #39311

If no regressions arise, payment will be issued on 2024-04-25. 🎊

For reference, here are some details about the assignees on this issue:

• @Krishna2323 requires payment (Needs manual offer from BZ)
• @situchan requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@hayata-suenaga] The PR that introduced the bug has been identified. Link to the PR:
• [@hayata-suenaga] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@hayata-suenaga] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@Krishna2323 / @situchan] Determine if we should create a regression test for this bug.
• [@Krishna2323 / @situchan] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@laurenreidexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[hayata-suenaga]

[@hayata-suenaga] The PR that introduced the bug has been identified. Link to the PR:
[@hayata-suenaga] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

#16394 (comment)

[@hayata-suenaga] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

No need for this

[laurenreidexpensify]

@Krishna2323 pls accept offer in Upwork thanks!

@situchan still offline this month

[Krishna2323]

@laurenreidexpensify, offer accepted :)

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @Krishna2323 paid $(AMOUNT) via Upwork (LINK)
• ROLE: @situchan paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@laurenreidexpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1760234439347433472/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[laurenreidexpensify]

Payment Summary:

Contributor: $500 @Krishna2323 paid in upwork
C+: $500 @situchan to be paid in Upwork

steps to complete when @situchan is back from OOO in May:

• payment
• regression steps

[situchan]

@laurenreidexpensify Sorry for late. Can you please send new offer?

[laurenreidexpensify]

@situchan offer sent in upwork 👍

[laurenreidexpensify]

Payment Summary:

Contributor: $500 @Krishna2323 paid in upwork
C+: $500 @situchan paid in upwork

[laurenreidexpensify]

@situchan can you confirm regression testing steps so we can close out thanks

[situchan]

Regression Test Proposal

1. Initiate a manual money request
2. Enter the amount
3. Do a text selection
4. Click on any part of the request page
5. Verify the text is deselected