[$250] skeleton view goes over the edges of the screen reported by @parasharrajat

[kavimuru]

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. Login to the app on mobile app.
2. Navigate to any report that shows skeleton view.

Expected result:

Skeleton view should have right padding like the left edge of 20px.

Actual:

There is not padding at the right edge of the Skeleton view, it seems to go over the edge.

Workaround:

unknown

Platform:

Where is this issue occurring?

• iOS
• Android
• Mobile Web

Version Number: 1.2.27-4
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Expensify/Expensify Issue URL:
Issue reported by: @parasharrajat
Slack conversation:
https://expensify.slack.com/archives/C049HHMV9SM/p1668532881726039
View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0104a6f656912f7483
• Upwork Job ID: 1593381015543803904

[melvin-bot]

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

[syedsaroshfarrukhdot]

Proposal :-

We need to increase width to 100% in SkeletonViewLines.js and pass styles.mr5 to SkeletonViewContentLoader

App/src/components/ReportActionsSkeletonView/SkeletonViewLines.js

Line 18 in 652a51a

<Rect x="67" y="31" width="90%" height="8" />

Like This

 <SkeletonViewContentLoader
    height={CONST.CHAT_SKELETON_VIEW.HEIGHT_FOR_ROW_COUNT[props.numberOfRows]}  
    style={styles.mr5}
>
    <Circle cx="40" cy="26" r="20" />
    <Rect x="67" y="11" width="20%" height="8" />
    <Rect x="67" y="31" width="100%" height="8" />
    {props.numberOfRows > 1 && <Rect x="67" y="51" width="50%" height="8" />}
    {props.numberOfRows > 2 && <Rect x="67" y="71" width="50%" height="8" />}
   
</SkeletonViewContentLoader>

After Fixing

Web :-

Native :-

mWeb :-

[parasharrajat]

@shawnborton Can you please review this issue?

[shawnborton]

Yup, that looks good. But do we know why this happened in the first place? There must have been a regression at some point right?

[syedsaroshfarrukhdot]

@shawnborton I looked up and and found that this initial commit in SkeletonViewLines.js 0bfe1ba had created issue on android / IOS. So I believe it wasn't a regression. It might have been overlooked during development.

The issue is there is no initial marginLeft we are setting cx="40" for the eclipse which move eclipse on x-axis and when we give width to Rect to "90%" it creates issue in native platform so we can increase width to "100%" and give marinRight 20 to parent view to achieve the desired Goal of spacing at the right side so it doesn't overflow. My Proposal #12744 (comment)

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0104a6f656912f7483

[melvin-bot]

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

[melvin-bot]

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

[mananjadhav]

Thanks for the proposal @syedsaroshfarrukhdot. Your proposal looks good to me. I had worked on the original implementation. I wonder how I missed this.

@grgia All yours.

🎀 👀 🎀 
C+ reviewed

[melvin-bot]

📣 @syedsaroshfarrukhdot You have been assigned to this job by @grgia!
Please apply to this job in Upwork 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 📖

[MitchExpensify]

Offers sent to all! https://www.upwork.com/jobs/~0104a6f656912f7483

[syedsaroshfarrukhdot]

Thank You. Just Accepted The Offer Will Be Making A PR Today.

[mananjadhav]

@MitchExpensify @grgia The PR was deployed to production 3 days back but the title with payment date wasn't updated

[mananjadhav]

@MitchExpensify @grgia This is ready for payout tomorrow. Also eligible for 50% bonus to complete within 3 days.

[mananjadhav]

@MitchExpensify Quick bump on the payout, and here's the offending PR that added the bug. I worked on it so I have it handy.

[MitchExpensify]

Sorry for the delay @mananjadhav, I've been sick the last few days. Paying now

[MitchExpensify]

Everyone's been paid - Thank you! Will need to manually get the bug zero checklist in here as a next step (Awaiting payment label didn't do the trick)

[MitchExpensify]

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:

• [@grgia @mananjadhav] The PR that introduced the bug has been identified. Link to the PR: feat: Added Skeleton UI to chats #8042
• [@grgia @mananjadhav] 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: feat: Added Skeleton UI to chats #8042
• [@grgia @mananjadhav] 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: https://expensify.slack.com/archives/C049HHMV9SM/p1670879207542109
• [@MitchExpensify] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: https://github.com/Expensify/Expensify/issues/248183

[mananjadhav]

Thanks @MitchExpensify. I've linked the offending PR.

[MitchExpensify]

@mananjadhav Nice! Mind taking the other steps here when you get a chance?

[MitchExpensify]

In terms of a regression test, asked here in Slack

[mananjadhav]

@grgia Can you help with the other two steps. I am not sure if we follow a template to post the comment and discussion?

[mananjadhav]

@grgia @MitchExpensify Quick bump here.

[MitchExpensify]

Created an issue for the regression test!

[MitchExpensify]

I am not sure if we follow a template to post the comment and discussion?

there is no template

[MitchExpensify]

@grgia Seeing as we only have those two checks to complete before closing this, mind handling that once done? I'm heading ooo so if you could cover closing this issue once they're checked off it'd be great!

[grgia]

Yep, sorry for the delay, getting on that now!

[grgia]

Done! Commented on PR and discussion started

[MitchExpensify]

Nice! Closing out