[$250] Bank account - Selected digit is not turned green &number entered is entered in incorrect place

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.62-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: No
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch app
2. Go to workspace settings- enable workflow
3. Tap connect bank account - connect manually
4. Enter an incorrect magic code
5. Try to select any digit by tapping on it
6. Enter a number

Expected Result:

Selected digit must be turned green and number entered must be entered in correct place

Actual Result:

Selected digit is not turned green and number entered is entered in incorrect place

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
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6665364_1731595506656.Screenrecorder-2024-11-14-20-06-21-271_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021858629538288881697
• Upwork Job ID: 1858629538288881697
• Last Price Increase: 2024-11-25

[melvin-bot]

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

[truph01]

Edited by proposal-police: This proposal was edited at 2024-11-14 16:27:08 UTC.

Proposal

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

Selected digit is not turned green and number entered is entered in incorrect place

What is the root cause of that problem?

We're using GestureDetector in MagicCodeInput but we didn't wrap it in GestureHandlerRootView

Reference: https://docs.swmansion.com/react-native-gesture-handler/docs/fundamentals/installation/

We also wrapped GestureDetector inside GestureHandlerRootView in other places like AvatarCropModal, ...

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

We should wrap MagicCodeInput inside GestureHandlerRootView, so in here, we can add GestureHandlerRootView

<GestureHandlerRootView
...

We can add GestureHandlerRootView in BaseModal, and PopoverWithoutOverlay then remove the unnecessary GestureHandlerRootView in other places

What alternative solutions did you explore? (Optional)

Result

Screen.Recording.2024-11-14.at.23.24.55.mov

[bernhardoj]

Proposal

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

Pressing on a magic code digit input doesn't highlight the digit input.

What is the root cause of that problem?

The magic code input consists of a single TextInput and we apply a gesture handler to detecte on which digit input placeholder is pressed/tapped.

App/src/components/MagicCodeInput.tsx

Lines 213 to 226 in a6f1348

	const tapGesture = Gesture.Tap()
	.runOnJS(true)
	// eslint-disable-next-line react-compiler/react-compiler
	.onBegin((event) => {
	const index = Math.floor(event.x / (inputWidth.current / maxLength));
	shouldFocusLast.current = false;
	// TapGestureHandler works differently on mobile web and native app
	// On web gesture handler doesn't block interactions with textInput below so there is no need to run `focus()` manually
	if (!Browser.isMobileChrome() && !Browser.isMobileSafari()) {
	inputRefs.current?.focus();
	}
	setInputAndIndex(index);
	lastFocusedIndex.current = index;
	});

However, the gesture handler doesn't work when used inside a modal, which is true for our case. This is mentioned on the rngh docs here.

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

We need to wrap the modal content with a gesture handler root view. RNGH provides a HOC (gestureHandlerRootHOC ) that only wraps it on Android.

We can wrap the color scheme wrapper with the HOC.

App/src/components/Modal/BaseModal.tsx

Lines 272 to 277 in a6f1348

	<View
	style={[styles.defaultModalContainer, modalPaddingStyles, modalContainerStyle, !isVisible && styles.pointerEventsNone]}
	ref={ref}
	>
	<ColorSchemeWrapper>{children}</ColorSchemeWrapper>
	</View>

const ColorSchemeWrapperWithGesture = useMemo(() => gestureHandlerRootHOC(ColorSchemeWrapper, {
    flex: modalContainerStyle.flex,
    height: modalContainerStyle.height,
    width: modalContainerStyle.width,
}), [modalContainerStyle.flex, modalContainerStyle.height, modalContainerStyle.width]);
...

<ColorSchemeWrapperWithGesture>{children}</ColorSchemeWrapperWithGesture>

We need to pass the sizing style based on the modalContainerStyle so it can be sized properly when sizing using flex or percentage value.

OR

We wrap the View wrapper instead, but GetureHandlerRootView doesn't accept ref, but I don't see we use the ref either.

[truph01]

Note for C+: The link I mentioned here suggest to use GestureHandlerRootView or gestureHandlerRootHOC, but using gestureHandlerRootHOC has 2 disadvantages:

1. We don't encourage using HOCs, refer: https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md
2. We're using GestureHandlerRootView in many places

