[HOLD for payment 2022-02-09] Attachment modal ui freeze/unfreeze when long height image displayed - reported by @PrashantMangukiya

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open app
2. Upload long image
3. Try to zoom, move image around, etc.

Expected Result:

User should be able to zoom, move image around

Actual Result:

Can not move image up/down without minor zoom. (sometime unfreeze and works, but again freeze) This happen when image height larger than screen height etc. Attached is the sample image used.

Workaround:

Unknown

Platform:

Where is this issue occurring?

• iOS
• Android
• Mobile Web

Version Number:
Reproducible in staging?:
Reproducible in production?:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Similar issues logged in the past
#6518
#5870
#4867

AttachmentModal.mov

Expensify/Expensify Issue URL:
Issue reported by: @PrashantMangukiya
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1640265916049300

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @alex-mechler (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[alex-mechler]

This doesnt look frozen, it looks more like we cannot move the image unless it is zoomed, like mentioned in the report

Can not move image up/down without minor zoom

This is purely on the client side, so this should be able to go external.

[MelvinBot]

Triggered auto assignment to @JmillsExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[sobitneupane]

Reason
While setting image height and width, we have only checked whether imageWidth is greater than windowWidth or not in

App/src/components/ImageView/index.native.js

Lines 64 to 73 in 7af06bc

	ImageSize.getSize(this.props.url).then(({width, height}) => {
	let imageWidth = width;
	let imageHeight = height;
	const aspectRatio = (imageHeight / imageWidth);
	if (imageWidth > this.props.windowWidth) {
	imageWidth = Math.round(this.props.windowWidth);
	imageHeight = Math.round(imageWidth * aspectRatio);
	}
	this.setState({imageHeight, imageWidth});
	});

To solve this issue, we must also check whether imageHeight is greather than windowHeight or not.

Proposal
We should replace

ImageSize.getSize(this.props.url).then(({width, height}) => {
            let imageWidth = width;
            let imageHeight = height;
            const aspectRatio = (imageHeight / imageWidth);
            if (imageWidth > this.props.windowWidth) {
                imageWidth = Math.round(this.props.windowWidth);
                imageHeight = Math.round(imageWidth * aspectRatio);
            }
+           if(imageHeight>this.props.windowHeight - variables.contentHeaderHeight) {
+               imageHeight = Math.round(this.props.windowHeight - variables.contentHeaderHeight)
+               imageWidth = Math.round(1/aspectRatio * imageHeight)
+           }
            this.setState({imageHeight, imageWidth});
        });

[JmillsExpensify]

Thanks for the video! Super helpful. Posting to Upwork now and we'll get a reviewer on this as well!

[JmillsExpensify]

Upwork is here: https://www.upwork.com/jobs/~0139e45ab43993899b.

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (Exported)

[MelvinBot]

Triggered auto assignment to @deetergp (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[parasharrajat]

Nice catch @sobitneupane. The proposal looks good.

cc: @deetergp

🎀 👀 🎀 C+ reviewed

[deetergp]

@sobitneupane's proposal looks good. Let's make sure we add some comments explaining why this bit of code is necessary.

[sobitneupane]

@sobitneupane's proposal looks good. Let's make sure we add some comments explaining why this bit of code is necessary.

Okay.

[sobitneupane]

Should I wait until the issue is assigned to me? Or Should I proceed my PR?

[parasharrajat]

You can proceed to create the PR. @deetergp can assign you later

[deetergp]

Thanks for the PR, @sobitneupane! Did we end up hiring you for the job in Upwork?

[sobitneupane]

@deetergp No. Not yet. I have applied for the job.

[deetergp]

@JmillsExpensify let's hire & pay @sobitneupane for the work 👍

[JmillsExpensify]

Jumping in now!

[JmillsExpensify]

I took a look at the associated PR and it's still in the process of getting to staging.

In the meantime, I've hired all contributors for this issue and we'll wait for the PR to go through the process of hitting production and waiting for no regressions.

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.34-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Reduce imageHeight to windowHeight if greater #7388

If no regressions arise, payment will be issued on 2022-02-09. 🎊

[JmillsExpensify]

Will hold on this issue pending regressions, per the message above. Circle back then.

[PrashantMangukiya]

@JmillsExpensify Should I also apply on Upwork job for issue reporting bonus?

[JmillsExpensify]

Yes, please!

[PrashantMangukiya]

Applied on Upwork job. Thank you.

[JmillsExpensify]

Payments processed for PR contributor and contributor plus. Will pay out for reporting as soon as the offer is accepted. :)

[PrashantMangukiya]

Offer accepted.

[JmillsExpensify]

Thanks everyone. Closing the issue, though don't hesitate to reach out if I can help with any questions or comments.