[$1000] mWeb - Chat – Unable to zoom the image with pinching
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 https://github.com/Expensify/App/pull/34080
Action Performed:
- Go to https://staging.new.expensify.com/
- Login
- Go to a report with attachments
- Open an (image) attachment
- 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
- [x] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/93399543/ce464be6-14ca-4d7a-a2eb-598f8ae21ba6
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~016170309acaa20014
- Upwork Job ID: 1758158238713884672
- Last Price Increase: 2024-03-25
- Automatic offers:
- situchan | Contributor | 0
Job added to Upwork: https://www.upwork.com/jobs/~016170309acaa20014
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External)
Triggered auto assignment to @laurenreidexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
We think that this bug might be related #vip-vsb CC @quinthar
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),
https://github.com/Expensify/App/blob/8bdb753045cf0eee6cf1d342adf8b76040cabac4/src/components/ImageView/index.tsx#L194-L215
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:
https://github.com/Expensify/App/assets/60419079/6daf30ef-b1e4-4d73-8b11-1469b4046c81
https://github.com/Expensify/App/assets/60419079/33ef5846-5069-4caa-ae78-995cd4c799db
What alternative solutions did you explore? (Optional)
Please reassign. I can't take it!
I can take over.
We are not yet supporting zoom image in mobile web. Is it right time to implement this new feature?
📣 @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 📖
Asking internally here https://expensify.slack.com/archives/C066HJM2CAZ/p1708364190263029 whether we need to fix this
Still chatting internally
@laurenreidexpensify, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!
Bumped thread internally for further clarity
@situchan confirmed internally - let's do it!
@laurenreidexpensify thanks
@andreasnw thanks for the proposal. Please test your solution yourself on android chrome and iOS safari.
Waiting proposals
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Proposal
What is the root cause of that problem?
We only have gestures like pinch on native because of the lightbox implementation here: https://github.com/Expensify/App/blob/8bdb753045cf0eee6cf1d342adf8b76040cabac4/src/components/ImageView/index.native.tsx
What changes do you think we should make in order to solve the problem?
- Similar to @andreasnw, we want to replace the Image component in https://github.com/Expensify/App/blob/8bdb753045cf0eee6cf1d342adf8b76040cabac4/src/components/ImageView/index.tsx#L200
However additionally: With the lightbox implementation exactly as defined in https://github.com/Expensify/App/blob/8bdb753045cf0eee6cf1d342adf8b76040cabac4/src/components/ImageView/index.native.tsx
- We should clean up the native implementation as it will be a redudancy.
- We should ensure that the new lightbox implementation for touch devices, has the default values such as zoomRange with DEFAULT_ZOOM_RANGE from '@components/MultiGestureCanvas'; As well as other's as defined in the native lightbox implementation.
- We can remove the existing loading logic from. https://github.com/Expensify/App/blob/8bdb753045cf0eee6cf1d342adf8b76040cabac4/src/components/ImageView/index.tsx#L212 and rely in the Lightbox's loading logic
- We don't need a zoomScale on lightbox
What alternative solutions did you explore? (Optional)
proposed implementation experience: Android: https://github.com/Expensify/App/assets/157416545/7f1d7a51-1dc8-4cfb-ac33-8fc24b648368 IOS:
https://github.com/Expensify/App/assets/157416545/7315f00a-0cc8-471e-a401-a873d32b9cdc
@jeremy-croff zoomScale is necessary to prevent tiny images from looking like this
@jeremy-croff zoomScale is necessary to . The native implementation does not have this behaviour; either it's intentional, or if we want to add a scale factor, both experiencesges.githubusercontent.com/60419079/307819073-342ef517-062c-45d2-9bf3-56f89faffc37.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MDg5NTY3NDQsIm5iZiI6MTcwODk1NjQ0NCwicGF0aCI6Ii82MDQxOTA3OS8zMDc4MTkwNzMtMzQyZWY1MTctMDYyYy00NWQyLTliZjMtNTZmODlmYWZmYzM3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDAyMjYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwMjI2VDE0MDcyNFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWQ0Zjg2MGFlNWRkNDEwZmFkMGVmZDRlM2RlMzYyYTA1NmE5MzJiZWU3MGQ4YmQ1OGI2MGE1YTcxZmQwZjc5Y2MmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.Vg6CjitMNGlPA5VbSgW1FOM-FsdFT2ky1SElXEGUFSE)
Thanks for the example, So my basis is that it's not a requirement, the native implementation does not have this behaviour either it's intentional, or if we want to add a scale factor both experience will need it.
@situchan some new proposals to review ^^
@andreasnw @jeremy-croff do you mind sharing test branches?
@andreasnw @jeremy-croff do you mind sharing test branches?
https://github.com/Expensify/App/pull/37424/files
sure @situchan https://github.com/andreasnw/Expensify/tree/feat/web_app_gesture_handler
@laurenreidexpensify @situchan this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
proposals are in review & testing
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@situchan any update on the review and testing yet? thanks