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

fix: add width, height and crossOrigin props on Image type declaration file #36196

Closed

Conversation

alvessteve
Copy link
Contributor

Summary

According this Issue #36182, the typescript compiler doesn't know the width, height and crossOrigin props for the Image component although they are taken in account in the code, docs and the flow types.

Changelog

[GENERAL] [FIXED] - Fix missing height, width, crossOrigin props on Typescript Image.d.ts file

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 17, 2023
@github-actions
Copy link

Warnings
⚠️ 📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.

Generated by 🚫 dangerJS against 6452572

@redpanda-bit
Copy link

Image.d.ts is also missing objectFit, referrerPolicy, src, srcSet, tintColor which are found in the docs. I'd say either submit a PR for all of the missing props or just submit a PR targeting height and width only. Otherwise crossOrigin is just random for this PR.

@facebook-github-bot
Copy link
Contributor

@rshest has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@rshest
Copy link
Contributor

rshest commented Feb 17, 2023

@alvessteve Looks ok to me, but please do take into account the @carlosalmonte04 comment.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Feb 18, 2023
@facebook-github-bot
Copy link
Contributor

@rshest merged this pull request in bcf493f.

@redpanda-bit
Copy link

TL;DR: It's more organized having PR's be associated to a specific task.

I guess it's not a big deal around here. crossOrigin deviated from the issue and it didn't fix the other problem fully either, which was adding all the props from the docs.

@alvessteve
Copy link
Contributor Author

thank you for the comments @carlosalmonte04 and @rshest ! As it was no big deal here, I put them together but next time I will split it.

@redpanda-bit
Copy link

Thank you. Please do. Otherwise it brings down repo morale and reputation.

@alvessteve alvessteve deleted the image-missing-height-width-props branch February 20, 2023 11:05
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…n file (facebook#36196)

Summary:
According this Issue [`https://github.com/facebook/react-native/issues/36182`](https://github.com/facebook/react-native/issues/36182), the typescript compiler doesn't know the `width`, `height` and `crossOrigin` props for the `Image` component although they are taken in account in the code, [docs](https://reactnative.dev/docs/image#crossorigin) and the flow types.

## Changelog

[GENERAL] [FIXED] - Fix missing `height`, `width`, `crossOrigin` props on Typescript Image.d.ts file

Pull Request resolved: facebook#36196

Reviewed By: christophpurrer

Differential Revision: D43406094

Pulled By: rshest

fbshipit-source-id: 547ed142d7c39c361d195275b79b0885ef829ba1
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. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants