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 support for web-style data-uris #576

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
@colinramsay
Contributor

colinramsay commented Apr 1, 2015

This implements base64-encoded data-uris for the Image component.

Add support for web-style data-uris
This implements base64-encoded data-uris for the <Image> component.
@iamdustan

This comment has been minimized.

Show comment
Hide comment
@iamdustan

iamdustan Apr 1, 2015

👏 I’m all for this!

👏 I’m all for this!

@vkurchatkin

This comment has been minimized.

Show comment
Hide comment
@vkurchatkin

vkurchatkin Apr 1, 2015

Contributor

I wonder, what is the use case for this?

Contributor

vkurchatkin commented Apr 1, 2015

I wonder, what is the use case for this?

@colinramsay

This comment has been minimized.

Show comment
Hide comment
@colinramsay

colinramsay Apr 1, 2015

Contributor

It came up from this question on SO:

http://stackoverflow.com/questions/29380265/does-react-native-support-base64-encoded-images

I think it's one of those things that's expected to just work.

Contributor

colinramsay commented Apr 1, 2015

It came up from this question on SO:

http://stackoverflow.com/questions/29380265/does-react-native-support-base64-encoded-images

I think it's one of those things that's expected to just work.

@skpodcast

This comment has been minimized.

Show comment
Hide comment
@skpodcast

skpodcast Apr 1, 2015

i am going to build SK genesis

i am going to build SK genesis

@vkurchatkin

This comment has been minimized.

Show comment
Hide comment
@vkurchatkin

vkurchatkin Apr 1, 2015

Contributor

What I mean is storing/transferring images in base64 is pretty wasteful

Contributor

vkurchatkin commented Apr 1, 2015

What I mean is storing/transferring images in base64 is pretty wasteful

@iamdustan

This comment has been minimized.

Show comment
Hide comment
@iamdustan

iamdustan Apr 1, 2015

@vkurchatkin I agree with @colinramsay that Base64 images should just work.

We’re using google drive spreadsheets to power a few internal tools. Images are often inserted as base64 images to guarantee their presence.

@vkurchatkin I agree with @colinramsay that Base64 images should just work.

We’re using google drive spreadsheets to power a few internal tools. Images are often inserted as base64 images to guarantee their presence.

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Apr 1, 2015

Contributor

👍 we have this in our internal image component and its pretty handy

Contributor

vjeux commented Apr 1, 2015

👍 we have this in our internal image component and its pretty handy

@wootwoot1234

This comment has been minimized.

Show comment
Hide comment
@wootwoot1234

wootwoot1234 Apr 1, 2015

I think this could be really useful because it will allow the use of js libraries that generate base64 images. For example, I'm currently wanting to generate a simple graph.

I think this could be really useful because it will allow the use of js libraries that generate base64 images. For example, I'm currently wanting to generate a simple graph.

@foghina

This comment has been minimized.

Show comment
Hide comment
@foghina

foghina Apr 7, 2015

Contributor

Merged internally and synced.

Contributor

foghina commented Apr 7, 2015

Merged internally and synced.

@foghina foghina closed this Apr 7, 2015

vjeux pushed a commit to vjeux/react-native that referenced this pull request Apr 13, 2015

Add support for web-style data-uris
Summary:
This implements base64-encoded data-uris for the Image component.
Closes facebook#576
Github Author: Colin Ramsay <colinramsay@gmail.com>

Test Plan: Imported from GitHub, without a `Test Plan:` line.

vjeux pushed a commit to vjeux/react-native that referenced this pull request Apr 14, 2015

Add support for web-style data-uris
Summary:
This implements base64-encoded data-uris for the Image component.
Closes facebook#576
Github Author: Colin Ramsay <colinramsay@gmail.com>

Test Plan: Imported from GitHub, without a `Test Plan:` line.

vjeux pushed a commit to vjeux/react-native that referenced this pull request Apr 15, 2015

Add support for web-style data-uris
Summary:
This implements base64-encoded data-uris for the Image component.
Closes facebook#576
Github Author: Colin Ramsay <colinramsay@gmail.com>

Test Plan: Imported from GitHub, without a `Test Plan:` line.
@rt2zz

