[$500] Android - Connect manually button does not trigger haptic feedback vibration

[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.48-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: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:

Issue found when executing PR https://github.com/Expensify/App/pull/37108

Action Performed:

1. Navigate to WS settings 2.Tap Bank account 3.Tap and long press on Connect manually button

Expected Result:

Feedback vibration should occur (As occurring when long pressed on Connect with Plaid button)

Actual Result:

No haptic vibration occurs if long pressed on the button Connect manually Note: It is a new feature, so can not be tested in Production

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/d8e99f3e-ae45-419c-83fc-79db90d03e06

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f1461856a7e2c854
• Upwork Job ID: 1765956089934372864
• Last Price Increase: 2024-03-08

[melvin-bot[bot]]

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

[lanitochka17]

We think that this bug might be related to #wave8 CC @zanyrenney

[lanitochka17]

@bfitzexpensify 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

[ShridharGoel]

Proposal

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

Connect manually button does not trigger haptic feedback vibration

What is the root cause of that problem?

Haptic feedback functionality is not enabled for connect manually.

https://github.com/Expensify/App/blob/86a990428df2c923ed4ebc291c0a3a73653cb7fe/src/pages/ReimbursementAccount/BankAccountStep.js#L154-L166

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

Use Button instead of MenuItem like it is being done for connect with plaid option. Then, shouldEnableHapticFeedback needs to be passed as true in Button for the connect manually button.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[ghost]

Proposal

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

Android - Connect manually button does not trigger haptic feedback vibration

What is the root cause of that problem?

The root cause is we are not passing shouldEnableHapticFeedback here : https://github.com/Expensify/App/blob/86a990428df2c923ed4ebc291c0a3a73653cb7fe/src/pages/ReimbursementAccount/BankAccountStep.js#L155-L165 which is basically Connect Manually button.

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

We should pass shouldEnableHapticFeedback here : https://github.com/Expensify/App/blob/86a990428df2c923ed4ebc291c0a3a73653cb7fe/src/pages/ReimbursementAccount/BankAccountStep.js#L155-L165 which will make it true, which by default it is false.

What alternative solutions did you explore? (Optional)

N/A

[tienifr]

Proposal

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

No haptic vibration occurs if long pressed on the button Connect manually Note: It is a new feature, so can not be tested in Production

What is the root cause of that problem?

We never have haptic feedback handling for MenuItem, which is being used for the Connect manually button.

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

I found that we already have the shouldUseHapticsOnPress and shouldUseHapticsOnLongPress property in GenericPressable so just passing it down as true from MenuItem should be enough.

So we need to:

1. Add shouldUseHapticsOnPress (and optionally shouldUseHapticsOnLongPress) prop to MenuItem, and pass it down to PressableWithSecondaryInteraction and then to PressableWithFeedback. We can set a custom defaults for those 2 props in MenuItem, depending on our expected behavior (like if we expect all MenuItem to have haptics by default), or make it optional so that it will fallback to the defaults in GenericPressable
2. Pass shouldUseHapticsOnPress (and optionally shouldUseHapticsOnLongPress) prop as true to MenuItem of the connect manually button, and any other MenuItem/Button that needs the behavior.

The Button has its own logic of shouldEnableHapticFeedback, if we want to be more DRY we can refactor to use the one in PressableWithFeedback instead.

Just to give more context, the above proposals are incorrect because there's no such shouldEnableHapticFeedback property or any haptic handling in MenuItem.

What alternative solutions did you explore? (Optional)

1. Add haptic feedback handling for MenuItem, just like we did for Button here and here, the default value of shouldEnableHapticFeedback it will be false.

Ideally we can make it a low level property of PressableWithFeedback itself, then pass the shouldEnableHapticFeedback from MenuItem to the PressableWithSecondaryInteraction and to PressableWithFeedback. Then in Button we can do a bit of refactor similarly to be DRY.

2. Pass shouldEnableHapticFeedback as true for the MenuItem used for connect manually button, and any other MenuItem/Button that needs the behavior.

[wildan-m]

Proposal

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

Android - Connect manually button does not trigger haptic feedback vibration

What is the root cause of that problem?

We use MenuItem for the 'Connect Manually' button, but MenuItem does not implement HapticFeedback like Button.

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

We use custom HapticFeedback for Button:

https://github.com/Expensify/App/blob/77375e86525257fcd7ccd9f813f9c6dc7d125f4b/src/components/Button/index.tsx#L280-L299

To ensure MenuItem and Button have similar behavior, we can implement the same HapticFeedback for MenuItem.

Change onPressAction to:

const onPressAction = (event: GestureResponderEvent | KeyboardEvent | undefined) => {
        if (disabled || !interactive) {
            return;
        }

        if (event?.type === 'click') {
            (event.currentTarget as HTMLElement).blur();
        }

        if (shouldEnableHapticFeedback) {
            HapticFeedback.press();
        }
.....

And onSecondaryAction to:

                    onSecondaryInteraction={(event) => {
                        shouldEnableHapticFeedback && HapticFeedback.longPress();
                        onSecondaryInteraction?.(event);
                    }}

What alternative solutions did you explore? (Optional)

Use Button instead of MenuItem for 'Connect Manually', it will ensure they both have the same behavior. This change will also alter the current styling behavior of Connect Manually.

[melvin-bot[bot]]

@fedirjh, @bfitzexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[fedirjh]

@bfitzexpensify I see that Haptic feedback was implemented in https://github.com/Expensify/App/issues/35070. My question is whether this is a valid bug or not? Looking at the regression test proposal, it seems that haptic feedback should only be enabled for long press that triggers an action (which doesn't seem to be the case in this issue).

cc @mountiny @ishpaul777 curious about your thoughts on this.

[mountiny]

Yep I think this is only for long press where that ends in a different event than classic tap/ press.

This is not the case