Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.89k forks source link

[$250] Description field doesn't autofocus correctly in submit expense flow #52414

Open m-natarajan opened 1 day ago

m-natarajan commented 1 day ago

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: 9.0.60-1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: 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 Expensify/Expensify Issue URL: Issue reported by: @shawnborton Slack conversation (hyperlinked to channel name): ts_external_expensify_bugs

Action Performed:

  1. Create an expense
  2. Choose a recipient
  3. Tap description field

    Expected Result:

    The field auto focused and keyboard open

    Actual Result:

    The field doesn't quite auto-focus. The border turns green as if it was auto-focused, but then in the end you have to tap again to get the cursor to focus into the field.

    Workaround:

    Unknown

    Platforms:

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

    • [ ] Android: Standalone
    • [ ] Android: HybridApp
    • [ ] Android: mWeb Chrome
    • [ ] iOS: Standalone
    • [ ] iOS: HybridApp
    • [x] iOS: mWeb Safari
    • [ ] MacOS: Chrome / Safari
    • [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/c9dde090-d9ba-4785-963d-cb68fd79071f

https://github.com/user-attachments/assets/c5ea7e3f-2efe-4cf6-90c0-1b328eae51d0

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021856445363759517675
  • Upwork Job ID: 1856445363759517675
  • Last Price Increase: 2024-11-12
Issue OwnerCurrent Issue Owner: @getusha
melvin-bot[bot] commented 1 day ago

Triggered auto assignment to @sakluger (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.

melvin-bot[bot] commented 1 day ago

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

melvin-bot[bot] commented 1 day ago

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

Anaslancer commented 1 day ago

Proposal

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

Description field doesn't autofocus correctly in submit expense flow

What is the root cause of that problem?

We are trying to set a focus in this code. And it is an animated text input box. So we should wait and set focus until the animation is ended.

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

We can use the useEffect instead of useFocusEffect in this below code. https://github.com/Expensify/App/blob/3023d6016ef59d3fc8f0190f48597c41e95d3a55/src/pages/iou/request/step/IOURequestStepDescription.tsx#L79

What alternative solutions did you explore? (Optional)

We can increase more milliseconds in setTimeout. https://github.com/Expensify/App/blob/3023d6016ef59d3fc8f0190f48597c41e95d3a55/src/pages/iou/request/step/IOURequestStepDescription.tsx#L91

Contributor details

Your Expensify account email: anasup1995@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01aff093c9a804b145

melvin-bot[bot] commented 1 day ago

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

ikevin127 commented 1 day ago

Proposal

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

The field doesn't quite auto-focus. The border turns green as if it was auto-focused, but then in the end you have to tap again to get the cursor to focus into the field.

What is the root cause of that problem?

The root cause is this IOURequestStepDescription.tsx block of logic which does not work as intended, instead only turning the input's border green without actualy focusing the input and showing the cursor / keyboard.

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

  1. Remove the mentioned block of logic completely as it doesn't work as intended and on top of that adds extra timeout and logic that only slows down the UI.
  2. Pass the autoFocus (boolean) prop to the TextInput (InputWrapperWithRef) here.

More context on the TextInput autoFocus logic here.

[!note] I tested the first proposal's suggested solutions and none of them fixed the issue for me.

Result videos (before / after)

iOS: mWeb Safari | Before | After | | ------------- | ------------- | |
huult commented 1 day ago

Edited by proposal-police: This proposal was edited at 2024-11-13 03:01:54 UTC.

Proposal

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

Description field doesn't autofocus correctly in submit expense flow

What is the root cause of that problem?

We set a 300ms timeout in this block to delay the inputRef focus until after animations or screen transitions. However, 300ms was not enough for inputRef to focus properly, so this issue occurred. https://github.com/Expensify/App/blob/7256ad6eef046be3345e0b260252f6ec47fc4203/src/pages/iou/request/step/IOURequestStepDescription.tsx#L79-L93

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

To resolve this issue, we should use InteractionManager.runAfterInteractions instead of setTimeout, as this React Native utility runs code only after animations and interactions have finished, ensuring that the component is ready. Here’s an example:

https://github.com/Expensify/App/blob/main/src/pages/iou/request/step/IOURequestStepDescription.tsx#L79-L93

    useFocusEffect(
        useCallback(() => {
-            focusTimeoutRef.current = setTimeout(() => {
+            focusTimeoutRef.current = InteractionManager.runAfterInteractions(() => {
                if (inputRef.current) {
                    inputRef.current.focus();
-                }
+                });
                return () => {
                    if (!focusTimeoutRef.current) {
                        return;
                    }
                    clearTimeout(focusTimeoutRef.current);
                };
-            }, CONST.ANIMATED_TRANSITION);
        }, []),
    );

What alternative solutions did you explore? (Optional)

Alternatively, we can remove this block of code and add autoFocus and shouldDelayFocus props to InputWrapperWithRef. We can use shouldDelayFocus to ensure the keyboard opens smoothly on Android. Without shouldDelayFocus, the keyboard may open during screen transitions on Android, which is the main difference from the second proposal. Here’s an example:

// src/pages/iou/request/step/IOURequestStepDescription.tsx#L171
     <InputWrapperWithRef
        InputComponent={TextInput}
        inputID={INPUT_IDS.MONEY_REQUEST_COMMENT}
        name={INPUT_IDS.MONEY_REQUEST_COMMENT}
        defaultValue={currentDescription}
        label={translate('moneyRequestConfirmationList.whatsItFor')}
        accessibilityLabel={translate('moneyRequestConfirmationList.whatsItFor')}
        role={CONST.ROLE.PRESENTATION}
        ref={(el) => {
            if (!el) {
                return;
            }
            if (!inputRef.current) {
                updateMultilineInputRange(el);
            }
            inputRef.current = el;
        }}
        autoGrowHeight
        maxAutoGrowHeight={variables.textInputAutoGrowMaxHeight}
        shouldSubmitForm
        isMarkdownEnabled
        excludedMarkdownStyles={!isReportInGroupPolicy ? ['mentionReport'] : []}
+        autoFocus
+        shouldDelayFocus
    />

Test branch

POC FOR MAIN SOLUTION https://github.com/user-attachments/assets/382a1da2-09ef-4afc-9177-297a33ba57cb POC FOR MAIN ALTERNATIVE SOLUTION https://github.com/user-attachments/assets/03256b94-9002-4eca-a493-262225ebc966
truph01 commented 22 hours ago

Proposal

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

The field doesn't quite auto-focus. The border turns green as if it was auto-focused, but then in the end you have to tap again to get the cursor to focus into the field.

What is the root cause of that problem?

https://github.com/Expensify/App/blob/7256ad6eef046be3345e0b260252f6ec47fc4203/src/pages/iou/request/step/IOURequestStepDescription.tsx#L79-L93

It works well in all platform except IOS Safari because of the following:

We (Apple) like the current behavior and do not want programmatic focus to bring up the keyboard when you do not have a hardware keyboard attached

Source

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

https://github.com/Expensify/App/blob/7256ad6eef046be3345e0b260252f6ec47fc4203/src/pages/iou/request/step/IOURequestStepDescription.tsx#L171

to make sure the bug is fixed in safari and does not introduce regression in other platform.

What alternative solutions did you explore? (Optional)

Optional:

https://github.com/Expensify/App/blob/7256ad6eef046be3345e0b260252f6ec47fc4203/src/pages/iou/request/step/IOURequestStepDescription.tsx#L179

to:

ref={inputCallbackRef}

with:

    const {inputCallbackRef} = useAutoFocusInput(true)

and remove this useEffect