This comment has been minimized.

Show comment
Hide comment
@rt2zz

rt2zz Apr 21, 2015

Contributor

heads up for people using data uris, you will need isStatic: true e.g.
<Image source={{uri: 'data:image/jpeg;base64,'+ base64ImageString, isStatic: true}} />

Contributor

rt2zz commented Apr 21, 2015

heads up for people using data uris, you will need isStatic: true e.g.
<Image source={{uri: 'data:image/jpeg;base64,'+ base64ImageString, isStatic: true}} />

@PaulBGD

This comment has been minimized.

Show comment
Hide comment
@PaulBGD

PaulBGD Oct 10, 2015

Thanks @rt2zz! That should be somewhere in the doc.

PaulBGD commented Oct 10, 2015

Thanks @rt2zz! That should be somewhere in the doc.

@geirman

This comment has been minimized.

Show comment
Hide comment
@geirman

geirman Dec 26, 2015

Contributor

I'm unclear whether this has made it into a release. Doesn't appear so, and I'm not sure whether "Commit has since been removed from the repository and is no longer available." means it never will or not. Can someone explain?

Contributor

geirman commented Dec 26, 2015

I'm unclear whether this has made it into a release. Doesn't appear so, and I'm not sure whether "Commit has since been removed from the repository and is no longer available." means it never will or not. Can someone explain?

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Dec 26, 2015

Contributor

we didn't have a good sync tooling at the time. Should be there

Contributor

vjeux commented Dec 26, 2015

we didn't have a good sync tooling at the time. Should be there

@geirman

This comment has been minimized.

Show comment
Hide comment
@geirman

geirman Dec 26, 2015

Contributor

Thanks @vjeux, it occurred to me I could just check, so I did. I wasn't able to find the code changes in master, nor could I find any reference to it in the RCTConvert.m history. To make doubly sure I'm not a complete idiot (which is totally still possible), I setup a test at rnplay using the isStatic: true referenced in @rt2zz's April 21st comment and get an error No suitable image URL loader found.

Contributor

geirman commented Dec 26, 2015

Thanks @vjeux, it occurred to me I could just check, so I did. I wasn't able to find the code changes in master, nor could I find any reference to it in the RCTConvert.m history. To make doubly sure I'm not a complete idiot (which is totally still possible), I setup a test at rnplay using the isStatic: true referenced in @rt2zz's April 21st comment and get an error No suitable image URL loader found.

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Dec 26, 2015

Contributor

I can see it here:

} else if ([scheme isEqualToString:@"data"]) {
image = [UIImage imageWithData:[NSData dataWithContentsOfURL:URL]];

But, the comment at the top says "/* This method is only used when loading images synchronously, e.g. for tabbar icons */" so maybe it's not working.

Would happily take in a pull request that actually makes it work :)

Contributor

vjeux commented Dec 26, 2015

I can see it here:

} else if ([scheme isEqualToString:@"data"]) {
image = [UIImage imageWithData:[NSData dataWithContentsOfURL:URL]];

But, the comment at the top says "/* This method is only used when loading images synchronously, e.g. for tabbar icons */" so maybe it's not working.

Would happily take in a pull request that actually makes it work :)

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Dec 26, 2015

Contributor

Also, isStatic should no longer be needed.

Contributor

vjeux commented Dec 26, 2015

Also, isStatic should no longer be needed.

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Dec 26, 2015

Contributor

Looks like most of the image code blames to @nicklockwood, maybe he knows what's going on here

Contributor

vjeux commented Dec 26, 2015

Looks like most of the image code blames to @nicklockwood, maybe he knows what's going on here

@nicklockwood

This comment has been minimized.

Show comment
Hide comment
@nicklockwood

nicklockwood Dec 27, 2015

Contributor

@geirman your code looks OK to me (as @vjeux says, isStatic isn't needed, but it shouldn't stop it from working either).

There's a base64 example in the UIExplorer Image example (the puzzle piece). It was working last time I checked.

Contributor

nicklockwood commented Dec 27, 2015

@geirman your code looks OK to me (as @vjeux says, isStatic isn't needed, but it shouldn't stop it from working either).

