Skip to content

Image - support SVG from uri #1712

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 8, 2021
Merged

Image - support SVG from uri #1712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions demo/src/screens/componentScreens/ImageScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const BROKEN_URL = 'file:///Desktop/website/img/cupcake.jpg';
const DEFAULT_SIZE = 100;

const file = Assets.svgs.demo.logo;
const uri = {uri: 'http://thenewcode.com/assets/images/thumbnails/homer-simpson.svg'};
const uriWithCss = {uri: 'http://thenewcode.com/assets/svg/accessibility.svg'};
const xml = `
<svg width="32" height="32" viewBox="0 0 32 32">
<path
Expand Down Expand Up @@ -38,6 +40,13 @@ enum SizeType {
Percentage = '50%'
}

enum SvgType {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels like this enum should be exported from the 'SvgImage' component... You can also use it in the component by first determining the source type and the have a switch case to the return instead of many 'if' statements - will be way cleaner

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think this should be in the SvgImage, the user does not need to be aware of these different types, I'm not sure if it's "wrong" to put the different type here or not, perhaps I should add some tests though.
Regarding the single return, generally I agree with it, but it is a matter of taste, and I feel like it is fine the way it is currently (since the type is not coming from the component).

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's your call

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the record, I tried to add some tests, but:

  1. There is no onLoad \ onError props in the SVG components.
  2. Snapshot tests have issues as well: some random things fail them (not sure how to mock these), might need to mock timers etc.
  3. Lastly I tried to use the (old method) of snapshot testing using react-test-renderer, again it did not work for me, since the remote images need to load somehow.

File = 'file',
Uri = 'uri',
UriWithCss = 'use_with_css',
Xml = 'xml'
}

interface State {
cover: boolean;
showOverlayContent: boolean;
Expand All @@ -46,7 +55,7 @@ interface State {
margin: number;
showErrorImage: boolean;
showSvg: boolean;
isFile: boolean;
svgType: SvgType;
sizeType: SizeType;
}

Expand All @@ -59,10 +68,25 @@ class ImageScreen extends Component<{}, State> {
margin: 0,
showErrorImage: false,
showSvg: false,
isFile: false,
svgType: SvgType.File,
sizeType: SizeType.None
};

getSvgSource() {
const {svgType} = this.state;
switch (svgType) {
case SvgType.File:
return file;
case SvgType.Uri:
return uri;
case SvgType.UriWithCss:
return uriWithCss;
case SvgType.Xml:
default:
return xml;
}
}

renderOverlayContent() {
const {cover, overlayType, showOverlayContent} = this.state;
if (showOverlayContent) {
Expand Down Expand Up @@ -105,14 +129,15 @@ class ImageScreen extends Component<{}, State> {
}

renderSvgImage() {
const {isFile, sizeType} = this.state;
const {sizeType} = this.state;
const size: any = Number(sizeType) || sizeType;
const source = this.getSvgSource();
return (
<>
{size ? (
<Image source={isFile ? file : xml} width={size} height={size}/>
<Image source={source} width={size} height={size}/>
) : (
<Image source={isFile ? file : xml}/>
<Image source={source}/>
)}
</>
);
Expand All @@ -134,10 +159,9 @@ class ImageScreen extends Component<{}, State> {
}

renderSvgOptions() {
const {isFile} = this.state;
return (
<>
{renderBooleanOption.call(this, isFile ? 'Load from file' : 'Use xml const', 'isFile')}
{renderRadioGroup.call(this, 'SVG Type', 'svgType', SvgType, {isRow: true})}
{renderRadioGroup.call(this, 'Size Type', 'sizeType', SizeType, {isRow: true})}
</>
);
Expand Down
3 changes: 3 additions & 0 deletions generatedTypes/src/components/image/SvgImage.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
/// <reference types="react" />
import { ImageProps } from 'react-native';
export declare function isSvg(source: ImageProps['source']): any;
export interface SvgImageProps {
data: any;
}
declare function SvgImage(props: SvgImageProps): JSX.Element | null;
declare namespace SvgImage {
var displayName: string;
var isSvg: typeof import("./SvgImage").isSvg;
}
export default SvgImage;
16 changes: 15 additions & 1 deletion src/components/image/SvgImage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
import React from 'react';
import {ImageProps} from 'react-native';
import {SvgPackage} from '../../optionalDependencies';
const SvgXml = SvgPackage?.SvgXml;
const SvgCssUri = SvgPackage?.SvgCssUri;
// const SvgProps = SvgPackage?.SvgProps; TODO: not sure how (or if) we can use their props

function isSvgUri(source: ImageProps['source']) {
// @ts-expect-error
return typeof source === 'object' && source?.uri?.endsWith('.svg');
}

export function isSvg(source: ImageProps['source']) {
return typeof source === 'string' || typeof source === 'function' || isSvgUri(source);
}

export interface SvgImageProps {
data: any; // TODO: I thought this should be string | React.ReactNode but it doesn't work properly
}
Expand All @@ -16,7 +27,9 @@ function SvgImage(props: SvgImageProps) {
return null;
}

if (typeof data === 'string') {
if (isSvgUri(data)) {
return <SvgCssUri {...others} uri={data.uri}/>;
} else if (typeof data === 'string') {
return <SvgXml xml={data} {...others}/>;
} else if (data) {
const File = data; // Must be with capital letter
Expand All @@ -27,5 +40,6 @@ function SvgImage(props: SvgImageProps) {
}

SvgImage.displayName = 'IGNORE';
SvgImage.isSvg = isSvg;

export default SvgImage;
3 changes: 1 addition & 2 deletions src/components/image/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,7 @@ class Image extends PureComponent<Props, State> {

render() {
const {source} = this.props;
const isSvg = typeof source === 'string' || typeof source === 'function';
if (isSvg) {
if (SvgImage.isSvg(source)) {
return this.renderSvg();
} else {
return this.renderRegularImage();
Expand Down