Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: 16528 upload gif image fluctuate #17454

Merged
3 changes: 3 additions & 0 deletions src/components/AttachmentModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ class AttachmentModal extends PureComponent {
modalType: CONST.MODAL.MODAL_TYPE.CENTERED_UNSWIPEABLE,
isConfirmButtonDisabled: false,
confirmButtonFadeAnimation: new Animated.Value(1),
file: props.originalFileName && {
name: props.originalFileName,
},
};

this.submitAndClose = this.submitAndClose.bind(this);
Expand Down
8 changes: 4 additions & 4 deletions src/components/ImageView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ImageView extends PureComponent {
this.trackPointerPosition = this.trackPointerPosition.bind(this);

this.state = {
isLoading: false,
isLoading: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to change this line because it can improve the performance a bit. isLoading is always set to true then false when image is loaded.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree, it should be set to true initially

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ready for testing?

containerHeight: 0,
containerWidth: 0,
isZoomed: false,
Expand Down Expand Up @@ -275,7 +275,7 @@ class ImageView extends PureComponent {
<Pressable
style={{
...StyleUtils.getZoomSizingStyle(this.state.isZoomed, this.state.imgWidth, this.state.imgHeight, this.state.zoomScale,
this.state.containerHeight, this.state.containerWidth),
this.state.containerHeight, this.state.containerWidth, this.state.isLoading),
...StyleUtils.getZoomCursorStyle(this.state.isZoomed, this.state.isDragging),
...this.state.isZoomed && this.state.zoomScale >= 1 ? styles.pRelative : styles.pAbsolute,
...styles.flex1,
Expand All @@ -285,10 +285,10 @@ class ImageView extends PureComponent {
>
<Image
source={{uri: this.props.url}}
style={this.state.isLoading ? undefined : [
style={[
styles.h100,
styles.w100,
]} // Hide image until finished loading to prevent showing preview with wrong dimensions.
]}
resizeMode={Image.resizeMode.contain}
onLoadStart={this.imageLoadingStart}
onLoad={this.imageLoad}
Expand Down
2 changes: 2 additions & 0 deletions src/libs/actions/PersonalDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,13 @@ function updateAvatar(file) {
[currentUserEmail]: {
avatar: file.uri,
avatarThumbnail: file.uri,
originalFileName: file.name,
errorFields: {
avatar: null,
},
pendingFields: {
avatar: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
originalFileName: null,
},
},
},
Expand Down
1 change: 1 addition & 0 deletions src/pages/DetailsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class DetailsPage extends React.PureComponent {
headerTitle={isSMSLogin ? this.props.toLocalPhone(details.displayName) : details.displayName}
source={ReportUtils.getFullSizeAvatar(details.avatar, details.login)}
isAuthTokenRequired
originalFileName={details.originalFileName}
>
{({show}) => (
<PressableWithoutFocus
Expand Down
13 changes: 6 additions & 7 deletions src/styles/StyleUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,13 @@ function getZoomCursorStyle(isZoomed, isDragging) {
* @param {Number} zoomScale
* @param {Number} containerHeight
* @param {Number} containerWidth
* @return {Object}
* @param {Boolean} isLoading
* @returns {Object | null}
*/
function getZoomSizingStyle(isZoomed, imgWidth, imgHeight, zoomScale, containerHeight, containerWidth) {
if (imgWidth === 0 || imgHeight === 0) {
return {
height: isZoomed ? '250%' : '100%',
width: isZoomed ? '250%' : '100%',
};
function getZoomSizingStyle(isZoomed, imgWidth, imgHeight, zoomScale, containerHeight, containerWidth, isLoading) {
// Hide image until finished loading to prevent showing preview with wrong dimensions
if (isLoading || imgWidth === 0 || imgHeight === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (isLoading || imgWidth === 0 || imgHeight === 0) {
// Hide image until finished loading to prevent showing preview with wrong dimensions
if (isLoading || imgWidth === 0 || imgHeight === 0) {

return undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return undefined;
return null;

Or if we have to return undefined, change jsdoc to * @returns {Object | undefined}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eVoloshchak I prefer using undefined. I tested all platforms with undefined and I believe it works well. And changing @returns {Object | undefined} is good. Thanks

}
const top = `${Math.max((containerHeight - imgHeight) / 2, 0)}px`;
const left = `${Math.max((containerWidth - imgWidth) / 2, 0)}px`;
Expand Down