Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-14500 Fix for pop caused by size_aware_image #2469

Merged
merged 3 commits into from
Mar 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 18 additions & 3 deletions components/__snapshots__/size_aware_image.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,31 @@ exports[`components/SizeAwareImage should render a placeholder and has loader wh
<div
style={
Object {
"left": "50%",
"position": "absolute",
"top": "50%",
"transform": "translate(-50%, -50%)",
}
}
>
<LoadingImagePreview
containerClass="file__image-loading"
/>
</div>
<img
src="data:image/png;base64,abc123"
/>
<div
className="image-loading__container undefined"
>
<svg
style={
Object {
"maxHeight": "200",
"maxWidth": "300",
"verticalAlign": "middle",
}
}
viewBox="0 0 300 200"
xmlns="http://www.w3.org/2000/svg"
/>
</div>
</Fragment>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export default class PostBodyAdditionalContent extends React.PureComponent {
this.generateToggleableEmbed = this.generateToggleableEmbed.bind(this);
this.generateStaticEmbed = this.generateStaticEmbed.bind(this);
this.isLinkToggleable = this.isLinkToggleable.bind(this);
this.handleLinkLoadError = this.handleLinkLoadError.bind(this);
this.handleLinkLoaded = this.handleLinkLoaded.bind(this);

this.state = {
Expand Down Expand Up @@ -150,10 +149,6 @@ export default class PostBodyAdditionalContent extends React.PureComponent {
image.onload = () => {
this.handleLinkLoaded();
};

image.onerror = () => {
this.handleLinkLoadError();
};
}
}

Expand Down Expand Up @@ -191,14 +186,6 @@ export default class PostBodyAdditionalContent extends React.PureComponent {
return false;
}

handleLinkLoadError() {
if (this.mounted) {
this.setState({
linkLoadError: true,
});
}
}

handleLinkLoaded() {
if (this.mounted) {
this.setState({
Expand Down Expand Up @@ -236,7 +223,6 @@ export default class PostBodyAdditionalContent extends React.PureComponent {
<PostImage
channelId={this.props.post.channel_id}
link={link}
onLinkLoadError={this.handleLinkLoadError}
onLinkLoaded={this.handleLinkLoaded}
handleImageClick={this.handleImageClick}
dimensions={dimensions}
Expand Down
16 changes: 0 additions & 16 deletions components/post_view/post_image/post_image.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ export default class PostImage extends React.PureComponent {
*/
onLinkLoaded: PropTypes.func,

/**
* The function to call if image load fails
*/
onLinkLoadError: PropTypes.func,

/**
* The function to call if image is clicked
*/
Expand Down Expand Up @@ -63,16 +58,6 @@ export default class PostImage extends React.PureComponent {
}
}

handleLoadError = () => {
if (!this.mounted) {
return;
}

if (this.props.onLinkLoadError) {
this.props.onLinkLoadError();
}
}

onImageClick = (e) => {
e.preventDefault();
this.props.handleImageClick();
Expand All @@ -89,7 +74,6 @@ export default class PostImage extends React.PureComponent {
dimensions={this.props.dimensions}
showLoader={true}
onImageLoaded={this.handleLoadComplete}
onImageLoadFail={this.handleLoadError}
onClick={this.onImageClick}
/>
</div>
Expand Down
67 changes: 43 additions & 24 deletions components/size_aware_image.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import PropTypes from 'prop-types';
import React from 'react';

import LoadingImagePreview from 'components/loading_image_preview';
import {createPlaceholderImage, loadImage} from 'utils/image_utils';
import {loadImage} from 'utils/image_utils';

const WAIT_FOR_HEIGHT_TIMEOUT = 100;

Expand Down Expand Up @@ -38,6 +38,11 @@ export default class SizeAwareImage extends React.PureComponent {
* A callback that is called when image load fails
*/
onImageLoadFail: PropTypes.func,

/*
* css classes that can added to the img as well as parent div on svg for placeholder
*/
className: PropTypes.string,
}

constructor(props) {
Expand Down Expand Up @@ -69,7 +74,7 @@ export default class SizeAwareImage extends React.PureComponent {
loadImage = () => {
const image = loadImage(this.props.src, this.handleLoad);

image.onerror = this.handleError();
image.onerror = this.handleError;

if (!this.props.dimensions) {
this.waitForHeight(image);
Expand Down Expand Up @@ -118,40 +123,54 @@ export default class SizeAwareImage extends React.PureComponent {
}
};

render() {
renderImageLoaderIfNeeded = () => {
if (!this.state.loaded && this.props.showLoader && !this.state.error) {
return (
<div style={{position: 'absolute', top: '50%', transform: 'translate(-50%, -50%)', left: '50%'}}>
<LoadingImagePreview
containerClass={'file__image-loading'}
/>
</div>
);
}
return null;
}

renderImageOrPlaceholder = () => {
const {
dimensions,
src,
...props
} = this.props;

if (dimensions && dimensions.width && !this.state.loaded) {
return (
<div className={`image-loading__container ${this.props.className}`}>
<svg
xmlns='http://www.w3.org/2000/svg'
viewBox={`0 0 ${dimensions.width} ${dimensions.height}`}
style={{verticalAlign: 'middle', maxHeight: `${dimensions.height}`, maxWidth: `${dimensions.width}`}}
/>
</div>
);
}
Reflect.deleteProperty(props, 'showLoader');
Reflect.deleteProperty(props, 'onImageLoaded');
Reflect.deleteProperty(props, 'onImageLoadFail');

let src;
if (!this.state.loaded && dimensions) {
// Generate a blank image as a placeholder because that will scale down to fit the available space
// while maintaining the correct aspect ratio
src = createPlaceholderImage(dimensions.width, dimensions.height);
} else if (this.state.error) {
return null;
} else {
src = this.props.src;
}
return (
<img
{...props}
src={src}
/>
);
}

render() {
return (
<React.Fragment>
{!this.state.loaded && this.props.showLoader &&
<div style={{position: 'absolute'}}>
<LoadingImagePreview
containerClass={'file__image-loading'}
/>
</div>
}
<img
{...props}
src={src}
/>
{this.renderImageLoaderIfNeeded()}
{this.renderImageOrPlaceholder()}
</React.Fragment>
);
}
Expand Down
20 changes: 9 additions & 11 deletions components/size_aware_image.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import LoadingImagePreview from 'components/loading_image_preview';

jest.mock('utils/image_utils');

import {createPlaceholderImage, loadImage} from 'utils/image_utils';
import {loadImage} from 'utils/image_utils';

describe('components/SizeAwareImage', () => {
const baseProps = {
Expand All @@ -23,26 +23,21 @@ describe('components/SizeAwareImage', () => {

loadImage.mockReturnValue(() => ({}));

test('should render a placeholder when first mounted with dimensions', () => {
createPlaceholderImage.mockImplementation(() => 'data:image/png;base64,abc123');

test('should render an svg when first mounted with dimensions', () => {
const wrapper = mount(<SizeAwareImage {...baseProps}/>);

const src = wrapper.find('img').prop('src');
expect(src.startsWith('data:image')).toBeTruthy();
const viewBox = wrapper.find('svg').prop('viewBox');
expect(viewBox).toEqual('0 0 300 200');
});

test('should render a placeholder and has loader when showLoader is true', () => {
const placeholder = 'data:image/png;base64,abc123';
createPlaceholderImage.mockImplementation(() => placeholder);
const props = {
...baseProps,
showLoader: true,
};

const wrapper = shallow(<SizeAwareImage {...props}/>);
expect(wrapper.find(LoadingImagePreview).exists()).toEqual(true);
expect(wrapper.find('img').prop('src')).toEqual(placeholder);
expect(wrapper).toMatchSnapshot();
});

Expand Down Expand Up @@ -94,15 +89,18 @@ describe('components/SizeAwareImage', () => {
expect(baseProps.onImageLoaded).toHaveBeenCalledWith({height, width});
});

test('should call onImageLoadFail when image load fails and should render empty/null', () => {
test('should call onImageLoadFail when image load fails and should have svg', () => {
const onImageLoadFail = jest.fn();
const props = {...baseProps, onImageLoadFail};

const wrapper = shallow(<SizeAwareImage {...props}/>);
const wrapper = mount(<SizeAwareImage {...props}/>);
wrapper.setState({loaded: false});
wrapper.instance().handleError();
expect(props.onImageLoadFail).toHaveBeenCalled();

expect(wrapper.state('error')).toBe(true);
expect(wrapper.find('img').exists()).toEqual(false);
expect(wrapper.find('svg').exists()).toEqual(true);
expect(wrapper.find(LoadingImagePreview).exists()).toEqual(false);
});
});
28 changes: 16 additions & 12 deletions sass/components/_files.scss
Original file line number Diff line number Diff line change
Expand Up @@ -189,25 +189,20 @@
@include border-radius(4px);
bottom: 0;
cursor: pointer;
display: inline-flex;
min-height: 40px;
min-width: 40px;
max-height: 350px;
max-width: 480px;
overflow: hidden;
position: relative;
z-index: 2;

&:hover {
@include box-shadow(0 2px 5px 0 rgba($black, 0.1), 0 2px 10px 0 rgba($black, 0.1));
}

max-width: 100%;
img {
@include single-transition(all, .1s, linear);
align-self: center;
max-height: 350px;
max-width: 480px;

overflow: hidden;
position: relative;
z-index: 2;
&:hover {
@include box-shadow(0 2px 5px 0 rgba($black, 0.1), 0 2px 10px 0 rgba($black, 0.1));
}
&.min-preview {
max-height: 50px;
max-width: 100%;
Expand All @@ -221,8 +216,12 @@
&.svg {
width: 100%;
height: inherit;
display: block;
img {
height: inherit;
&:hover {
@include box-shadow(0 2px 5px 0 rgba($black, 0.1), 0 2px 10px 0 rgba($black, 0.1));
}
}
}
}
Expand Down Expand Up @@ -502,3 +501,8 @@
opacity: .6;
}
}

.image-loading__container {
max-height: inherit;
overflow: hidden;
}
16 changes: 0 additions & 16 deletions utils/image_utils.jsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,6 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

// createPlaceholderImage returns a data URI for a blank image with the given dimensions.
export function createPlaceholderImage(width, height) {
const placeholder = document.createElement('canvas');

placeholder.width = width;
placeholder.height = height;
placeholder.style.backgroundColor = 'transparent';

try {
return placeholder.toDataURL();
} catch (e) {
// Canvas.toDataURL throws an error if the dimensions are too large
return '';
}
}

// loadImage loads an image in the background without rendering it or using an XMLHttpRequest.
export function loadImage(src, onLoad) {
const image = new Image();
Expand Down