[PAID][$250] Taxes - When creating a Tax, the keyboard does not automatically appear in the "Name" menu

[lanitochka17]

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

---

Version Number: 1.4.71-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: https://expensify.testrail.io/index.php?/tests/view/4545979
Email or phone of affected tester (no customers): sustinov@applausemail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Open New Expensify app
2. Create a WS
3. Navigate to the "More features" menu
4. Enable "Taxes"
5. Navigate to the "Taxes" menu
6. Tap on the "Add rate" button
7. Tap on the Name field

Expected Result:

When creating a Tax, the keyboard should automatically appear in the "Name" menu

Actual Result:

When creating a Tax, the keyboard does not automatically appear in the "Name" menu

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

Add any screenshot/video evidence

Bug6474103_1715105608383.screencapture-1715105071899.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016fb7ed1a2ab55442
• Upwork Job ID: 1787997169242755072
• Last Price Increase: 2024-05-08
• Automatic offers:
• jayeshmangwani | Reviewer | 0
• Krishna2323 | Contributor | 0

Issue Owner

Current Issue Owner: @strepanier03

[melvin-bot]

Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@strepanier03 FYI 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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[ShridharGoel]

Proposal

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

Taxes - When creating a Tax, the keyboard does not automatically appear in the "Name" menu

What is the root cause of that problem?

The name field uses TextPicker which internally uses TextInput without autofocus.

App/src/pages/workspace/taxes/WorkspaceCreateTaxPage.tsx

Lines 87 to 98 in badec94

	<InputWrapper
	InputComponent={TextPicker}
	inputID={INPUT_IDS.NAME}
	label={translate('common.name')}
	description={translate('common.name')}
	rightLabel={translate('common.required')}
	accessibilityLabel={translate('workspace.editor.nameInputLabel')}
	maxLength={CONST.TAX_RATES.NAME_MAX_LENGTH}
	multiline={false}
	role={CONST.ROLE.PRESENTATION}
	autoFocus
	/>

autoFocus is not being passed as true:

App/src/components/TextPicker/TextSelectorModal.tsx

Lines 45 to 50 in badec94

	<TextInput
	// eslint-disable-next-line react/jsx-props-no-spreading
	{...rest}
	value={currentValue}
	onInputChange={setValue}
	/>

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

We can add a new prop to TextPicker and TextSelectorModal called autofocus which will be passed TextInput which is there inside TextSelectorModal.

App/src/components/TextPicker/index.tsx

Lines 50 to 58 in badec94

	<TextSelectorModal
	value={value}
	isVisible={isPickerVisible}
	description={description}
	onClose={hidePickerModal}
	onValueSelected={updateInput}
	// eslint-disable-next-line react/jsx-props-no-spreading
	{...rest}
	/>

<TextInput
    // eslint-disable-next-line react/jsx-props-no-spreading
    {...rest}
    value={currentValue}
    onInputChange={setValue}
    autoFocus={autoFocus}
/>

Now, we can control whether to autofocus or not using this prop.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016fb7ed1a2ab55442

[melvin-bot]

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

[strepanier03]

Easy to repro on Android Native, adding external.

[nkdengineer]

Proposal

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

When creating a Tax, the keyboard does not automatically appear in the "Name" menu

What is the root cause of that problem?

We pass autoFocus to the input of TextPicker so the input is focused but the keyboard doesn't appear

App/src/pages/workspace/taxes/WorkspaceCreateTaxPage.tsx

Line 97 in 55c56bf

autoFocus

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

• We should remove all autoFocus prop that we passed to TextPicker to prevent this bug in other places

App/src/pages/workspace/taxes/WorkspaceCreateTaxPage.tsx

Line 97 in 55c56bf

autoFocus

• We should use the ref to focus the TextInput whenever the text select modal is opened.

1. Create a ref

const inputRef = useRef<BaseTextInputRef | null>(null);

2. Create a useEffect to focus the TextInput whenever the TextSelectorModal is opened

const prevIsVisible = usePrevious(isVisible);

const focus = () => {
    focusTimeoutRef.current = setTimeout(() => {
        inputRef.current?.focus();
        return () => {
            if (!focusTimeoutRef.current) {
                return;
            }
            clearTimeout(focusTimeoutRef.current);
        };
    }, CONST.ANIMATED_TRANSITION);
}

useEffect(() => {
    if (prevIsVisible || !isVisible) {
        return;
    }

    focus();

}, [prevIsVisible, isVisible])

What alternative solutions did you explore? (Optional)

[Krishna2323]

Proposal

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

Taxes - When creating a Tax, the keyboard does not automatically appear in the "Name" menu

What is the root cause of that problem?

We are using autoFocus prop which doesn't work properly on all platform.

