[HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [$1000] mWeb - Chat – Unable to zoom the image with pinching

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: v1.4.42-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Interna; Team
Slack conversation:

Issue found when executing PR #34080

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Login
3. Go to a report with attachments
4. Open an (image) attachment
5. Check that image gestures/transformations are working (pinching)

Expected Result:

Able to zoom the image with pinching

Actual Result:

Unable to zoom the image with pinching

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6380765_1708008491410.Screenrecorder-2024-02-15-14-08-47-83.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016170309acaa20014
• Upwork Job ID: 1758158238713884672
• Last Price Increase: 2024-06-04
• Automatic offers:
• situchan | Contributor | 0
• dukenv0307 | Reviewer | 102684851
• ZhenjaHorbach | Contributor | 102711497

Issue Owner

Current Issue Owner: @dukenv0307

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016170309acaa20014

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @laurenreidexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[kbecciv]

We think that this bug might be related #vip-vsb
CC @quinthar

[andreasnw]

Proposal

Please re-state the problem that we are trying to solve in this issue.

gestures won't work when viewing image attachments on the web app

What is the root cause of that problem?

new feature

What changes do you think we should make in order to solve the problem?

on web ImageView component, if the device has a touch screen (canUseTouchScreen is true),

App/src/components/ImageView/index.tsx

Lines 194 to 215 in 8bdb753

	if (canUseTouchScreen) {
	return (
	<View
	style={[styles.imageViewContainer, styles.overflowHidden]}
	onLayout={onContainerLayoutChanged}
	>
	<Image
	source={{uri: url}}
	isAuthTokenRequired={isAuthTokenRequired}
	// Hide image until finished loading to prevent showing preview with wrong dimensions.
	style={isLoading || zoomScale === 0 ? undefined : [styles.w100, styles.h100]}
	// When Image dimensions are lower than the container boundary(zoomscale <= 1), use `contain` to render the image with natural dimensions.
	// Both `center` and `contain` keeps the image centered on both x and y axis.
	resizeMode={zoomScale > 1 ? RESIZE_MODES.center : RESIZE_MODES.contain}
	onLoadStart={imageLoadingStart}
	onLoad={imageLoad}
	onError={onError}
	/>
	{(isLoading || zoomScale === 0) && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
	</View>
	);
	}

we display images using the Lightbox component instead of regular images. This will allow users to interact with the images using touch gestures.

 if (canUseTouchScreen) {
        return (
            <Lightbox
                uri={url}
                zoomRange={zoomRange}
                isAuthTokenRequired={isAuthTokenRequired}
                onError={onError}
                style={style}
            />
        );
    }

Lightbox component also requires additional zoomScale to define how images are resized.

const zoomScale = Math.min((canvasSize?.width ?? 0) / (contentSize?.width ?? 0), (canvasSize?.height?? 0) / (contentSize?.height ?? 0));

this controls how the image fits within the frame when its original size differs from the frame's dimensions.

 <MultiGestureCanvas
                                isActive={isActive}
                                canvasSize={canvasSize}
                                contentSize={contentSize}
                                zoomRange={zoomRange}
                                pagerRef={pagerRef}
                                shouldDisableTransformationGestures={isPagerScrolling}
                                onTap={onTap}
                                onScaleChanged={scaleChange}
                            >
                                <Image
                                    source={{uri}}
                                    style={contentSize ?? DEFAULT_IMAGE_DIMENSION}
                                    isAuthTokenRequired={isAuthTokenRequired}
                                    onError={onError}
                                    onLoad={updateContentSize}
                                    onLoadEnd={() => {
                                        setLightboxImageLoaded(true);
                                    }}
                                    resizeMode={zoomScale > 1 ? RESIZE_MODES.center : RESIZE_MODES.contain}
                                />
                            </MultiGestureCanvas>

result:

Simulator.Screen.Recording.-.iPhone.14.-.2024-02-26.at.00.03.37.mp4

untitled.mp4

What alternative solutions did you explore? (Optional)

[thesahindia]

Please reassign. I can't take it!

[situchan]

I can take over.

We are not yet supporting zoom image in mobile web.
Is it right time to implement this new feature?

[melvin-bot]

📣 @situchan 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[laurenreidexpensify]

Asking internally here https://expensify.slack.com/archives/C066HJM2CAZ/p1708364190263029 whether we need to fix this

[laurenreidexpensify]

Still chatting internally

[melvin-bot]

@laurenreidexpensify, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[laurenreidexpensify]

Bumped thread internally for further clarity

[laurenreidexpensify]

@situchan confirmed internally - let's do it!

[situchan]

@laurenreidexpensify thanks

[melvin-bot]

📣 @ZhenjaHorbach 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[ZhenjaHorbach]

@mallenexpensify
Thank you 😀

PR will be ready today or tomorrow
If you don't mind
I want to spend some time investigating the issue and the chosen proposal.

[dukenv0307]

@badeggg In the PR phase, the author used LightBox as you suggested before. I also test your mention here

Unable to make FlatList to be part of react-native-gesture-handler's gesture system.
That's why we can't swipe to switch after change Image to Lightbox, like this
I have tried every possible way I can imagine.
So we have to do scrolling by self

I still can swipe

Screen.Recording.2024-06-18.at.10.15.51.mov

manualActivation(true) of Gesture.Pan() not working on mWeb

how can we verify that?

double tap .numberOfTaps(2) not working on chrome for iOS, does work on safari on iOS

@ZhenjaHorbach Can you check the bug on chrome iOS?

[email protected] may not reporting 'allTouches' when onTouchesMove correctly
on mWeb, stale touch may exist.

Can you elaborate more?

[badeggg]

If the bug I noticed does not exist any more, maybe it's because dependency version updating.

And, just curious, why the assigned contributor is not the one who proposed the c+ accepted proposal.

[ZhenjaHorbach]

@badeggg In the PR phase, the author used LightBox as you suggested before. I also test your mention here

Unable to make FlatList to be part of react-native-gesture-handler's gesture system.
That's why we can't swipe to switch after change Image to Lightbox, like this
I have tried every possible way I can imagine.
So we have to do scrolling by self

I still can swipe

Screen.Recording.2024-06-18.at.10.15.51.mov

manualActivation(true) of Gesture.Pan() not working on mWeb

how can we verify that?

double tap .numberOfTaps(2) not working on chrome for iOS, does work on safari on iOS

@ZhenjaHorbach Can you check the bug on chrome iOS?

[email protected] may not reporting 'allTouches' when onTouchesMove correctly
on mWeb, stale touch may exist.

Can you elaborate more?

Actually yes, the bug exists
double tap.numberOfTaps(2) is working on chrome for iOS but very strange and unexpectedly
For the zoom to work we need to click 3 or 4 times
So I will update the PR today or tomorrow

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Image zoom for mobile browser apps #43620

If no regressions arise, payment will be issued on 2024-07-17. 🎊

For reference, here are some details about the assignees on this issue:

• @ZhenjaHorbach requires payment automatic offer (Contributor)
• @dukenv0307 requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@ZhenjaHorbach / @dukenv0307] The PR that introduced the bug has been identified. Link to the PR:
• [@ZhenjaHorbach / @dukenv0307] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@ZhenjaHorbach / @dukenv0307] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@ZhenjaHorbach / @dukenv0307] Determine if we should create a regression test for this bug.
• [@ZhenjaHorbach / @dukenv0307] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

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

• Image zoom for mobile browser apps #43620

If no regressions arise, payment will be issued on 2024-07-22. 🎊

For reference, here are some details about the assignees on this issue:

• @ZhenjaHorbach requires payment automatic offer (Contributor)
• @dukenv0307 requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@ZhenjaHorbach / @dukenv0307] The PR that introduced the bug has been identified. Link to the PR:
• [@ZhenjaHorbach / @dukenv0307] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@ZhenjaHorbach / @dukenv0307] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@ZhenjaHorbach / @dukenv0307] Determine if we should create a regression test for this bug.
• [@ZhenjaHorbach / @dukenv0307] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[mallenexpensify]

Contributor: @ZhenjaHorbach paid $1000 via Upwork
Contributor+: @dukenv0307 paid $1000 via Upwork.

@dukenv0307 plz complete the BZ checklist above.

[dukenv0307]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: N/A
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• Determine if we should create a regression test for this bug. Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression tests:

1. Open reports with attachments on mobile web.
2. Open image attachment.
3. Verify image can be dragged and zoomed using pan and pinch gestures.

Do we 👍 or 👎

[mallenexpensify]

Thanks @dukenv0307
TestRail update

• https://github.com/Expensify/Expensify/issues/413867