As in the docs, they said that we should use GestureHandlerRootView in the root view, but we didn't do it in BaseModal or PopoverWithoutOverlay. I believe there's a reason that encourages us to use GestureHandlerRootView in the individual modal.

[melvin-bot]

@johncschuster Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[melvin-bot]

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

[truph01]

@alitoshmatov Can you please review my proposal? Thanks

[johncschuster]

Bumped in Slack

[melvin-bot]

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

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[johncschuster]

I'm working a shorter week this week and couldn't fit this in today

[johncschuster]

(I've bumped Slack)

[alitoshmatov]

@truph01 Thank you for your proposal, I suggest you provide more explanation on your root causes it seemed like it was just justifying your solution. I understood it after seeing @bernhardoj 's reasoning. Though your solution is correct and solves the issue.

[alitoshmatov]

@bernhardoj Thank you for your proposal, your solution works, but I think it is better to just use GestureHandlerRootView inside modal component fixing GestureDetector to work in any modal

[melvin-bot]

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

[bernhardoj]

My proposal also adds GestureHandlerRootView inside the modal through the gestureHandlerRootHOC. gestureHandlerRootHOC only applies it on Android.

, but I think it is better to just use GestureHandlerRootView

I don't see a reason why we prefer applying it to all platforms when the docs recommend using the HOC because it's an Android issue only. @truph01 proposal also doesn't handle the style break after applying GestureHandlerRootView.

[melvin-bot]

@johnmlee101 @johncschuster @alitoshmatov this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[truph01]

@bernhardoj The docs recommend we use GestureHandlerRootView or gestureHandlerRootHOC.

After installation, wrap your entry point with <GestureHandlerRootView> or gestureHandlerRootHOC.

But HOC is not recommend in our App.

@truph01 proposal also doesn't handle the style break after applying GestureHandlerRootView.

If we decide to add it in BaseValidateCodeForm, we don't need to add styles, you can see my video in the proposal. We just need to apply modalContainerStyle in case we add it in BaseModal

[bernhardoj]

The docs recommend we use GestureHandlerRootView or gestureHandlerRootHOC.

I'm talking about this part of the doc: https://docs.swmansion.com/react-native-gesture-handler/docs/fundamentals/installation#usage-with-modals-on-android

But HOC is not recommend in our App.

This is not true.

Hooks and HOCs
Use hooks whenever possible, avoid using HOCs.

The docs stated to use hooks whenever possible, but GestureHandlerRootView is a View and not replaceable by a hook.

[bernhardoj]

Looks like @alitoshmatov think that my proposal doesn't solve the issue globally which in fact does. Can you recheck all the conversation above?

[truph01]

We also wrapped GestureDetector inside GestureHandlerRootView in other places like AvatarCropModal, ...

So I believe we prefer using GestureHandlerRootView

[melvin-bot]

@johnmlee101, @johncschuster, @alitoshmatov Huh... This is 4 days overdue. Who can take care of this?

[alitoshmatov]

Looks like @alitoshmatov think that my proposal doesn't solve the issue globally which in fact does. Can you recheck all the conversation above?

I am sorry, I think I miscommunicated there about this point.

[alitoshmatov]

My though process was that since we use GestureHandlerRootView in several places for modals, why not remove them all and apply it BaseModal component

[alitoshmatov]

@bernhardoj Can we also remove all GestureHandlerRootView usages when applying your solution?

[bernhardoj]

Yes, we should be able to do that since the HOC wraps the component with GestureHanderRootView, but it's only for Android based on the doc.

I mentioned this in my proposal too.

We need to wrap the modal content with a gesture handler root view. RNGH provides a HOC (gestureHandlerRootHOC ) that only wraps it on Android.

[truph01]

@alitoshmatov GestureHandlerRootView also can apply in BaseModal

[alitoshmatov]

Issue not reproducible during KI retests. (First week)

I also couldn't reproduce the issue on main

[bernhardoj]

It was fixed in #52802.

[alitoshmatov]

Looks like this issue was duplicate of #51293 and is resolved now

[alitoshmatov]

I think we can close this issue

[melvin-bot]

@johnmlee101 @johncschuster @alitoshmatov this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!