[HOLD for payment 2022-09-23] [$250] Web - Login - When the page size is reduced, the information on the banner is cut off

[kbecciv]

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. Go to Login page NewDot
2. Decrease the screen width

Expected Result:

When the page size is reduced, the information on the banner is cut off.
Clipped only beige banner.

Actual Result:

Describe what actually happened

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.88.13

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5691712_Recording__1065.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

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

[kakajann]

Proposal

This line:

App/src/pages/signin/SignInPageLayout/index.js

Line 56 in 25a7a90

resizeMode={props.isMediumScreenWidth ? 'contain' : 'cover'}

should be:

resizeMode="contain"

Result

Before

before.mov

After

after.mov

[thesahindia]

@Beamanator, we can close this one as we already have an internal issue for this.

[Beamanator]

@thesahindia good find!

@ctkochan22 any reason you'd prefer making this issue internal? If this is just a styling thing let's make it external, but if there's other considerations please let me know :D

[melvin-bot]

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

[ctkochan22]

Lets handle this issue here.

[ctkochan22]

I don't think just switching it to contain works. We use "cover" so that when the area is larger than the image, it scales up accordingly. If we just applied "contain" it would not do so.

@shawnborton I introduced this bug originally here: #7771 (sorry). I think the issue is that the graphic/text for the CPA image is closer to the margin than the usual background images.

• We are pulling the images here:

  App/src/styles/StyleUtils.js

  Line 379 in a70aa0c

  backgroundImageUri: `${CONST.CLOUDFRONT_URL}/images/homepage/brand-stories/cpa-card.svg`,

• When we reach a certain screen width, isMediumScreenWidth we switch the <Image> component's "resizeMode" from "cover" to "contain":

  App/src/pages/signin/SignInPageLayout/index.js

  Line 56 in a0c8daf

  resizeMode={props.isMediumScreenWidth ? 'contain' : 'cover'}

I think the issue is that isMediumScreenWidth turns true after we cut off the CPA banner.

Solutions include,

1. having a stricter cut-off for the login page. Specifically for the CPA banner.
2. editing the image itself to give it more berth
3. Maybe give the CPA image margin on the right

[arielgreen]

Unassigning myself since we're fixing internally.

[shawnborton]

What happens if we switch contain to cover in the isMediumScreenWidth scenario?

I would be down to just do nothing here.

[ctkochan22]

So "cover" will scale it up, the larger the window gets it'll scale up. While "contain" will always make sure the image is shown by scaling down.

Here is a video: https://recordit.co/kWeNVNLzUo

It seems a few people keep reporting it because their desktop's are smaller? So its often show close or cut off:

[ctkochan22]

So if we just set it as "contain", it wont' scale up per se. So stretched out , it'll look like this:

Instead of:

[shawnborton]

But what about keeping it as contain just at the IsMedium width? Maybe that will just force the whole thing to be shown at that very particular width?

[ctkochan22]

But what about keeping it as contain just at the IsMedium width?

I think we are already doing that,

App/src/pages/signin/SignInPageLayout/index.js

Line 56 in a0c8daf

resizeMode={props.isMediumScreenWidth ? 'contain' : 'cover'}

At least if I'm understanding you correctly. So if isMedium width is true, use "contain". But for anything greater, use "cover"

[shawnborton]

Maybe I am suggesting the opposite then. What happens when we just use cover for isMedium?

[kakajann]

contain is also responsive. you can check it in my proposal video

[shawnborton]

Okay so that might do the trick then at the isMedium size - can you also share what it looks like for the CPA card image?

[mananjadhav]

@dylanexpensify A bit confused here, is C+ required here? and PR is not merged yet it says Payment sent.

Quick note, I went through the history, I'll review the PR today in a few hours(if it still needs a review).

[dylanexpensify]

@mananjadhav ah this is my fault, I thought we needed to create/assign the job and pay it out, but I see we needed to wait for review and merge. Please proceed as normal with review when you can! cc @ctkochan22

[mananjadhav]

Thanks for clarifying. I'll get to it in sometime.

[dylanexpensify]

Any update @mananjadhav?

[mananjadhav]

@dylanexpensify PR is merged and deployed to staging 2 days back

[melvin-bot]

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

• fix: web banner cut off #10879

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

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[dylanexpensify]

reassigning for payment as I'm OOO tomorrow!

[melvin-bot]

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

[melvin-bot]

Current assignee @mananjadhav is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Current assignee @ctkochan22 is eligible for the External assigner, not assigning anyone new.

[puneetlath]

Hm, it looks like @kakajann was already paid for this job.

@mananjadhav sent you a hiring offer for C+.

[mananjadhav]

Accepted @puneetlath

[puneetlath]

All paid!