seekshiva / react-native-remote-svg

Adds support for loading svg images in React Native
MIT License
184 stars 71 forks source link

Latest v2.0.0 issue related to WebView scalesPageToFit and default useWebkit={true} property #38

Closed oleksandr-dziuban closed 5 years ago

oleksandr-dziuban commented 5 years ago

Hello, I have found an issue after upgrade to v2.0.0 that we can't use "react-native-remote-svg" anymore because of WebView update for scalesPageToFit and default useWebkit={true} property.

Latest "react-native-webview" package used internally in "react-native-remote-svg" uses useWebkit={true} prop as default, and when we scalesPageToFit={true} it shows the warning that we can't scale svg image to fit when useWebkit is true.

As result we can see a bit broken SVG image, because it is not scaled to fit the WebView. And we can't pass useWebkit or scalesPageToFit to internal WebView to disable Weblit View on iOS for example.

The best solution is to add scalesPageToFit and useWebkit props to remote svg Image component and pass them to WebView internally. Fo example if we pass:

<Image 
  useWebkit={false} 
  scalesPageToFit={true} 
  source={{ uri: 'data:image/svg+xml;utf8,...' }}
/>

then scaling will work correctly on iOS platform.

I use: react-native: 0.59.1 react: 16.8.3 react-native-remote-svg: 2.0.0

MacOS: 10.14.3 Xcode: 10.1

isaac-ivins commented 5 years ago

Ran into the same issue. The svgs come through but wont adjust in size

oleksandr-dziuban commented 5 years ago

Hi @IsaacWIvins, we just need to add 2 additional properties for SVG Image component and pass them to WebView: scalesPageToFit and useWebkit. I think it will be useful for a lot of use cases.

isaac-ivins commented 5 years ago

Yeah adding useWebKit={false} and scalesPageToFit={true} to both SVG Image component and the WebView on SvgImage.js does the trick for me

oleksandr-dziuban commented 5 years ago

@IsaacWIvins Yep, I can prepare PR to implement this fix. Just need a green light from repository holder

seekshiva commented 5 years ago

we just need to add 2 additional properties for SVG Image component and pass them to WebView: scalesPageToFit and useWebkit

@oleksandr-dziuban Can't we pass useWebkit={false} scalesPageToFit={true} directly to WebView? Why should the SvgImage component take it in as props?

PR welcome for this change!

oleksandr-dziuban commented 5 years ago

we just need to add 2 additional properties for SVG Image component and pass them to WebView: scalesPageToFit and useWebkit

@oleksandr-dziuban Can't we pass useWebkit={false} scalesPageToFit={true} directly to WebView? Why should the SvgImage component take it in as props?

PR welcome for this change!

Hello @seekshiva , yes, sure, we can, but if someone wants to control these 2 props on <SvgImage /> level? For example someone still needs useWebkit={true} on iOS or scalesPageToFit={false} to disable scale to fit option? It will be just a bit more flexible for end user, I think.

What do you think?

If you don't want these 2 props on <SvgImage /> we could just add new property, for example

<SvgImage
  scaleToFit={true/false}
/>

If someone wants to enable scaleToFit he will pass scaleToFit={true} and we pass scaleToFit={true} + useWebkit={false} to WebView to enable scaling. If someone want to disable scaling, just pass useWebkit={false} and we pass scaleToFit={false} + default useWebkit={true} will be still enabled in WebView.

oleksandr-dziuban commented 5 years ago

@seekshiva Hello, I can't create PR, because don't have required access rights to push my branch. My change is just adding useWebkit={false} to <WebView>.

Could you please add this change and publish new version on npm?

seekshiva commented 5 years ago

Is there any usecase where a developer may want an image to not fit in the container they want? Where they load an image in an app and the image could appear on screen in any size?

I'm trying to think if there are any legitimate use cases where a developer may want an image to not scaleToFit within the container they want.

seekshiva commented 5 years ago

If you click on the "edit" button over here, github would create a fork of this repo in your name, and you would be able to edit in your fork and send a PR to this project.

Screenshot 2019-03-28 at 11 40 48 AM
oleksandr-dziuban commented 5 years ago

Is there any usecase where a developer may want an image to not fit in the container they want? Where they load an image in an app and the image could appear on screen in any size?

I'm trying to think if there are any legitimate use cases where a developer may want an image to not scaleToFit within the container they want.

It's just a case, of course, maybe we don't need this thing.

oleksandr-dziuban commented 5 years ago

@seekshiva Pull request https://github.com/seekshiva/react-native-remote-svg/pull/40 created, please check and publish new version if it's OK.

Thank you

oleksandr-dziuban commented 5 years ago

Hi @seekshiva , I have added incorrect useWebkit prop. Found in docs it should be useWebKit. I created another PR https://github.com/seekshiva/react-native-remote-svg/pull/41.

Please check it and publish new npm packages. Thanks

seekshiva commented 5 years ago

v2.0.3 is out now

oleksandr-dziuban commented 5 years ago

@seekshiva Thanks!