Skip to content

Commit

Permalink
[MM-54819] Convert ./components/external_image/external_image.tsx fro…
Browse files Browse the repository at this point in the history
…m Class Component to Function Component (mattermost#24941)
  • Loading branch information
gibsonliketheguitar authored Oct 30, 2023
1 parent 7480fcf commit af8c9ae
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 144 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('ExternalImage', () => {
};

test('should render an image', () => {
const wrapper = shallow<ExternalImage>(<ExternalImage {...baseProps}/>);
const wrapper = shallow(<ExternalImage {...baseProps}/>);

expect(baseProps.children).toHaveBeenCalledWith(baseProps.src);
expect(wrapper.find('img').exists()).toBe(true);
Expand All @@ -35,7 +35,7 @@ describe('ExternalImage', () => {
imageMetadata: undefined,
};

const wrapper = shallow<ExternalImage>(<ExternalImage {...props}/>);
const wrapper = shallow(<ExternalImage {...props}/>);

expect(baseProps.children).toHaveBeenCalledWith(baseProps.src);
expect(wrapper.find('img').exists()).toBe(true);
Expand All @@ -53,7 +53,7 @@ describe('ExternalImage', () => {
src: 'https://example.com/logo.svg',
};

const wrapper = shallow<ExternalImage>(<ExternalImage {...props}/>);
const wrapper = shallow(<ExternalImage {...props}/>);

expect(props.children).toHaveBeenCalledWith(props.src);
expect(wrapper.find('img').exists()).toBe(true);
Expand All @@ -72,7 +72,7 @@ describe('ExternalImage', () => {
src: 'https://example.com/logo.svg',
};

const wrapper = shallow<ExternalImage>(<ExternalImage {...props}/>);
const wrapper = shallow(<ExternalImage {...props}/>);

expect(props.children).toHaveBeenCalledWith('');
expect(wrapper.find('img').exists()).toBe(true);
Expand All @@ -84,85 +84,9 @@ describe('ExternalImage', () => {
hasImageProxy: true,
};

const wrapper = shallow<ExternalImage>(<ExternalImage {...props}/>);
const wrapper = shallow(<ExternalImage {...props}/>);

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<ExternalImage>(<ExternalImage {...props}/>);

expect(wrapper.instance().isSVGImage()).toBe(testCase.expected);
});
}
});
});

40 changes: 13 additions & 27 deletions webapp/channels/src/components/external_image/external_image.tsx
Original file line number Diff line number Diff line change
@@ -1,43 +1,29 @@
// 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;
imageMetadata?: PostImage;
src: string;
}

export default class ExternalImage extends React.PureComponent<Props> {
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);
Original file line number Diff line number Diff line change
@@ -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);
});
}
});
13 changes: 13 additions & 0 deletions webapp/channels/src/components/external_image/is_svg_image.ts
Original file line number Diff line number Diff line change
@@ -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';
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`components/MarkdownImage should match snapshot 1`] = `
<Connect(ExternalImage)
<Connect(Component)
imageMetadata={
Object {
"format": "png",
Expand All @@ -13,7 +13,7 @@ exports[`components/MarkdownImage should match snapshot 1`] = `
src="/images/logo.png"
>
<Component />
</Connect(ExternalImage)>
</Connect(Component)>
`;

exports[`components/MarkdownImage should match snapshot for SizeAwareImage dimensions 1`] = `
Expand All @@ -40,7 +40,7 @@ exports[`components/MarkdownImage should match snapshot for SizeAwareImage dimen
`;

exports[`components/MarkdownImage should match snapshot for broken link 1`] = `
<Connect(ExternalImage)
<Connect(Component)
imageMetadata={
Object {
"format": "png",
Expand All @@ -52,7 +52,7 @@ exports[`components/MarkdownImage should match snapshot for broken link 1`] = `
src="brokenLink"
>
<Component />
</Connect(ExternalImage)>
</Connect(Component)>
`;

exports[`components/MarkdownImage should provide image src as an alt text for MarkdownImageExpand if image has no own alt text 1`] = `
Expand Down
Loading

0 comments on commit af8c9ae

Please sign in to comment.