[$250] Android - Receipt - Receipt view is inconsistent in mweb&Android

[lanitochka17]

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

---

Version Number: 1.4.64 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4504759 Issue reported by:Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home / launch app
2. Tap on Workspace chat
3. Tap plus icon -- submit expense
4. Submit an expense with below receipt attached
5. Open the expense and tap on the receipt

Expected Result:

Receipt view must not be inconsistent in mweb and Android

Actual Result:

Receipt is shown smaller in mweb and bigger in Android

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [ ] 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/78819774/35c95315-1fee-43ff-935d-bb9d2849ac7f

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a54a18910078b715
• Upwork Job ID: 1783190710289416192
• Last Price Increase: 2024-05-08
• Automatic offers:
• samilabud | Contributor | 0

[melvin-bot[bot]]

Triggered auto assignment to @CortneyOfstad (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@CortneyOfstad FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01a54a18910078b715

[melvin-bot[bot]]

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

[samilabud]

Proposal

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

Android receipts are displayed as the same size of the canvas size

What is the root cause of that problem?

We are using the MultigestureCanvas to display the image/receipt, this component always try to fit the image to the content, this works perfectly for image that are bigger than the canvas size but when the image is lower than the canvas we can notice this issue. This is because we are getting the min scale of the image like below:

https://github.com/Expensify/App/blob/abf9f8d3caaad397b7233502ff60386ca39e7889/src/components/MultiGestureCanvas/utils.ts#L5-L13

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

We should verify if we need to use the scale to fit the canvas size before we try to fit the image in getCanvasFitScale, and if the image not need to be resized we should use the same image size like:

type ShouldResizeToFit = (canvasSize: CanvasSize, contentSize: ContentSize) => boolean;
type GetCanvasFitScale = (props: {canvasSize: CanvasSize; contentSize: ContentSize}) => {scaleX: number; scaleY: number; minScale: number; maxScale: number};

const shouldResizeToFit: ShouldResizeToFit = (canvasSize, contentSize) => {
    // If the image is smaller than canvas should no fit to canvas scale
    if (canvasSize && contentSize) {
        return canvasSize.width < contentSize.width || canvasSize.height < contentSize.height;
    }
    return false;
};

const getCanvasFitScale: GetCanvasFitScale = ({canvasSize, contentSize}) => {
    const shouldResize = shouldResizeToFit(canvasSize, contentSize);

    const scaleX = canvasSize.width / contentSize.width;
    const scaleY = canvasSize.height / contentSize.height;

    const minScale = !shouldResize ? 1 : Math.min(scaleX, scaleY);
    const maxScale = Math.max(scaleX, scaleY);

    return {scaleX, scaleY, minScale, maxScale};
};

Testing the change:

https://github.com/Expensify/App/assets/5216036/6e4d751d-f4f5-41f9-93a0-49024fcee2c4

[CortneyOfstad]

@eVoloshchak any thoughts on the proposal above?

[eVoloshchak]

@samilabud, thank you for the proposal! Unfortunately, it breaks the layout of bigger images (i.e. if the image resolution is bigger than the screen resolution, it isn't resized to fit)

https://github.com/Expensify/App/assets/9059945/cde04716-e4ae-47f3-ba3b-ad7b201e1062

[samilabud]

@samilabud, thank you for the proposal! Unfortunately, it breaks the layout of bigger images (i.e. if the image resolution is bigger than the screen resolution, it isn't resized to fit)

Screen.Recording.2024-05-01.at.14.22.45.mov

Oh you were right, I have updated my proposal with a new video and added a function to determinate if we should resize the image to fit the canvas size, please let me know if it looks good to you 🙏🏼

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[CortneyOfstad]

Thanks @samilabud!

CC @eVoloshchak 👍

[eVoloshchak]

@samilabud, I still think this is more of a workaround than an actual fix of the root cause, let's try to dig a bit deeper. Why is passing correct content size as contentSize results in image being displayed incorrectly? Is this something that could be fixed inside of the MultiGestureCanvas itself?

[melvin-bot[bot]]

@eVoloshchak @CortneyOfstad 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!

[samilabud]

Is this something that could be fixed inside of the MultiGestureCanvas itself? Hi @eVoloshchak, please see my alternative proposal, and thanks for the suggestions it makes more sense!

Proposal

Updated

[melvin-bot[bot]]

@eVoloshchak @CortneyOfstad 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!

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[CortneyOfstad]

@eVoloshchak it looks like we have an updated proposal here. Can you take a look when you have a chance? Thanks!

[CortneyOfstad]

@eVoloshchak just bumping for feedback on the proposal above — thanks!

[CortneyOfstad]

@eVoloshchak bump again — please have the feedback provided by EOD. Thanks!

[eVoloshchak]

@samilabud, I think I'm missing the updated proposal, GH says the proposal was last updated on May 1st

Why is passing correct content size as contentSize results in image being displayed incorrectly?

We need to know the answer to this question. We pass the correct contentSize, correct canvasSize, but if the content is smaller than the canvas, it is displayed incorrectly. Could this be a problem with MultiGestureCanvas itself?

[samilabud]

@samilabud, I think I'm missing the updated proposal, GH says the proposal was last updated on May 1st

Why is passing correct content size as contentSize results in image being displayed incorrectly?

We need to know the answer to this question. We pass the correct contentSize, correct canvasSize, but if the content is smaller than the canvas, it is displayed incorrectly. Could this be a problem with MultiGestureCanvas itself?

Oh my bad, I didn't save the update of the proposal, I'm going to do that and let you know

[samilabud]

@eVoloshchak Now I have updated it, sorry for the inconvenience, any suggestions I am at your service.

Proposal

Updated

[eVoloshchak]

@samilabud, awesome! I don't think we even need the shouldResizeToFit function, we could simply cap the scale at 1. If the scale is above 1, it means the canvas is bigger than the image and we should just set the scale to 1

I think we should proceed with @samilabud's proposal 🎀👀🎀 C+ reviewed!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @samilabud 🎉 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 📖

[CortneyOfstad]

@eVoloshchak any update on the PR review? Thanks!