App/src/pages/workspace/taxes/WorkspaceCreateTaxPage.tsx

Line 97 in e0b6a06

autoFocus

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

We need to use useFocusEffect to focus on the input which is also suggested in the docs. In this case we can't useuseAutoFocus hook because we need to focus whenever the isVisible prop changes so we need to use useFocusEffect.

    // In TextSelectorModal.tsx
    const inputRef = useRef<TextInputT | null>(null);
    const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null);

    useFocusEffect(
        useCallback(() => {
            focusTimeoutRef.current = setTimeout(() => {
                if (inputRef.current && isVisible) {
                    inputRef.current.focus();
                }
                return () => {
                    if (!focusTimeoutRef.current || !isVisible) {
                        return;
                    }
                    clearTimeout(focusTimeoutRef.current);
                };
            }, CONST.ANIMATED_TRANSITION);
        }, [isVisible]),
    );

What alternative solutions did you explore? (Optional)

[jayeshmangwani]

Thanks for the proposals @nkdengineer @Krishna2323; both approach will work in this case, but we can use the useFocusEffect approach. For consistency here, we are already using the same way on many pages.

@Krishna2323 's Proposal looks good to me

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[nkdengineer]

@jayeshmangwani Since the idea is the same, I think using useEffect or useFocusEffect can be discussed in the PR phrase.

[jayeshmangwani]

The idea may be similar @nkdengineer , but I chose the other proposal because we discussion around the autofocus prop and decided to use the useFocusEffect hook.And You mentioned in the proposal that we should use the useEffect, but I'm sorry if you feel otherwise. I will let the internal engineer decide, I hope thats is ok 🙏

[nkdengineer]

No problem.
I think the useEffect is just an example detail with my idea and it can be discussed more in the PR.

Feel free with the decision from @MonilBhavsar

[MonilBhavsar]

Since the other proposal completely mentioned the correct hook. It is fair to select it. Thanks for the proposal though!

[melvin-bot]

📣 @jayeshmangwani 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[Krishna2323]

@jayeshmangwani, PR ready for review.

[melvin-bot]

This issue has not been updated in over 15 days. @strepanier03, @MonilBhavsar, @jayeshmangwani, @Krishna2323 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[MonilBhavsar]

The PR was deployed to production 2 weeks ago and there's no regression, so we need to pay @Krishna2323 and @jayeshmangwani(C+) cc @strepanier03

---

Not sure why automation didn't work

[MonilBhavsar]

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:

• [@jayeshmangwani] The PR that introduced the bug has been identified. Link to the PR:
• [@jayeshmangwani] 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:
• [@jayeshmangwani] 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:
• [@jayeshmangwani] Determine if we should create a regression test for this bug.
• [@jayeshmangwani] 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.
• [@strepanier03] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jayeshmangwani]

• [@jayeshmangwani] The PR that introduced the bug has been identified. Link to the PR: [Simplified Collect][Taxes] Creating new taxes #37967
• [@jayeshmangwani] 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: https://github.com/Expensify/App/pull/37967/files#r1629432041
• [@jayeshmangwani] 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: N/A - we already have a section in the dc for the similar issues
• [@jayeshmangwani] Determine if we should create a regression test for this bug.
• [@jayeshmangwani] 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.

Regression Test Proposal

1. Open App -> Go to Settings -> Workspaces -> Create a WS
2. Navigate to the "More features" -> Enable "Taxes"
3. Navigate to the "Taxes" menu -> Add rate
4. Tap on the Name field
5. Verify that input field is focused and keyboard appears automatically

Do we agree 👍 or 👎

[jayeshmangwani]

@strepanier03 Is that possible to get payment through ND for this issue as now I am getting paid through it and cancel the Upwork offer? If not then no issues we can go with the Upwork offer.

[jayeshmangwani]

Not Overdue! Issue is Awaiting payment
@strepanier03 Please check this comment

[strepanier03]

Thanks all, I have been out of ht office and just got back in today.

@jayeshmangwani - Sure thing! I'll cancel the contract in Upwork and make a payment summary post so you can do the manual request.

[strepanier03]

@Krishna2323 - THe job in Upwork expired so I had to make a new one. I hired you for it and you should see an offer to accept. I'll check tomorrow to pay it.

[strepanier03]

Payment Summary

• $250 - @jayeshmangwani via Manual Requests
• $250 - @Krishna2323 via Upwork

@JmillsExpensify - Payment request incoming.

[Krishna2323]

@strepanier03, accepted.

[jayeshmangwani]

Requested on ND

[JmillsExpensify]

$250 approved for @jayeshmangwani

[strepanier03]

I've paid @Krishna2323 and closed the contract as well.

Thank you, everyone. Have a great week!