App: [HOLD for payment 2023-07-06] [$1000] Android - Keyboard jumps when clicking on `fix the errors` after entering incorrect contact method

Proposal

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

Android/ios - Keyboard jumps when clicking on fix the errors after entering incorrect contact method

What is the root cause of that problem?

We are dismissing the keyboard every time fix the errors is pressed.

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

We have to add the condition if focusInput .blur is not a function only then dismiss the keyboard.

Add Following Lines if(focusInput.hasOwnProperty(‘focus’)){ Keyboard.dismiss() }

Or

Remove Keyboard.dismiss() and add following lines.

if(typeof focusInput.blur !== ‘function’){ Keyboard.dismiss(); } Or dismiss the keyboard in focus method of BasePicker and remove it from onFixErrors in Form component

Results ios

https://github.com/Expensify/App/assets/93134676/abdd192e-847b-4973-857b-1f4a1f0abb0d

https://github.com/Expensify/App/assets/93134676/f4fc6442-fc2b-481a-8cdc-76502922561b

Result android

https://github.com/Expensify/App/assets/93134676/bb54b70b-0c42-491a-98a2-98932037cc3f

https://github.com/Expensify/App/assets/93134676/977ab9a7-02b7-4601-af1b-f455b37d99e1

What alternative solutions did you explore? (Optional)

Awesome, issued the payment to you in Upwork @Nodebrute!

• Link to the PR: https://github.com/Expensify/App/pull/16433
• Link to comment: https://github.com/Expensify/App/pull/16433#discussion_r1253645184
• Link to discussion: N/A There in nothing we can add to checklist, We already have checklist items to test other affected components. Testing different scenarios will help catching these kind of bugs during PR review.
• Determine if we should create a regression test for this bug. Since this affect the Form , let’s add a regression test.

Regression Test Proposal

1. Open the App
2. Login with any account
3. Go to Settings -> Profile -> Contact method -> New contact method
4. Enter the incorrect contact method and press Add
5. Click on fix the errors several times
6. Verify the keyboard is open and consistent ( keyboard should not close or jump when clicking fix the errors

• Do we agree 👍 or 👎

@fedirjh Yup you are assignee of both issues I am working on 😂😂. I got confused.

@fedirjh I would like to add something hope this will make things clear @cubuspl42 thank you for raising that issue but I don’t think this is the right place to handle that issue.

If we do the Keyboard.dismiss() in BasePicker It will not dismiss the keyboard when checkbox is the only incomplete option. My Initial solution to check for blur methods will work for checkboxes as well as Pickers 🙏

Please note that this might require a minor update to the Checkbox class, probably fixing #12508 as a side-effect.

@cubuspl42 Why is this needed for this issue? How is that related to the suggested changes, as it will not affect the Checkbox component?

@fedirjh

I haven’t tested this, to be honest, I just assumed that. I’m a bit busy right now, but I could try testing this scenario later today.

Currently, the line which we discuss in Form…

// Start with dismissing the keyboard, so when we focus a non-text input, the keyboard is hidden
Keyboard.dismiss();

…has a comment saying when we focus a non-text input. I wrote that comment. “Non-text” input includes checkboxes. When you forget about an obligatory checkbox in a form while a text input is focused, and press “Fix the errors”, you’d like to hide the keyboard, analogically to the picker. It’s the reason Keyboard.dismiss(); landed in Form in the first place. But as it causes visual problems when both the “old” (previously focused) and the “new” (yet to be focused) input is a text input, as we’re aware now, we can move the responsibility of hiding the keyboard to non-text inputs’ focus method.

Currently, Checkbox on Native forwards to Pressable, for no particular reason I believe. We could implement focus in Checkbox (or in CheckboxWithLabel, maybe?) explicitly, as we do in BasePicker (and hide a keyboard in that method).

This will, kind of accidentally, fix an exception being thrown on iOS when you try to call focus on Pressable, because Pressable’s focus throws an exception on iOS (at least it used to). That’s what https://github.com/Expensify/App/issues/12508 is about (closed with a “won’t fix” resolution).

We should also ensure that all other Form-compatible inputs are fine with the proposed change, but I think they will be. There are not so many of them.

@cubuspl42 that’s a separate issue and is closed for good reasons. It has nothing to do with dismissing keyboard.

@payal-lathidadiya did you tested your solution with IOS ?

Hmm, seems like some issue with iOS. I’ll look into further investigation.

@tjferriss Please update the Op as this issue affects Native platforms : IOS and Android

Proposal

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

The keyboard unexpectedly jumps when pressing the “Fix the errors” button after inputting incorrect contact information.

What is the root cause of that problem?

The Keyboard.dismiss() invocation in the Form component is intended to hide the keyboard open for a previously focused text input, when the non-text input (especially a picker) is being focused.

The code, as written, is unfortunately also fired in the specific case when the text input we want to focus is already focused, resulting in the discussed unexpected behavior.

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

Approach 1

I suggest modifying the mentioned logic so it’s closer to the original intention. Let’s blur other inputs than the one we want to focus.

Instead of the unconditional…

Keyboard.dismiss();

…let’s do…

// Blur other inputs
_.values(inputRefs.current).forEach((input) => {
    if (input !== focusInput && input.blur && typeof input.blur === 'function') {
        input.blur();
    }
});

Approach 2

We can:

• remove the Keyboard.dismiss() call from Form
• modify the BasePicker.focus method so dismisses the keyboard (for the sake of Native):

    focus() {
        // This is required on Native and harmless on Web
        Keyboard.dismiss();

        if (this.props.shouldFocusPicker) {
            // Defer the focusing to work around a bug on Mobile Safari, where focusing the `select` element in the same
            // task when we scrolled to it left that element in a glitched state, where the dropdown list can't be opened
            // until the element gets re-focused
            _.defer(() => {
                this.picker.focus();
            });
        }
    }

We might need to ensure the Checkbox class is updated accordingly with this approach.