Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TS Migration] Adjustments on KYCWall #34870

Merged
merged 3 commits into from
Jan 23, 2024
Merged

[TS Migration] Adjustments on KYCWall #34870

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a68f19d
[TS Migration] Adjusted KYCWall based on some regressions
ruben-rebelo Jan 22, 2024
459a9f8
[TS migration] BaseKYCWall code improvements
ruben-rebelo Jan 22, 2024
f4b8565
[TS migration] KYCWall addressed suggestion
ruben-rebelo Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions src/components/KYCWall/BaseKYCWall.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, {useCallback, useEffect, useRef, useState} from 'react';
import type {SyntheticEvent} from 'react';
import {Dimensions} from 'react-native';
import type {EmitterSubscription, NativeTouchEvent} from 'react-native';
import type {EmitterSubscription, GestureResponderEvent} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import AddPaymentMethodMenu from '@components/AddPaymentMethodMenu';
Expand Down Expand Up @@ -68,8 +67,8 @@ function KYCWall({
walletTerms,
shouldShowPersonalBankAccountOption = false,
}: BaseKYCWallProps) {
const anchorRef = useRef<HTMLDivElement>(null);
const transferBalanceButtonRef = useRef<HTMLDivElement | null>(null);
const anchorRef = useRef<HTMLElement>(null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's related with this comment: #33714 (comment)
In order to make the types compatible with MenuItem and Button we had to change the event type which than the target is not compatible with HTMLDivElement and just with HTMLElement.

const transferBalanceButtonRef = useRef<HTMLElement | null>(null);

const [shouldShowAddPaymentMenu, setShouldShowAddPaymentMenu] = useState(false);

Expand Down Expand Up @@ -146,7 +145,7 @@ function KYCWall({
*
*/
const continueAction = useCallback(
(event?: SyntheticEvent<NativeTouchEvent>, iouPaymentType?: TransferMethod) => {
(event?: GestureResponderEvent | KeyboardEvent | undefined, iouPaymentType?: TransferMethod) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you have both ?:and | undefined. If the param is not required leave ?:, if it's always passed to the function, but can be undefined leave | undefined

const currentSource = walletTerms?.source ?? source;

/**
Expand All @@ -161,7 +160,7 @@ function KYCWall({
}

// Use event target as fallback if anchorRef is null for safety
const targetElement = anchorRef.current ?? (event?.nativeEvent.target as HTMLDivElement);
const targetElement = anchorRef.current ?? (event?.currentTarget as HTMLElement);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure this change works for all platforms (native/web)


transferBalanceButtonRef.current = targetElement;

Expand Down Expand Up @@ -203,7 +202,7 @@ function KYCWall({

Log.info('[KYC Wallet] User has valid payment method and passed KYC checks or did not need them');

onSuccessfulKYC(currentSource, iouPaymentType);
onSuccessfulKYC(iouPaymentType, currentSource);
},
[
bankAccountList,
Expand Down
8 changes: 4 additions & 4 deletions src/components/KYCWall/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {ForwardedRef, SyntheticEvent} from 'react';
import type {NativeTouchEvent} from 'react-native';
import type {ForwardedRef} from 'react';
import type {GestureResponderEvent} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type CONST from '@src/CONST';
Expand Down Expand Up @@ -64,10 +64,10 @@ type KYCWallProps = {
shouldShowPersonalBankAccountOption?: boolean;

/** Callback for the end of the onContinue trigger on option selection */
onSuccessfulKYC: (currentSource?: Source, iouPaymentType?: TransferMethod) => void;
onSuccessfulKYC: (iouPaymentType?: TransferMethod, currentSource?: Source) => void;

/** Children to build the KYC */
children: (continueAction: (event: SyntheticEvent<NativeTouchEvent>, method: TransferMethod) => void, anchorRef: ForwardedRef<HTMLElement>) => void;
children: (continueAction: (event: GestureResponderEvent | KeyboardEvent | undefined, method: TransferMethod) => void, anchorRef: ForwardedRef<HTMLElement>) => void;
};

export type {AnchorPosition, KYCWallProps, PaymentMethod, TransferMethod, DomRect};
6 changes: 3 additions & 3 deletions src/libs/actions/PaymentMethods.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {createRef} from 'react';
import type {MutableRefObject, SyntheticEvent} from 'react';
import type {NativeTouchEvent} from 'react-native';
import type {MutableRefObject} from 'react';
import type {GestureResponderEvent} from 'react-native';
import type {OnyxUpdate} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx/lib/types';
Expand All @@ -17,7 +17,7 @@ import type PaymentMethod from '@src/types/onyx/PaymentMethod';
import type {FilterMethodPaymentType} from '@src/types/onyx/WalletTransfer';

type KYCWallRef = {
continueAction?: (event?: SyntheticEvent<NativeTouchEvent>, iouPaymentType?: TransferMethod) => void;
continueAction?: (event?: GestureResponderEvent | KeyboardEvent | undefined, iouPaymentType?: TransferMethod) => void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

};

/**
Expand Down
Loading