Infra/rn73 #3264
Infra/rn73 #3264
Conversation
ethanshar
requested review from
Inbal-Tish,
adids1221,
M-i-k-e-l and
nitzanyiz
| @@ -104,7 +104,7 @@ describe('Incubator.Dialog sanity checks', () => { | |||
| const onDismiss = jest.fn(); | |||
| const component = <TestCase2 onDismiss={onDismiss}/>; | |||
| const {dialogDriver} = getDriver(component); | |||
| expect(dialogDriver.exists()).toBeTruthy(); | |||
| expect(dialogDriver.exists()).toBeFalsy(); | |||
There was a problem hiding this comment.
You need to change the description of the test - Modal should not exist...
| const {renderTree, testID} = props; | ||
|
|
||
| const driver = usePressableDriver(useComponentDriver({renderTree, testID})); | ||
| const isUsingDialog = !!renderTree.queryByTestId(`${testID}.overlay.modal`); | ||
| // const isUsingDialog = !!renderTree.queryByTestId(`${testID}.overlay.modal`); | ||
| const isUsingDialog = useDialog; |
There was a problem hiding this comment.
Why we need isUsingDialog const now?
src/testkit/new/Component.driver.ts
Outdated
| export interface ComponentProps { | ||
| renderTree: ReturnType<typeof within>; // Note: This changed was asked for integration with amino. This still gives all querying functionality. | ||
| testID: string | RegExp; | ||
| } | ||
|
|
||
| export interface ComponentDriverResult { | ||
| getElement: () => ReactTestInstance; | ||
| queryElement:() => ReactTestInstance | undefined; |
There was a problem hiding this comment.
Missing a space after the colon
|
@ethanshar Looks good. Left super small comments and you need to take it out of draft mode |
| @@ -42,11 +42,11 @@ export type CardProps = ViewProps & | |||
| /** | |||
| * card custom width | |||
| */ | |||
| width?: number | string; | |||
| width?: DimensionValue; | |||
There was a problem hiding this comment.
DimensionValue doesn't support string value without %.
Do we want to remove the string support from these props?
Relevant for all the components with DimensionValue type.
There was a problem hiding this comment.
What do you mean by "remove string support"?
Do we do anything special with the strings? we just pass it on to style props..
|
Managed to install Android after updating Android Studio and jdk to version 17. |
| { | ||
| extensions: ['.js', '.jsx', '.ts', '.tsx'], | ||
| root: ['.'], | ||
| alias: { |
There was a problem hiding this comment.
Can we delete src/.babelrc.json? Or remove some of the configuration there?
There was a problem hiding this comment.
I'm not sure TBH, I forgot about this file... now I see it did the same thing in the main babel
metro.config.js
Outdated
| // inlineRequires: false | ||
| // } | ||
| // }), | ||
| // babelTransformerPath: require.resolve('react-native-svg-transformer') |
There was a problem hiding this comment.
SVG file is not working in RN73 (did not test in master yet)
There was a problem hiding this comment.
Yes, I put it aside during upgrade. Fixed now
Thanks 🙏
src/components/dialog/index.tsx
Outdated
| /** | ||
| * The dialog height (default: undefined) | ||
| */ | ||
| height?: string | number | null; | ||
| height?: DimensionValue | null; |
There was a problem hiding this comment.
| height?: DimensionValue | null; | |
| height?: DimensionValue; |
There was a problem hiding this comment.
I think the null is on purpose
That's the use way to make the height be content based
There was a problem hiding this comment.
I think that DimensionValue already has null (can't verify since it's not installed ATM)
There was a problem hiding this comment.
You are right, fixed
Description
I know it's a huge PR, I suggest going over it by commits
There are other less important small commits in between...
Changelog
Upgrade support to React Native 0.73
Additional info