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

Added documentation for additional fields of Image #9304

Closed
wants to merge 1 commit into from

Conversation

DanielMSchmidt
Copy link
Contributor

Hi there,

As @ospfranco found there is some documentation for the code coming from #7338 missing, so I added it. It concerns the method, headers and bdoy field on the Image source object.

If you would like to have any changes made, please don't hesitate to comment, I will add them.

Have a nice day and thank you for maintaining React Native!

@ghost
Copy link

ghost commented Aug 9, 2016

By analyzing the blame information on this pull request, we identified @davidaurelio and @JoelMarcey to be potential reviewers.

@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 Aug 9, 2016
docs/Images.md Outdated
@@ -91,6 +91,22 @@ Many of the images you will display in your app will not be available at compile
<Image source={{uri: 'https://facebook.github.io/react/img/logo_og.png'}} />
```

### Network Requests for Images

If you would like to set things as HTTP-Verb, Headers or a Body along with the image request you may do this by defining these properties on the source object:

Choose a reason for hiding this comment

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

nit: comma after "image request"

Choose a reason for hiding this comment

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

Should it be "set such things as" ?

@JoelMarcey
Copy link

Thanks. Just a couple of comments. Then we can ship.

@ospfranco
Copy link
Contributor

thanks @DanielMSchmidt please bear in mind, the android counterpart of that commit has not been merged yet, 7791, so current documentation should at least mention this? or maybe it is better to hold it until it has been merged

@ghost ghost 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 Aug 9, 2016
@ghost
Copy link

ghost commented Sep 8, 2016

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @davidaurelio as a potential reviewer. Could you take a look please or cc someone with more context?

@ghost ghost 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 Sep 8, 2016
@JoelMarcey
Copy link

I think we may still be waiting for #7791 to be merged here -- is that right @ospfranco?

@JoelMarcey
Copy link

The small changes I requested can be made in the meantime.

@ghost ghost 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 Sep 9, 2016
@mkonicek
Copy link
Contributor

mkonicek commented Sep 9, 2016

Thanks for the review @JoelMarcey 👍

@ghost ghost 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 Sep 9, 2016
@DanielMSchmidt
Copy link
Contributor Author

DanielMSchmidt commented Sep 9, 2016

I just updated the PR to deal with your comments @joelcloralt

@facebook-github-bot
Copy link
Contributor

@DanielMSchmidt updated the pull request - view changes

@JoelMarcey
Copy link

Thanks. SO now we just need to decide whether this needs to wait for #7791

@ghost ghost 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 Sep 9, 2016
@facebook-github-bot
Copy link
Contributor

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @davidaurelio as a potential reviewer. Could you take a look please or cc someone with more context?

@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 Oct 10, 2016
@JoelMarcey
Copy link

#7791 is still open. We are still waiting for that, right?

@ospfranco
Copy link
Contributor

Hey @JoelMarcey, sorry I was short on time, yes #7791 is still open:

it was dependent on a change @rigdern made on Fresco framework for android, now that is merged so we are waiting on the pull request to be merged to react-native, once that is merged you can push this too.

@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 Oct 12, 2016
@hramos
Copy link
Contributor

hramos commented Nov 10, 2016

Waiting on #7791 ...

@lacker
Copy link
Contributor

lacker commented Dec 14, 2016

Still waiting on this functionality actually getting in.

@hramos
Copy link
Contributor

hramos commented Jan 19, 2017

Waiting...

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Apr 19, 2017
@facebook-github-bot
Copy link
Contributor

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

@javache
Copy link
Member

javache commented Apr 19, 2017

Could you rebase this change?

Add method, headers and body example for Image component
@DanielMSchmidt
Copy link
Contributor Author

@javache Did so 👍

@facebook-github-bot
Copy link
Contributor

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

Maxwell2022 pushed a commit to Maxwell2022/react-native that referenced this pull request Apr 19, 2017
Summary:
Hi there,

As ospfranco found there is some documentation for the code coming from facebook#7338 missing, so I added it. It concerns the method, headers and bdoy field on the Image source object.

If you would like to have any changes made, please don't hesitate to comment, I will add them.

Have a nice day and thank you for maintaining React Native!
Closes facebook#9304

Differential Revision: D4913262

Pulled By: javache

fbshipit-source-id: 922430ec3388560686e1cf53cb5dff7f30e4e31f
thotegowda pushed a commit to thotegowda/react-native that referenced this pull request May 7, 2017
Summary:
Hi there,

As ospfranco found there is some documentation for the code coming from facebook#7338 missing, so I added it. It concerns the method, headers and bdoy field on the Image source object.

If you would like to have any changes made, please don't hesitate to comment, I will add them.

Have a nice day and thank you for maintaining React Native!
Closes facebook#9304

Differential Revision: D4913262

Pulled By: javache

fbshipit-source-id: 922430ec3388560686e1cf53cb5dff7f30e4e31f
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.

8 participants