From af8c9ae0caf51fafd83dea20e613c778cea7e3fb Mon Sep 17 00:00:00 2001 From: Gibson Han <66423127+gibsonliketheguitar@users.noreply.github.com> Date: Mon, 30 Oct 2023 09:43:06 -0700 Subject: [PATCH] [MM-54819] Convert ./components/external_image/external_image.tsx from Class Component to Function Component (#24941) --- .../external_image/external_image.test.tsx | 86 ++----------------- .../external_image/external_image.tsx | 40 +++------ .../external_image/is_svg_image.test.tsx | 73 ++++++++++++++++ .../components/external_image/is_svg_image.ts | 13 +++ .../markdown_image.test.tsx.snap | 8 +- .../message_attachment.test.tsx.snap | 44 +++++----- .../post_attachment_opengraph.test.tsx.snap | 12 +-- .../__snapshots__/youtube_video.test.tsx.snap | 8 +- 8 files changed, 140 insertions(+), 144 deletions(-) create mode 100644 webapp/channels/src/components/external_image/is_svg_image.test.tsx create mode 100644 webapp/channels/src/components/external_image/is_svg_image.ts diff --git a/webapp/channels/src/components/external_image/external_image.test.tsx b/webapp/channels/src/components/external_image/external_image.test.tsx index 50a0283251..5448e0ca27 100644 --- a/webapp/channels/src/components/external_image/external_image.test.tsx +++ b/webapp/channels/src/components/external_image/external_image.test.tsx @@ -23,7 +23,7 @@ describe('ExternalImage', () => { }; test('should render an image', () => { - const wrapper = shallow(); + const wrapper = shallow(); expect(baseProps.children).toHaveBeenCalledWith(baseProps.src); expect(wrapper.find('img').exists()).toBe(true); @@ -35,7 +35,7 @@ describe('ExternalImage', () => { imageMetadata: undefined, }; - const wrapper = shallow(); + const wrapper = shallow(); expect(baseProps.children).toHaveBeenCalledWith(baseProps.src); expect(wrapper.find('img').exists()).toBe(true); @@ -53,7 +53,7 @@ describe('ExternalImage', () => { src: 'https://example.com/logo.svg', }; - const wrapper = shallow(); + const wrapper = shallow(); expect(props.children).toHaveBeenCalledWith(props.src); expect(wrapper.find('img').exists()).toBe(true); @@ -72,7 +72,7 @@ describe('ExternalImage', () => { src: 'https://example.com/logo.svg', }; - const wrapper = shallow(); + const wrapper = shallow(); expect(props.children).toHaveBeenCalledWith(''); expect(wrapper.find('img').exists()).toBe(true); @@ -84,85 +84,9 @@ describe('ExternalImage', () => { hasImageProxy: true, }; - const wrapper = shallow(); + const wrapper = shallow(); expect(props.children).toHaveBeenCalledWith(Client4.getBaseRoute() + '/image?url=' + encodeURIComponent(props.src)); expect(wrapper.find('img').exists()).toBe(true); }); - - describe('isSVGImage', () => { - for (const testCase of [ - { - name: 'no metadata, no extension', - src: 'https://example.com/image.png', - imageMetadata: undefined, - expected: false, - }, - { - name: 'no metadata, svg extension', - src: 'https://example.com/image.svg', - imageMetadata: undefined, - expected: true, - }, - { - name: 'no metadata, svg extension with query parameter', - src: 'https://example.com/image.svg?a=1', - imageMetadata: undefined, - expected: true, - }, - { - name: 'no metadata, svg extension with hash', - src: 'https://example.com/image.svg#abc', - imageMetadata: undefined, - expected: true, - }, - { - name: 'no metadata, proxied image', - src: 'https://mattermost.example.com/api/v4/image?url=' + encodeURIComponent('https://example.com/image.png'), - imageMetadata: undefined, - expected: false, - }, - { - name: 'no metadata, proxied svg image', - src: 'https://mattermost.example.com/api/v4/image?url=' + encodeURIComponent('https://example.com/image.svg'), - imageMetadata: undefined, - expected: true, - }, - { - name: 'with metadata, not an SVG', - src: 'https://example.com/image.png', - imageMetadata: { - format: 'png', - frameCount: 40, - width: 100, - height: 200, - }, - expected: false, - }, - { - name: 'with metadata, SVG', - src: 'https://example.com/image.svg', - imageMetadata: { - format: 'svg', - frameCount: 30, - width: 10, - height: 20, - }, - expected: true, - }, - ]) { - test(testCase.name, () => { - const props = { - ...baseProps, - src: testCase.src, - imageMetadata: testCase.imageMetadata, - }; - - const wrapper = shallow(); - - expect(wrapper.instance().isSVGImage()).toBe(testCase.expected); - }); - } - }); }); - diff --git a/webapp/channels/src/components/external_image/external_image.tsx b/webapp/channels/src/components/external_image/external_image.tsx index 8fa9bc9a03..3633226878 100644 --- a/webapp/channels/src/components/external_image/external_image.tsx +++ b/webapp/channels/src/components/external_image/external_image.tsx @@ -1,13 +1,15 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React from 'react'; +import React, {memo} from 'react'; import type {PostImage} from '@mattermost/types/posts'; import {getImageSrc} from 'utils/post_utils'; -interface Props { +import {isSVGImage} from './is_svg_image'; + +type Props = { children: (src: string) => React.ReactNode; enableSVGs: boolean; hasImageProxy: boolean; @@ -15,29 +17,13 @@ interface Props { src: string; } -export default class ExternalImage extends React.PureComponent { - isSVGImage = () => { - if (!this.props.imageMetadata) { - // Just check if the string contains an svg extension instead of if it ends with one because it avoids - // having to deal with query strings and proxied image URLs - return this.props.src.indexOf('.svg') !== -1; - } - - return this.props.imageMetadata.format === 'svg'; - }; - - shouldRenderImage = () => { - // Return true unless the image is an SVG and we have SVG rendering disabled - return this.props.enableSVGs || !this.isSVGImage(); - }; - - render() { - let src = getImageSrc(this.props.src, this.props.hasImageProxy); - - if (!this.shouldRenderImage()) { - src = ''; - } - - return this.props.children(src); +const ExternalImage = (props: Props) => { + const shouldRenderImage = props.enableSVGs || !isSVGImage(props.imageMetadata, props.src); + let src = getImageSrc(props.src, props.hasImageProxy); + if (!shouldRenderImage) { + src = ''; } -} + return (<>{props.children(src)}); +}; + +export default memo(ExternalImage); diff --git a/webapp/channels/src/components/external_image/is_svg_image.test.tsx b/webapp/channels/src/components/external_image/is_svg_image.test.tsx new file mode 100644 index 0000000000..312624bda8 --- /dev/null +++ b/webapp/channels/src/components/external_image/is_svg_image.test.tsx @@ -0,0 +1,73 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {isSVGImage} from './is_svg_image'; + +describe('ExternalIImage isSVGImage', () => { + for (const testCase of [ + { + name: 'no metadata, no extension', + src: 'https://example.com/image.png', + imageMetadata: undefined, + expected: false, + }, + { + name: 'no metadata, svg extension', + src: 'https://example.com/image.svg', + imageMetadata: undefined, + expected: true, + }, + { + name: 'no metadata, svg extension with query parameter', + src: 'https://example.com/image.svg?a=1', + imageMetadata: undefined, + expected: true, + }, + { + name: 'no metadata, svg extension with hash', + src: 'https://example.com/image.svg#abc', + imageMetadata: undefined, + expected: true, + }, + { + name: 'no metadata, proxied image', + src: 'https://mattermost.example.com/api/v4/image?url=' + encodeURIComponent('https://example.com/image.png'), + imageMetadata: undefined, + expected: false, + }, + { + name: 'no metadata, proxied svg image', + src: 'https://mattermost.example.com/api/v4/image?url=' + encodeURIComponent('https://example.com/image.svg'), + imageMetadata: undefined, + expected: true, + }, + { + name: 'with metadata, not an SVG', + src: 'https://example.com/image.png', + imageMetadata: { + format: 'png', + frameCount: 40, + width: 100, + height: 200, + }, + expected: false, + }, + { + name: 'with metadata, SVG', + src: 'https://example.com/image.svg', + imageMetadata: { + format: 'svg', + frameCount: 30, + width: 10, + height: 20, + }, + expected: true, + }, + ]) { + test(testCase.name, () => { + const {imageMetadata, src} = testCase; + + expect(isSVGImage(imageMetadata, src)).toBe(testCase.expected); + }); + } +}); diff --git a/webapp/channels/src/components/external_image/is_svg_image.ts b/webapp/channels/src/components/external_image/is_svg_image.ts new file mode 100644 index 0000000000..103df42f63 --- /dev/null +++ b/webapp/channels/src/components/external_image/is_svg_image.ts @@ -0,0 +1,13 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {PostImage} from '@mattermost/types/posts'; + +export const isSVGImage = (imageMetadata: PostImage | undefined, src: string) => { + if (!imageMetadata) { + // Just check if the string contains an svg extension instead of if it ends with one because it avoids + // having to deal with query strings and proxied image URLs + return src.indexOf('.svg') !== -1; + } + return imageMetadata.format === 'svg'; +}; diff --git a/webapp/channels/src/components/markdown_image/__snapshots__/markdown_image.test.tsx.snap b/webapp/channels/src/components/markdown_image/__snapshots__/markdown_image.test.tsx.snap index 519c46631a..8575b07951 100644 --- a/webapp/channels/src/components/markdown_image/__snapshots__/markdown_image.test.tsx.snap +++ b/webapp/channels/src/components/markdown_image/__snapshots__/markdown_image.test.tsx.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`components/MarkdownImage should match snapshot 1`] = ` - - + `; exports[`components/MarkdownImage should match snapshot for SizeAwareImage dimensions 1`] = ` @@ -40,7 +40,7 @@ exports[`components/MarkdownImage should match snapshot for SizeAwareImage dimen `; exports[`components/MarkdownImage should match snapshot for broken link 1`] = ` - - + `; exports[`components/MarkdownImage should provide image src as an alt text for MarkdownImageExpand if image has no own alt text 1`] = ` diff --git a/webapp/channels/src/components/post_view/message_attachments/message_attachment/__snapshots__/message_attachment.test.tsx.snap b/webapp/channels/src/components/post_view/message_attachments/message_attachment/__snapshots__/message_attachment.test.tsx.snap index ec0955ffd4..069f8ced2a 100644 --- a/webapp/channels/src/components/post_view/message_attachments/message_attachment/__snapshots__/message_attachment.test.tsx.snap +++ b/webapp/channels/src/components/post_view/message_attachments/message_attachment/__snapshots__/message_attachment.test.tsx.snap @@ -29,12 +29,12 @@ exports[`components/post_view/MessageAttachment should call actions.doPostAction key="attachment__author-name" location="message_attachment" > - - + - - +
- - + footer @@ -121,7 +121,7 @@ exports[`components/post_view/MessageAttachment should call actions.doPostAction
- - +
- - + - - +
- - + footer @@ -251,7 +251,7 @@ exports[`components/post_view/MessageAttachment should match snapshot 1`] = `
- - +
- - + - - +
- - +
[Function] - + } slot2={
diff --git a/webapp/channels/src/components/youtube_video/__snapshots__/youtube_video.test.tsx.snap b/webapp/channels/src/components/youtube_video/__snapshots__/youtube_video.test.tsx.snap index f1c1c5a6d9..7d836faf7a 100644 --- a/webapp/channels/src/components/youtube_video/__snapshots__/youtube_video.test.tsx.snap +++ b/webapp/channels/src/components/youtube_video/__snapshots__/youtube_video.test.tsx.snap @@ -70,10 +70,10 @@ exports[`YoutubeVideo should match init snapshot 1`] = `
- - - - + +