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

Commit

Permalink
MM-14500 Fix for pop caused by size_aware_image
Browse files Browse the repository at this point in the history
  * Use svg for creating placeholder instead of canvas and dataUrl
    Change to inline-block for hover css shadow effect
    Add defensive check for dimensions
  • Loading branch information
sudheerDev committed Mar 8, 2019
1 parent 52ef9b9 commit 2882763
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 43 deletions.
14 changes: 11 additions & 3 deletions components/__snapshots__/size_aware_image.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,16 @@ exports[`components/SizeAwareImage should render a placeholder and has loader wh
containerClass="file__image-loading"
/>
</div>
<img
src="data:image/png;base64,abc123"
/>
<div>
<svg
style={
Object {
"verticalAlign": "middle",
}
}
viewBox="0 0 300 200"
xmlns="http://www.w3.org/2000/svg"
/>
</div>
</Fragment>
`;
37 changes: 23 additions & 14 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 @@ -121,22 +126,16 @@ export default class SizeAwareImage extends React.PureComponent {
render() {
const {
dimensions,
src,
...props
} = this.props;

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) {
if (this.state.error) {
return null;
} else {
src = this.props.src;
}

return (
Expand All @@ -148,10 +147,20 @@ export default class SizeAwareImage extends React.PureComponent {
/>
</div>
}
<img
{...props}
src={src}
/>
{dimensions && dimensions.width && !this.state.loaded ? (
<div className={this.props.className}>
<svg
xmlns='http://www.w3.org/2000/svg'
viewBox={`0 0 ${dimensions.width} ${this.props.dimensions.height}`}
style={{verticalAlign: 'middle'}}
/>
</div>
) : (
<img
{...props}
src={src}
/>
)}
</React.Fragment>
);
}
Expand Down
13 changes: 4 additions & 9 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
2 changes: 1 addition & 1 deletion sass/components/_files.scss
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
@include border-radius(4px);
bottom: 0;
cursor: pointer;
display: inline-flex;
display: inline-block;
min-height: 40px;
min-width: 40px;
max-height: 350px;
Expand Down
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

0 comments on commit 2882763

Please sign in to comment.