From 646deb171e82a06ae5fd468334e067ab491ac52c Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Fri, 6 May 2022 01:24:04 +0500 Subject: [PATCH 1/8] fix: incorporated the new image params --- .../HTMLRenderers/ImageRenderer.js | 5 ++++ src/components/ThumbnailImage.js | 17 ++++++++--- .../home/report/ReportActionItemFragment.js | 29 +++++++------------ 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js index 49e2e72829d4..97577b9eaea2 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js @@ -43,6 +43,10 @@ const ImageRenderer = (props) => { Config.EXPENSIFY.EXPENSIFY_URL, Config.EXPENSIFY.URL_API_ROOT, ); + const imageSize = { + width: htmlAttribs['data-expensify-width'] ? parseInt(htmlAttribs['data-expensify-width'], 10) : undefined, + height: htmlAttribs['data-expensify-height'] ? parseInt(htmlAttribs['data-expensify-height'], 10) : undefined, + }; return ( { previewSourceURL={previewSource} style={styles.webViewStyles.tagStyles.img} isAuthTokenRequired={isAttachment} + imageSize={imageSize} /> )} diff --git a/src/components/ThumbnailImage.js b/src/components/ThumbnailImage.js index 9d15be907014..7051491a7579 100644 --- a/src/components/ThumbnailImage.js +++ b/src/components/ThumbnailImage.js @@ -19,11 +19,14 @@ const propTypes = { /** Do the urls require an authToken? */ isAuthTokenRequired: PropTypes.bool.isRequired, + imageSize: PropTypes.objectOf(PropTypes.number), + ...windowDimensionsPropTypes, }; const defaultProps = { style: {}, + imageSize: {width: 200, height: 200}, }; class ThumbnailImage extends PureComponent { @@ -31,14 +34,16 @@ class ThumbnailImage extends PureComponent { super(props); this.updateImageSize = this.updateImageSize.bind(this); - + const {width, height} = this.calculateThumbnailImageSize(props.imageSize); this.state = { - thumbnailWidth: 200, - thumbnailHeight: 200, + thumbnailWidth: width || defaultProps.imageSize.width, + thumbnailHeight: height || defaultProps.imageSize.height, }; } - updateImageSize({width, height}) { + calculateThumbnailImageSize({width, height}) { + if (!width || !height) { return {}; } + // Width of the thumbnail works better as a constant than it does // a percentage of the screen width since it is relative to each screen // Note: Clamp minimum width 40px to support touch device @@ -54,7 +59,11 @@ class ThumbnailImage extends PureComponent { } else { thumbnailScreenHeight = Math.round(thumbnailScreenWidth * aspectRatio); } + return {width: thumbnailScreenWidth, height: thumbnailScreenHeight}; + } + updateImageSize({width, height}) { + const {width: thumbnailScreenWidth, height: thumbnailScreenHeight} = this.calculateThumbnailImageSize({width, height}); this.setState({thumbnailWidth: thumbnailScreenWidth, thumbnailHeight: Math.max(40, thumbnailScreenHeight)}); } diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 194a865636f7..fac432c74a23 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -1,5 +1,5 @@ import React, {memo} from 'react'; -import {ActivityIndicator, ImageBackground, View} from 'react-native'; +import {ActivityIndicator, View} from 'react-native'; import PropTypes from 'prop-types'; import Str from 'expensify-common/lib/str'; import reportActionFragmentPropTypes from './reportActionFragmentPropTypes'; @@ -68,32 +68,22 @@ const defaultProps = { const ReportActionItemFragment = (props) => { switch (props.fragment.type) { - case 'COMMENT': + case 'COMMENT': { // If this is an attachment placeholder, return the placeholder component if (props.isAttachment && props.loading) { return ( - - {Str.isImage(props.attachmentInfo.name) - ? ( - - - - ) : ( + Str.isImage(props.attachmentInfo.name) + ? ( + `} /> + ) : ( + - )} - + + ) ); } @@ -119,6 +109,7 @@ const ReportActionItemFragment = (props) => { )} ); + } case 'TEXT': return ( From 8a950850652e8965ce2084d6d530620676263734 Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sat, 7 May 2022 01:38:38 +0500 Subject: [PATCH 2/8] fix: default props problem --- .../HTMLRenderers/ImageRenderer.js | 10 +++++----- src/components/ThumbnailImage.js | 13 ++++++++----- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js index 97577b9eaea2..650e387a436b 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js @@ -43,10 +43,9 @@ const ImageRenderer = (props) => { Config.EXPENSIFY.EXPENSIFY_URL, Config.EXPENSIFY.URL_API_ROOT, ); - const imageSize = { - width: htmlAttribs['data-expensify-width'] ? parseInt(htmlAttribs['data-expensify-width'], 10) : undefined, - height: htmlAttribs['data-expensify-height'] ? parseInt(htmlAttribs['data-expensify-height'], 10) : undefined, - }; + + const imageWidth = htmlAttribs['data-expensify-width'] ? parseInt(htmlAttribs['data-expensify-width'], 10) : undefined; + const imageHeight = htmlAttribs['data-expensify-height'] ? parseInt(htmlAttribs['data-expensify-height'], 10) : undefined; return ( { previewSourceURL={previewSource} style={styles.webViewStyles.tagStyles.img} isAuthTokenRequired={isAttachment} - imageSize={imageSize} + imageWidth={imageWidth} + imageHeight={imageHeight} /> )} diff --git a/src/components/ThumbnailImage.js b/src/components/ThumbnailImage.js index 7051491a7579..4ee1417ff2b0 100644 --- a/src/components/ThumbnailImage.js +++ b/src/components/ThumbnailImage.js @@ -19,14 +19,17 @@ const propTypes = { /** Do the urls require an authToken? */ isAuthTokenRequired: PropTypes.bool.isRequired, - imageSize: PropTypes.objectOf(PropTypes.number), + /** Image size */ + imageWidth: PropTypes.number, + imageHeight: PropTypes.number, ...windowDimensionsPropTypes, }; const defaultProps = { style: {}, - imageSize: {width: 200, height: 200}, + imageWidth: 200, + imageHeight: 200, }; class ThumbnailImage extends PureComponent { @@ -34,10 +37,10 @@ class ThumbnailImage extends PureComponent { super(props); this.updateImageSize = this.updateImageSize.bind(this); - const {width, height} = this.calculateThumbnailImageSize(props.imageSize); + const {width, height} = this.calculateThumbnailImageSize({width: props.imageWidth, height: props.imageHeight}); this.state = { - thumbnailWidth: width || defaultProps.imageSize.width, - thumbnailHeight: height || defaultProps.imageSize.height, + thumbnailWidth: width, + thumbnailHeight: height, }; } From 50e38f7447f9cd13b3b624d170eefdbe7b0fe2ad Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Wed, 11 May 2022 02:22:43 +0500 Subject: [PATCH 3/8] fix: more comments --- .../HTMLEngineProvider/HTMLRenderers/ImageRenderer.js | 1 + src/components/ThumbnailImage.js | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js index 650e387a436b..de7aafa20c03 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js @@ -49,6 +49,7 @@ const ImageRenderer = (props) => { return ( Date: Wed, 11 May 2022 02:24:17 +0500 Subject: [PATCH 4/8] fix: removed stale code --- src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js index de7aafa20c03..650e387a436b 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js @@ -49,7 +49,6 @@ const ImageRenderer = (props) => { return ( Date: Fri, 20 May 2022 21:51:39 +0500 Subject: [PATCH 5/8] fix: handled pr comments --- src/components/ThumbnailImage.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/components/ThumbnailImage.js b/src/components/ThumbnailImage.js index 492a20e3c9b0..ff24dedb2053 100644 --- a/src/components/ThumbnailImage.js +++ b/src/components/ThumbnailImage.js @@ -44,6 +44,14 @@ class ThumbnailImage extends PureComponent { }; } + /** + * Compute the thumbnails width and height given original image dimensions. + * + * @param {Number} width - Width of the original image. + * @param {Number} height - Height of the original image. + * @returns {Object} - Object containing thumbnails width and height. + */ + calculateThumbnailImageSize(width, height) { if (!width || !height) { return {}; } @@ -66,8 +74,8 @@ class ThumbnailImage extends PureComponent { } updateImageSize({width, height}) { - const {width: thumbnailScreenWidth, height: thumbnailScreenHeight} = this.calculateThumbnailImageSize(width, height); - this.setState({thumbnailWidth: thumbnailScreenWidth, thumbnailHeight: Math.max(40, thumbnailScreenHeight)}); + const {width: thumbnailWidth, height: thumbnailScreenHeight} = this.calculateThumbnailImageSize(width, height); + this.setState({thumbnailWidth, thumbnailHeight: Math.max(40, thumbnailScreenHeight)}); } render() { From 268d43a78aeedfc8d7054362f915423dd614618c Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sat, 28 May 2022 00:38:23 +0500 Subject: [PATCH 6/8] Refactored Thumbnail Image --- src/components/ThumbnailImage.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/ThumbnailImage.js b/src/components/ThumbnailImage.js index ff24dedb2053..a0bb26269ced 100644 --- a/src/components/ThumbnailImage.js +++ b/src/components/ThumbnailImage.js @@ -19,8 +19,10 @@ const propTypes = { /** Do the urls require an authToken? */ isAuthTokenRequired: PropTypes.bool.isRequired, - /** Image size */ + /** Width of the thumbnail image */ imageWidth: PropTypes.number, + + /** Height of the thumbnail image */ imageHeight: PropTypes.number, ...windowDimensionsPropTypes, @@ -37,10 +39,10 @@ class ThumbnailImage extends PureComponent { super(props); this.updateImageSize = this.updateImageSize.bind(this); - const {width, height} = this.calculateThumbnailImageSize(props.imageWidth, props.imageHeight); + const {thumbnailWidth, thumbnailHeight} = this.calculateThumbnailImageSize(props.imageWidth, props.imageHeight); this.state = { - thumbnailWidth: width, - thumbnailHeight: height, + thumbnailWidth, + thumbnailHeight, }; } @@ -70,12 +72,12 @@ class ThumbnailImage extends PureComponent { } else { thumbnailScreenHeight = Math.round(thumbnailScreenWidth * aspectRatio); } - return {width: thumbnailScreenWidth, height: thumbnailScreenHeight}; + return {thumbnailWidth: thumbnailScreenWidth, thumbnailHeight: Math.max(40, thumbnailScreenHeight)}; } updateImageSize({width, height}) { - const {width: thumbnailWidth, height: thumbnailScreenHeight} = this.calculateThumbnailImageSize(width, height); - this.setState({thumbnailWidth, thumbnailHeight: Math.max(40, thumbnailScreenHeight)}); + const {thumbnailWidth, thumbnailHeight} = this.calculateThumbnailImageSize(width, height); + this.setState({thumbnailWidth, thumbnailHeight}); } render() { From 88bd693034c2d345596a9b055ae19c27a3357dab Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sat, 4 Jun 2022 00:52:45 +0500 Subject: [PATCH 7/8] Handled comments related to documentation --- src/components/ThumbnailImage.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/ThumbnailImage.js b/src/components/ThumbnailImage.js index a0bb26269ced..72b40d584893 100644 --- a/src/components/ThumbnailImage.js +++ b/src/components/ThumbnailImage.js @@ -53,9 +53,10 @@ class ThumbnailImage extends PureComponent { * @param {Number} height - Height of the original image. * @returns {Object} - Object containing thumbnails width and height. */ - calculateThumbnailImageSize(width, height) { - if (!width || !height) { return {}; } + if (!width || !height) { + return {}; + } // Width of the thumbnail works better as a constant than it does // a percentage of the screen width since it is relative to each screen @@ -75,6 +76,12 @@ class ThumbnailImage extends PureComponent { return {thumbnailWidth: thumbnailScreenWidth, thumbnailHeight: Math.max(40, thumbnailScreenHeight)}; } + /** + * Update the state with the computed thumbnail sizes. + * + * @param {{ width: number, height: number }} Params - width and height of the original image. + * @returns {void} + */ updateImageSize({width, height}) { const {thumbnailWidth, thumbnailHeight} = this.calculateThumbnailImageSize(width, height); this.setState({thumbnailWidth, thumbnailHeight}); From 421f084784a222c63c81320b0584ef1f98f75e27 Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sat, 4 Jun 2022 01:34:05 +0500 Subject: [PATCH 8/8] Removed return type for updateImageSize fn --- src/components/ThumbnailImage.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/ThumbnailImage.js b/src/components/ThumbnailImage.js index 72b40d584893..3088a13a425b 100644 --- a/src/components/ThumbnailImage.js +++ b/src/components/ThumbnailImage.js @@ -80,7 +80,6 @@ class ThumbnailImage extends PureComponent { * Update the state with the computed thumbnail sizes. * * @param {{ width: number, height: number }} Params - width and height of the original image. - * @returns {void} */ updateImageSize({width, height}) { const {thumbnailWidth, thumbnailHeight} = this.calculateThumbnailImageSize(width, height);