There's a base64 example in the UIExplorer Image example (the puzzle piece). It was working last time I checked.

@geirman

This comment has been minimized.

Show comment
Hide comment
@geirman

geirman Dec 27, 2015

Contributor

Thanks @nicklockwood. I found the UIExplorer Image Example you referenced and updated my rnplay example to match. I wasn't able to get it to work, but noticed something strange as I was setting it up. When I add borderWidth to the styles element, I get the No suitable image URL loader error. If I remove it, the image isn't showing, but that's probably something I'm doing wrong. I think this might be a bug.

See for yourself by taking the following steps.

  1. Go to my rnplay example
  2. with the image styles set to style={[styles.base64, styles.outline]}, notice the No suitable image url loader error
  3. now set Image styles to style={[styles.base64]}, notice there is no error (also no image, but that might be my fault)
Contributor

geirman commented Dec 27, 2015

Thanks @nicklockwood. I found the UIExplorer Image Example you referenced and updated my rnplay example to match. I wasn't able to get it to work, but noticed something strange as I was setting it up. When I add borderWidth to the styles element, I get the No suitable image URL loader error. If I remove it, the image isn't showing, but that's probably something I'm doing wrong. I think this might be a bug.

See for yourself by taking the following steps.

  1. Go to my rnplay example
  2. with the image styles set to style={[styles.base64, styles.outline]}, notice the No suitable image url loader error
  3. now set Image styles to style={[styles.base64]}, notice there is no error (also no image, but that might be my fault)
@kevando

This comment has been minimized.

Show comment
Hide comment
@kevando

kevando Dec 28, 2015

I get the same "No suitable image URL loader error. "

IT happens the Image component, but I can render the base64 icons with the TabIOS component. Anything beyond javascript is sort of a black box to me, so I'm happy to provide any of my configuration settings if that helps.

kevando commented Dec 28, 2015

I get the same "No suitable image URL loader error. "

IT happens the Image component, but I can render the base64 icons with the TabIOS component. Anything beyond javascript is sort of a black box to me, so I'm happy to provide any of my configuration settings if that helps.

@geirman

This comment has been minimized.

Show comment
Hide comment
@geirman

geirman Dec 28, 2015

Contributor

Looks like we need to reopen this issue

Contributor

geirman commented Dec 28, 2015

Looks like we need to reopen this issue

@vjeux vjeux reopened this Dec 28, 2015

@kevando

This comment has been minimized.

Show comment
Hide comment
@kevando

kevando Dec 29, 2015

Alternatively - I really just want a way to upload images from the camera roll (ios) in my app and just store the base64 encoding and not have to worry about the file path (because it breaks between app versions).

kevando commented Dec 29, 2015

Alternatively - I really just want a way to upload images from the camera roll (ios) in my app and just store the base64 encoding and not have to worry about the file path (because it breaks between app versions).

@mkonicek

This comment has been minimized.

Show comment
Hide comment
@mkonicek

mkonicek Mar 20, 2016

Contributor

Based on Nick's comment:

@geirman your code looks OK to me (as @vjeux says, isStatic isn't needed, but it shouldn't stop it from working either). There's a base64 example in the UIExplorer Image example (the puzzle piece). It was working last time I checked.

And then I see people reporting "No suitable image URL loader error." I'd say we should open a GitHub issue for this, or better a StackOverflow question, to figure out what's going on and if Base64 Images are supported now.

This pull request is almost a year old now and would have to be rebased. I suggest we close it and discuss what needs to be done first and then make a new PR if needed.

Contributor

mkonicek commented Mar 20, 2016

Based on Nick's comment:

@geirman your code looks OK to me (as @vjeux says, isStatic isn't needed, but it shouldn't stop it from working either). There's a base64 example in the UIExplorer Image example (the puzzle piece). It was working last time I checked.

And then I see people reporting "No suitable image URL loader error." I'd say we should open a GitHub issue for this, or better a StackOverflow question, to figure out what's going on and if Base64 Images are supported now.

This pull request is almost a year old now and would have to be rebased. I suggest we close it and discuss what needs to be done first and then make a new PR if needed.

@mkonicek mkonicek closed this Mar 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment