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.

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

Add missing Android-only props to ImageProps #47719

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dmytrorykun
Copy link
Contributor

Summary:
There is a subset of ImageProps that are used by Android but not used by iOS. They were missing in the ImageProps.h. It was fine when they were only consumed by the Android mounting layer. But it becomes a problem if we want to write some shared C++ code based on those props.
This diff adds those props to the corresponding C++ type.
This should have no practical effect for both platforms.
Changelog: [Internal]

Differential Revision: D65426569

Summary:

There is Android code that supports remapping `defaultSrc` -> `defaultSource` for `ImageNativeComponent`.
The docs reference this prop as `defaultSource`: https://reactnative.dev/docs/next/image#defaultsource
It is not referenced as `defaultSrc` anywhere. Let's unify it as `defaultSource` across both platforms.
Changelog: [Internal]

Differential Revision: D65819218
Summary:

`defaultSource` is defined as a single `ImageSource` in docs (https://reactnative.dev/docs/next/image#defaultsource) but its type is a vector of `ImageSource`s in `ImageProps.h`.

This diff changes its C++ type to just `ImageSource`.

Technically, this is a breaking change, however I don't think folks should directly access `ImageProps` outside of `RCTImageComponentView.mm`
Moreover, this prop is actually not implemented in Fabric, so this change should have no practical effect.

Changelog: [Internal]

Facebook
T208171435 - [RN][Fabric][iOS] Implement defaultSource support for Image

Differential Revision: D65821570
Summary:
There is a subset of `ImageProps` that are used by Android but not used by iOS. They were missing in the `ImageProps.h`. It was fine when they were only consumed by the Android mounting layer. But it becomes a problem if we want to write some shared C++ code based on those props.
This diff adds those props to the corresponding C++ type.
This should have no practical effect for both platforms.
Changelog: [Internal]

Differential Revision: D65426569
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Nov 19, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D65426569

dmytrorykun added a commit to dmytrorykun/react-native that referenced this pull request Nov 19, 2024
Summary:

There is a subset of `ImageProps` that are used by Android but not used by iOS. They were missing in the `ImageProps.h`. It was fine when they were only consumed by the Android mounting layer. But it becomes a problem if we want to write some shared C++ code based on those props.
This diff adds those props to the corresponding C++ type.
This should have no practical effect for both platforms.
Changelog: [Internal]

Reviewed By: javache

Differential Revision: D65426569
dmytrorykun added a commit to dmytrorykun/react-native that referenced this pull request Nov 19, 2024
Summary:

There is a subset of `ImageProps` that are used by Android but not used by iOS. They were missing in the `ImageProps.h`. It was fine when they were only consumed by the Android mounting layer. But it becomes a problem if we want to write some shared C++ code based on those props.
This diff adds those props to the corresponding C++ type.
This should have no practical effect for both platforms.
Changelog: [Internal]

Reviewed By: javache

Differential Revision: D65426569
dmytrorykun added a commit to dmytrorykun/react-native that referenced this pull request Nov 19, 2024
Summary:

There is a subset of `ImageProps` that are used by Android but not used by iOS. They were missing in the `ImageProps.h`. It was fine when they were only consumed by the Android mounting layer. But it becomes a problem if we want to write some shared C++ code based on those props.
This diff adds those props to the corresponding C++ type.
This should have no practical effect for both platforms.
Changelog: [Internal]

Reviewed By: javache

Differential Revision: D65426569
dmytrorykun added a commit to dmytrorykun/react-native that referenced this pull request Nov 19, 2024
Summary:

There is a subset of `ImageProps` that are used by Android but not used by iOS. They were missing in the `ImageProps.h`. It was fine when they were only consumed by the Android mounting layer. But it becomes a problem if we want to write some shared C++ code based on those props.
This diff adds those props to the corresponding C++ type.
This should have no practical effect for both platforms.
Changelog: [Internal]

Reviewed By: javache

Differential Revision: D65426569
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants