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

Automatically size images if no size is specified in the style property #192

Closed
ide opened this issue Mar 25, 2015 · 8 comments
Closed

Automatically size images if no size is specified in the style property #192

ide opened this issue Mar 25, 2015 · 8 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@ide
Copy link
Contributor

ide commented Mar 25, 2015

This is a feature request for <Image source={require('image!a.png')} /> to display the image even though there is no style property that specifies the size. For static images, the packager can compute the size of the image up front (and with a module like sharp, 2x-resolution images could be automatically generated from 3x ones when necessary).

For non-static images, I think the right thing is to resize the Image component after the image size is known, but to warn about the potentially janky user experience and extra reflow pass. So - basically like web but with warnings about suboptimal practices.

@vjeux
Copy link
Contributor

vjeux commented Mar 25, 2015

For static images, it would be nice to automatically compute the image size but it requires some infrastructure that we don't have. Need to parse all the image files, maintain a cache of their dimensions, feed it into js or the layout system...

So - basically like web but with warnings about suboptimal practices.

Warnings don't work in practice, everyone ignores them. I strongly believe that forcing people to know the size of their images, even if it's super painful, is the right way to go.

@amasad
Copy link
Contributor

amasad commented Mar 25, 2015

Need to parse all the image files, maintain a cache of their dimensions, feed it into js or the layout system

Shouldn't be too hard, we can include the image size in the image module we generate.

2x-resolution images could be automatically generated from 3x ones when necessary

When would that be necessary? should the package have device information?

@ide
Copy link
Contributor Author

ide commented Mar 25, 2015

Warnings don't work in practice, everyone ignores them. I strongly believe that forcing people to know the size of their images, even if it's super painful, is the right way to go.

It's not so much that it's painful but more of a emphasis on quick development and shipping sooner. If I can put together a couple of React components with images and can see my progress even if there are reflows, that's awesome.

For production I'm more ambivalent. It's nice on the web that you can load arbitrary images and the browser ends up displaying the right thing. That said, reflows feel way jankier on mobile (web or native) so I can see why it might not be desirable... At any rate, being able to automatically size static components would be a real win for convenience without any perf/UX tradeoffs.

@amasad: about generating 2x images: apps targeting iOS 7 need to have 2x images since the 6 Plus didn't exist back then. In iOS 8, if you only have a 3x image, the system will automatically downscale it for you. That may introduce unwanted performance overhead (though in practice it seems fine) but on the other hand, you can ship a smaller app binary by excluding the 2x assets.

As a starting point, it could be good if the project author could specify targetPlatformVersion=7.0, downscaleImageAssets=true (something like that -- this is just a hypothetical idea) in a config file like their package.json. Totally open to ideas. Asset management is a minor pain point for typical iOS development. Xcode 6 does support vector graphics but they have to be PDF files so some kind of SVG support would also put React Native a step ahead in that regard.

@vjeux
Copy link
Contributor

vjeux commented Mar 25, 2015

It's not so much that it's painful but more of a emphasis on quick development and shipping sooner.

The problem with image sizing is that it's not trivial to know from an arbitrary URL what the size is. So, if you give the developer a way to get away with not knowing the size, by paying the cost of a reflow, he will take it in a whim (my long experience in the fb codebase).

Then, once you have an entire codebase that has no idea what sizes the images it display are, but sometimes you do need to know them, you are screwed. You add all sorts of hacks, threading them down, having all sorts of heuristics to figure them out, parsing the URI attempting to recover it...

What you need is to ask the image size when you get the URI, and then always keep this metadata around. By forcing the developer to know the image size to render it, I hope that it's going to lead them to fall into the pit of success by implementing that pattern by themselves. While this is a pretty sneaky technique, this is the best I got.

At any rate, being able to automatically size static components would be a real win for convenience without any perf/UX tradeoffs.

Yes, we need to make it happen at some point in the future.

@ide
Copy link
Contributor Author

ide commented Mar 25, 2015

OK, I see where you're coming from. Maybe an opt-in approach would be better, with an <AutoImage> component that a third-party module could provide.

@vjeux
Copy link
Contributor

vjeux commented Mar 25, 2015

Sure, if you want to have an AutoImage in your codebase, on npm... be my guest :) But it's not going to be merged in core :)

@joshbedo
Copy link

The problem with image sizing is that it's not trivial to know from an arbitrary URL what the size is. So, if you give the developer a way to get away with not knowing the size, by paying the cost of a reflow, he will take it in a whim (my long experience in the fb codebase).

Then, once you have an entire codebase that has no idea what sizes the images it display are, but sometimes you do need to know them, you are screwed. You add all sorts of hacks, threading them down, having all sorts of heuristics to figure them out, parsing the URI attempting to recover it...

What you need is to ask the image size when you get the URI, and then always keep this metadata around. By forcing the developer to know the image size to render it, I hope that it's going to lead them to fall into the pit of success by implementing that pattern by themselves. While this is a pretty sneaky technique, this is the best I got.

@vjeux i also thought this was a little annoying but after reading that explanation i can totally understand where you're coming from. I've seen many people add 500x500 images and then resize them to 50x50 through the interwebs. It also encourages like you said creating design patterns.. I could have a Icon react class which sets default width and height to 50px as well as other properties and i can specify other props like rightCorner, leftCorner etc.

@brentvatne
Copy link
Collaborator

This is happening now with static images. To summarize the above: core will not include any auto-sizing of network images, but feel free to develop and publish a module to take care of this if you would like to see it 😄

harrykiselev pushed a commit to harrykiselev/react-native that referenced this issue Aug 5, 2015
@facebook facebook locked as resolved and limited conversation to collaborators May 31, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

6 participants