Skip to content

Fix various typing issues #1291

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.

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

Merged
merged 3 commits into from
May 11, 2021
Merged

Fix various typing issues #1291

merged 3 commits into from
May 11, 2021

Conversation

ethanshar
Copy link
Collaborator

Description

Fix various typing issues

Changelog

Fix Button iconSource type, Card.Section ImageProps to not require source and Text to support animated text style

@@ -39,6 +39,7 @@ export type TextProps = RNTextProps & TypographyModifiers & ColorsModifiers & Ma
*/
animated?: boolean;
textAlign?: string;
style?: StyleProp<TextStyle | Animated.AnimatedProps<TextStyle>>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to Omit<RNTextProps, 'style'> in TextProps

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, weirdly it didn't throw an error on that, anyway fixed it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should move more screens to tsx (found it there)

@@ -44,7 +44,7 @@ export type CardSectionProps = ViewProps & {
/**
* Other image props that will be passed to the image
*/
imageProps?: ImageProps;
imageProps?: Partial<ImageProps>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This: imageProps?: Omit<ImageProps, 'source' | 'style'>; feels a bit more accurate, WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really, technically you can also pass source in imageProps and it will work

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know, but I'm not sure it's intentional, approving

@ethanshar ethanshar requested a review from M-i-k-e-l May 11, 2021 05:40
@M-i-k-e-l M-i-k-e-l merged commit ab60fff into master May 11, 2021
@ethanshar ethanshar deleted the fix/typings branch November 30, 2021 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants