From e6191a1ea35b9938815f6e543e1b7073f5396436 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Thu, 4 Jul 2024 12:27:35 +0100 Subject: [PATCH 1/2] fix: add support for svg uris without viewbox --- app/components/Base/RemoteImage/index.js | 68 ++++++++++++++++--- .../Base/RemoteImage/index.test.tsx | 43 +++++++++++- 2 files changed, 102 insertions(+), 9 deletions(-) diff --git a/app/components/Base/RemoteImage/index.js b/app/components/Base/RemoteImage/index.js index 53aba49ae0d..c48487385c7 100644 --- a/app/components/Base/RemoteImage/index.js +++ b/app/components/Base/RemoteImage/index.js @@ -1,4 +1,4 @@ -import React, { useMemo, useState } from 'react'; +import React, { useEffect, useMemo, useState } from 'react'; import PropTypes from 'prop-types'; import { Image, ViewPropTypes, View, StyleSheet } from 'react-native'; import FadeIn from 'react-native-fade-in-image'; @@ -18,6 +18,49 @@ const createStyles = () => }, }); +/** + * Support svg images urls that do not have a view box + * See: https://github.com/software-mansion/react-native-svg/issues/1202#issuecomment-1891110599 + * @param {string} uri - uri to fetch + * @param {boolean} isSVG - check to see if the uri is an svg + * @returns {string | undefined} viewbox + */ +export function useSVGViewBox(uri, isSVG) { + const [viewBox, setViewBox] = useState(undefined); + + useEffect(() => { + if (!isSVG) { + return; + } + + fetch(uri) + .then((response) => response.text()) + .then((svgContent) => { + const widthMatch = svgContent.match(/width="([^"]+)"/); + const heightMatch = svgContent.match(/height="([^"]+)"/); + const viewBoxMatch = svgContent.match(/viewBox="([^"]+)"/); + + // Image already has a view box, no additional work required + if (viewBoxMatch) { + return; + } + + if (widthMatch && heightMatch) { + const width = widthMatch[1]; + const height = heightMatch[1]; + setViewBox(`0 0 ${width} ${height}`); + } + }) + .catch((error) => console.error('Error fetching SVG:', error)); + }, [isSVG, uri]); + + if (!viewBox) { + return undefined; + } + + return viewBox; +} + const RemoteImage = (props) => { const [error, setError] = useState(undefined); // Avoid using this component with animated SVG @@ -40,16 +83,19 @@ const RemoteImage = (props) => { const onError = ({ nativeEvent: { error } }) => setError(error); + const isSVG = + source && + source.uri && + source.uri.match('.svg') && + (isImageUrl || resolvedIpfsUrl); + + const viewbox = useSVGViewBox(uri, isSVG); + if (error && props.address) { return ; } - if ( - source && - source.uri && - source.uri.match('.svg') && - (isImageUrl || resolvedIpfsUrl) - ) { + if (isSVG) { const style = props.style || {}; if (source.__packager_asset && typeof style !== 'number') { if (!style.width) { @@ -66,7 +112,13 @@ const RemoteImage = (props) => { componentLabel="RemoteImage-SVG" > - + ); diff --git a/app/components/Base/RemoteImage/index.test.tsx b/app/components/Base/RemoteImage/index.test.tsx index 64e352fa413..8982acaba41 100644 --- a/app/components/Base/RemoteImage/index.test.tsx +++ b/app/components/Base/RemoteImage/index.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { shallow } from 'enzyme'; -import RemoteImage from './'; +import RemoteImage, { useSVGViewBox } from './'; +import { renderHook, waitFor } from '@testing-library/react-native'; jest.mock('react-redux', () => ({ ...jest.requireActual('react-redux'), @@ -45,3 +46,43 @@ describe('RemoteImage', () => { expect(wrapper).toMatchSnapshot(); }); }); + +describe('useSVGViewBox()', () => { + const MOCK_SVG_WITH_VIEWBOX = ``; + const MOCK_SVG_WITHOUT_VIEWBOX = ``; + + function arrangeMocks() { + const mockResponseTextFn = jest + .fn() + .mockResolvedValue(MOCK_SVG_WITHOUT_VIEWBOX); + jest + .spyOn(global, 'fetch') + .mockResolvedValue({ text: mockResponseTextFn } as unknown as Response); + + return { mockText: mockResponseTextFn }; + } + + it('should return view-box if svg if missing a view-box', async () => { + const { mockText } = arrangeMocks(); + mockText.mockResolvedValueOnce(MOCK_SVG_WITHOUT_VIEWBOX); + + const hook = renderHook(() => useSVGViewBox('URI', true)); + await waitFor(() => expect(hook.result.current).toBeDefined()); + }); + + it('should not return a view-box if svg already has view-box', async () => { + const { mockText } = arrangeMocks(); + mockText.mockResolvedValueOnce(MOCK_SVG_WITH_VIEWBOX); + + const hook = renderHook(() => useSVGViewBox('URI', true)); + await waitFor(() => expect(hook.result.current).toBeUndefined()); + }); + + it('should not make async calls if image is not an svg', async () => { + const mocks = arrangeMocks(); + const hook = renderHook(() => useSVGViewBox('URI', false)); + + await waitFor(() => expect(hook.result.current).toBeUndefined()); + expect(mocks.mockText).not.toHaveBeenCalled(); + }); +}); From f91517ead1c6ff680951057bcbf865ecae1251fb Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Thu, 4 Jul 2024 17:00:54 +0100 Subject: [PATCH 2/2] refactor: move hook into separate file organize hooks into separate file inside the /hooks folder --- app/components/Base/RemoteImage/index.js | 48 ++---------------- .../Base/RemoteImage/index.test.tsx | 43 +--------------- app/components/hooks/useSvgUriViewBox.test.ts | 42 ++++++++++++++++ app/components/hooks/useSvgUriViewBox.ts | 49 +++++++++++++++++++ 4 files changed, 95 insertions(+), 87 deletions(-) create mode 100644 app/components/hooks/useSvgUriViewBox.test.ts create mode 100644 app/components/hooks/useSvgUriViewBox.ts diff --git a/app/components/Base/RemoteImage/index.js b/app/components/Base/RemoteImage/index.js index c48487385c7..f47269f60ce 100644 --- a/app/components/Base/RemoteImage/index.js +++ b/app/components/Base/RemoteImage/index.js @@ -1,4 +1,4 @@ -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useMemo, useState } from 'react'; import PropTypes from 'prop-types'; import { Image, ViewPropTypes, View, StyleSheet } from 'react-native'; import FadeIn from 'react-native-fade-in-image'; @@ -10,6 +10,7 @@ import ComponentErrorBoundary from '../../UI/ComponentErrorBoundary'; import useIpfsGateway from '../../hooks/useIpfsGateway'; import { getFormattedIpfsUrl } from '@metamask/assets-controllers'; import Identicon from '../../UI/Identicon'; +import useSvgUriViewBox from '../../hooks/useSvgUriViewBox'; const createStyles = () => StyleSheet.create({ @@ -18,49 +19,6 @@ const createStyles = () => }, }); -/** - * Support svg images urls that do not have a view box - * See: https://github.com/software-mansion/react-native-svg/issues/1202#issuecomment-1891110599 - * @param {string} uri - uri to fetch - * @param {boolean} isSVG - check to see if the uri is an svg - * @returns {string | undefined} viewbox - */ -export function useSVGViewBox(uri, isSVG) { - const [viewBox, setViewBox] = useState(undefined); - - useEffect(() => { - if (!isSVG) { - return; - } - - fetch(uri) - .then((response) => response.text()) - .then((svgContent) => { - const widthMatch = svgContent.match(/width="([^"]+)"/); - const heightMatch = svgContent.match(/height="([^"]+)"/); - const viewBoxMatch = svgContent.match(/viewBox="([^"]+)"/); - - // Image already has a view box, no additional work required - if (viewBoxMatch) { - return; - } - - if (widthMatch && heightMatch) { - const width = widthMatch[1]; - const height = heightMatch[1]; - setViewBox(`0 0 ${width} ${height}`); - } - }) - .catch((error) => console.error('Error fetching SVG:', error)); - }, [isSVG, uri]); - - if (!viewBox) { - return undefined; - } - - return viewBox; -} - const RemoteImage = (props) => { const [error, setError] = useState(undefined); // Avoid using this component with animated SVG @@ -89,7 +47,7 @@ const RemoteImage = (props) => { source.uri.match('.svg') && (isImageUrl || resolvedIpfsUrl); - const viewbox = useSVGViewBox(uri, isSVG); + const viewbox = useSvgUriViewBox(uri, isSVG); if (error && props.address) { return ; diff --git a/app/components/Base/RemoteImage/index.test.tsx b/app/components/Base/RemoteImage/index.test.tsx index 8982acaba41..64e352fa413 100644 --- a/app/components/Base/RemoteImage/index.test.tsx +++ b/app/components/Base/RemoteImage/index.test.tsx @@ -1,7 +1,6 @@ import React from 'react'; import { shallow } from 'enzyme'; -import RemoteImage, { useSVGViewBox } from './'; -import { renderHook, waitFor } from '@testing-library/react-native'; +import RemoteImage from './'; jest.mock('react-redux', () => ({ ...jest.requireActual('react-redux'), @@ -46,43 +45,3 @@ describe('RemoteImage', () => { expect(wrapper).toMatchSnapshot(); }); }); - -describe('useSVGViewBox()', () => { - const MOCK_SVG_WITH_VIEWBOX = ``; - const MOCK_SVG_WITHOUT_VIEWBOX = ``; - - function arrangeMocks() { - const mockResponseTextFn = jest - .fn() - .mockResolvedValue(MOCK_SVG_WITHOUT_VIEWBOX); - jest - .spyOn(global, 'fetch') - .mockResolvedValue({ text: mockResponseTextFn } as unknown as Response); - - return { mockText: mockResponseTextFn }; - } - - it('should return view-box if svg if missing a view-box', async () => { - const { mockText } = arrangeMocks(); - mockText.mockResolvedValueOnce(MOCK_SVG_WITHOUT_VIEWBOX); - - const hook = renderHook(() => useSVGViewBox('URI', true)); - await waitFor(() => expect(hook.result.current).toBeDefined()); - }); - - it('should not return a view-box if svg already has view-box', async () => { - const { mockText } = arrangeMocks(); - mockText.mockResolvedValueOnce(MOCK_SVG_WITH_VIEWBOX); - - const hook = renderHook(() => useSVGViewBox('URI', true)); - await waitFor(() => expect(hook.result.current).toBeUndefined()); - }); - - it('should not make async calls if image is not an svg', async () => { - const mocks = arrangeMocks(); - const hook = renderHook(() => useSVGViewBox('URI', false)); - - await waitFor(() => expect(hook.result.current).toBeUndefined()); - expect(mocks.mockText).not.toHaveBeenCalled(); - }); -}); diff --git a/app/components/hooks/useSvgUriViewBox.test.ts b/app/components/hooks/useSvgUriViewBox.test.ts new file mode 100644 index 00000000000..30bf04a88f8 --- /dev/null +++ b/app/components/hooks/useSvgUriViewBox.test.ts @@ -0,0 +1,42 @@ +import { renderHook, waitFor } from '@testing-library/react-native'; +import useSvgUriViewBox from './useSvgUriViewBox'; + +describe('useSvgUriViewBox()', () => { + const MOCK_SVG_WITH_VIEWBOX = ``; + const MOCK_SVG_WITHOUT_VIEWBOX = ``; + + function arrangeMocks() { + const mockResponseTextFn = jest + .fn() + .mockResolvedValue(MOCK_SVG_WITHOUT_VIEWBOX); + jest + .spyOn(global, 'fetch') + .mockResolvedValue({ text: mockResponseTextFn } as unknown as Response); + + return { mockText: mockResponseTextFn }; + } + + it('should return view-box if svg if missing a view-box', async () => { + const { mockText } = arrangeMocks(); + mockText.mockResolvedValueOnce(MOCK_SVG_WITHOUT_VIEWBOX); + + const hook = renderHook(() => useSvgUriViewBox('URI', true)); + await waitFor(() => expect(hook.result.current).toBeDefined()); + }); + + it('should return view-box if svg already has view-box', async () => { + const { mockText } = arrangeMocks(); + mockText.mockResolvedValueOnce(MOCK_SVG_WITH_VIEWBOX); + + const hook = renderHook(() => useSvgUriViewBox('URI', true)); + await waitFor(() => expect(hook.result.current).toBeDefined()); + }); + + it('should not make async calls if image is not an svg', async () => { + const mocks = arrangeMocks(); + const hook = renderHook(() => useSvgUriViewBox('URI', false)); + + await waitFor(() => expect(hook.result.current).toBeUndefined()); + expect(mocks.mockText).not.toHaveBeenCalled(); + }); +}); diff --git a/app/components/hooks/useSvgUriViewBox.ts b/app/components/hooks/useSvgUriViewBox.ts new file mode 100644 index 00000000000..bbfb1ee2bc3 --- /dev/null +++ b/app/components/hooks/useSvgUriViewBox.ts @@ -0,0 +1,49 @@ +import { useEffect, useState } from 'react'; + +/** + * Support svg images urls that do not have a view box + * See: https://github.com/software-mansion/react-native-svg/issues/1202#issuecomment-1891110599 + * + * This will return the default SVG ViewBox from an SVG URI + * @param uri - uri to fetch + * @param isSVG - check to see if the uri is an svg + * @returns viewbox string + */ +export default function useSvgUriViewBox( + uri: string, + isSVG: boolean, +): string | undefined { + const [viewBox, setViewBox] = useState(undefined); + + useEffect(() => { + if (!isSVG) { + return; + } + + fetch(uri) + .then((response) => response.text()) + .then((svgContent) => { + const widthMatch = svgContent.match(/width="([^"]+)"/); + const heightMatch = svgContent.match(/height="([^"]+)"/); + const viewBoxMatch = svgContent.match(/viewBox="([^"]+)"/); + + if (viewBoxMatch?.[1]) { + setViewBox(viewBoxMatch[1]); + return; + } + + if (widthMatch?.[1] && heightMatch?.[1]) { + const width = widthMatch[1]; + const height = heightMatch[1]; + setViewBox(`0 0 ${width} ${height}`); + } + }) + .catch((error) => console.error('Error fetching SVG:', error)); + }, [isSVG, uri]); + + if (!viewBox) { + return undefined; + } + + return viewBox; +}