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

Specify width and height for Image(remote source) #8486

Closed
wants to merge 1 commit into from

Conversation

sunnylqm
Copy link
Contributor

No description provided.

@ghost
Copy link

ghost commented Jun 29, 2016

By analyzing the blame information on this pull request, we identified @lacker to be a potential reviewer.

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jun 29, 2016
@javache
Copy link
Member

javache commented Jun 29, 2016

@facebook-github-bot shipit

@ghost ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jun 29, 2016
@ghost
Copy link

ghost commented Jun 29, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review.

@ghost ghost closed this in 6b2a49e Jun 29, 2016
@lacker
Copy link
Contributor

lacker commented Jun 29, 2016

Hey, is this change needed? At this point if you are reading the tutorial linearly the reader has not learned about "style" yet, and the text only mentions the "source" prop, so it's a bit odd. We were trying to avoid extraneous styling in the examples.

@lacker lacker reopened this Jun 29, 2016
@ghost
Copy link

ghost commented Jun 29, 2016

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

@ghost ghost added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Jun 29, 2016
@sunnylqm
Copy link
Contributor Author

@lacker I know what you mean. But the fact is that "style" is required for Image, otherwise the readers would get nothing to show on their screens. (Somehow the react-native-web-player shows the image because the different implementations between web and RN I guess). So either change to a simpler example which would work with only one prop, or have some words about style here. I am not a native speaker: (, and not so confident with long sentences. So I choose the simplest way to PR, that is, to remind you since issues would probably be ignored.

@lacker
Copy link
Contributor

lacker commented Jun 30, 2016

Arrrr ok. Let's get this in and then I can mess around with the wording later. The bot seems to have rejected it though... lets just try again, if it doesnt work again maybe it needs a rebase.

@lacker
Copy link
Contributor

lacker commented Jun 30, 2016

@facebook-github-bot shipit

@ghost ghost added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jun 30, 2016
@ghost
Copy link

ghost commented Jun 30, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review.

@sunnylqm
Copy link
Contributor Author

Seems already done 6b2a49e

@lacker
Copy link
Contributor

lacker commented Jun 30, 2016

Oh - I'm confused why the Import Failed tag is on here then. Dunno....

@sunnylqm
Copy link
Contributor Author

sunnylqm commented Jun 30, 2016

And I forgot the space between style and the ending tag...sorry to left it to you.

@JoelMarcey
Copy link

Optimally, the import failed bot should be removed after import started and pull request closed. Just a tooling thing.

samerce pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
Summary: Closes facebook#8486

Differential Revision: D3496850

Pulled By: javache

fbshipit-source-id: f2456b914c420089558484fb87e3d92ed2c923af
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
Summary: Closes facebook#8486

Differential Revision: D3496850

Pulled By: javache

fbshipit-source-id: f2456b914c420089558484fb87e3d92ed2c923af
@sunnylqm sunnylqm deleted the patch-30 branch December 7, 2016 07:05
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. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants