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

Jump to bottom

[TS migration] Migrate 'FormElement.js' component to TypeScript #31012

Merged
merged 13 commits into from
Nov 16, 2023
Merged

[TS migration] Migrate 'FormElement.js' component to TypeScript #31012

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
78a09b7
ref: moved FormElement to TS
kubabutkiewicz Oct 16, 2023
af566d8
Merge branch 'main' of github.com:kubabutkiewicz/expensify-app into t…
kubabutkiewicz Nov 6, 2023
d340487
fix: typing
kubabutkiewicz Nov 7, 2023
b52fbb3
Merge branch 'main' of github.com:kubabutkiewicz/expensify-app into t…
kubabutkiewicz Nov 7, 2023
b2dc291
fix: changed a type
kubabutkiewicz Nov 7, 2023
7e7602f
Revert "ref: moved FormElement to TS"
kubabutkiewicz Nov 8, 2023
1d8c3a3
Merge branch 'main' of github.com:kubabutkiewicz/expensify-app into t…
kubabutkiewicz Nov 8, 2023
5578518
Revert "ref: moved FormElement to TS"
kubabutkiewicz Nov 8, 2023
f3133cb
fix: removed unnecessary type
kubabutkiewicz Nov 8, 2023
30a3043
fix: changed dispalyName of component
kubabutkiewicz Nov 9, 2023
cadb376
Merge branch 'main' of github.com:kubabutkiewicz/expensify-app into t…
kubabutkiewicz Nov 13, 2023
627b85a
fix: resolve comments
kubabutkiewicz Nov 15, 2023
2571645
Merge branch 'main' of github.com:kubabutkiewicz/expensify-app into t…
kubabutkiewicz Nov 15, 2023
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
10 changes: 5 additions & 5 deletions ios/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ PODS:
- nanopb/encode (= 2.30908.0)
- nanopb/decode (2.30908.0)
- nanopb/encode (2.30908.0)
- Onfido (28.3.0)
- onfido-react-native-sdk (8.3.0):
- Onfido (~> 28.3.0)
- Onfido (27.4.0)
- onfido-react-native-sdk (7.4.0):
- Onfido (= 27.4.0)
- React
- OpenSSL-Universal (1.1.1100)
- Plaid (4.1.0)
Expand Down Expand Up @@ -1209,8 +1209,8 @@ SPEC CHECKSUMS:
MapboxMaps: af50ec61a7eb3b032c3f7962c6bd671d93d2a209
MapboxMobileEvents: de50b3a4de180dd129c326e09cd12c8adaaa46d6
nanopb: a0ba3315591a9ae0a16a309ee504766e90db0c96
Onfido: c7d010d9793790d44a07799d9be25aa8e3814ee7
onfido-react-native-sdk: b346a620af5669f9fecb6dc3052314a35a94ad9f
Onfido: e36f284b865adcf99d9c905590a64ac09d4a576b
onfido-react-native-sdk: 4ecde1a97435dcff9f00a878e3f8d1eb14fabbdc
kubabutkiewicz marked this conversation as resolved.
Show resolved Hide resolved
OpenSSL-Universal: ebc357f1e6bc71fa463ccb2fe676756aff50e88c
Plaid: 7d340abeadb46c7aa1a91f896c5b22395a31fcf2
PromisesObjC: 09985d6d70fbe7878040aa746d78236e6946d2ef
Expand Down
16 changes: 0 additions & 16 deletions src/components/FormElement.js
View file
Open in desktop

This file was deleted.

18 changes: 18 additions & 0 deletions src/components/FormElement.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React, {ForwardedRef, forwardRef, LegacyRef} from 'react';
import {View, ViewProps} from 'react-native';
import * as ComponentUtils from '@libs/ComponentUtils';

type FormElementProps = ViewProps;
kubabutkiewicz marked this conversation as resolved.
Show resolved Hide resolved
function FormElement(props: FormElementProps, ref: ForwardedRef<View | HTMLFormElement | null>) {
return (
<View
role={ComponentUtils.ACCESSIBILITY_ROLE_FORM}
ref={ref as LegacyRef<View>}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use the same type to avoid assertion?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, could you clarify please why do you use LegacyRef here and why ref is defined as ForwardedRef<View | HTMLFormElement | null>?

// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
/>
);
}

FormElement.displayName = 'BaseForm';
Copy link
Contributor

@barttom barttom Nov 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

is it a proper displayName? Doesn't follow file name

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 was here before so I am not sure if this should be changed in TS migration

Copy link
Contributor

Choose a reason for hiding this comment

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

If you take a look at blame here they must've missed it.

Let's change it to 'FormElement' to follow the guidelines, other than that LGTM!

kubabutkiewicz marked this conversation as resolved.
Show resolved Hide resolved
export default forwardRef(FormElement);
Loading