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

feat: add support for SVG images in ImageZoom component #756

Merged
merged 2 commits into from
Mar 29, 2018

Conversation

machour
Copy link
Member

@machour machour commented Mar 29, 2018

Question Response
Version? v1.4.1
Devices tested? iPhone 7, OnePlus 5t
Bug fix? no
New feature? yes
Includes tests? no
All Tests pass? yes
Related ticket? #584

Screenshots

screen shot 2018-03-29 at 6 00 20 pm

Description

Added support for remote SVG images in the ImageZoom component.

The idea is to still use the <Image> component from React Native, and onError, let react-native-svg-from-uri take a chance at rendering.

(I forked react-native-svg-from-uri from react-native-svg-uri which is not maintained anymore, and fixed some text rendering issues that made me walk away when I first tried this lib a while ago.)

SVG are not zoomable for now, as react-native-photo-view doesn't support SVG. No biggies IMHO, cause most of the times, SVG images in markdown comments are badges.

This needs to be tested on more badges / issues 🖐

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.009%) to 43.391% when pulling fd66587 on machour:feat/svg-support into 21af8b7 on gitpoint:master.

Copy link
Member

@housseindjirdeh housseindjirdeh left a comment

Choose a reason for hiding this comment

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

Thank you mate <3

Agreed, I'm not picky with it not being zoomable. I think it's perfectly fine the way it is.

@housseindjirdeh housseindjirdeh merged commit e624c54 into gitpoint:master Mar 29, 2018
@machour machour deleted the feat/svg-support branch March 29, 2018 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants