Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2024-12-19] [$250] Web - Search - Empty state page skeleton is cut off from the right #53251

Closed
1 of 8 tasks
IuliiaHerets opened this issue Nov 28, 2024 · 32 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Nov 28, 2024

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: 9.0.68-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue was found when executing this PR: #51913
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Navigate to Search
  3. Observe the background skeleton empty state on the right side

Expected Result:

The oval skeleton doesn't get cut off from the right

Actual Result:

The oval skeleton gets cut off from the right when there is a scroll bar on the right

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6679181_1732806399096!image

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021865178692528155887
  • Upwork Job ID: 1865178692528155887
  • Last Price Increase: 2024-12-06
Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @jliexpensify
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 28, 2024
Copy link

melvin-bot bot commented Nov 28, 2024

Triggered auto assignment to @jliexpensify (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.

@bernhardoj
Copy link
Contributor

Proposal

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

The search page skeleton right side is cut off when there is a scrollbar.

What is the root cause of that problem?

This is how the calculation looks to decide the position of the right-most skeleton. We want to right-aligned the skeleton.

<Rect
// We have to calculate this value to make sure the element is aligned to the right border.
x={windowWidth - leftPaneWidth - rightSideElementWidth - gapWidth - centralPanePadding}

The problem here is that when there is a scrollbar, the width of the search central pane is reduced, but since we used the windowWidth - lhn width to get the central pane width, the value is the same because windowWidth isn't reduced when there is a scrollbar. So, the skeleton position doesn't change either, thus getting cut off.

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

To solve it, first, we can get the width of the skeleton container from onLayout

<View style={[styles.flex1, containerStyle]}>

const [containerWidth, setContainerWidth] = useState(0);

<View style={[styles.flex1, containerStyle]} onLayout={(e) => setContainerWidth(e.nativeEvent.layout.width)}>

Then, use the containerWidth on the calculation like this:

x={containerWidth - rightSideElementWidth - gapWidth - centralPanePadding}

We need to do the same for this skeleton

<Rect
// We have to calculate this value to make sure the element is aligned to the button on the right side.
x={windowWidth - leftPaneWidth - rightButtonWidth - gapWidth - centralPanePadding - gapWidth - rightSideElementWidth}

x={containerWidth - rightButtonWidth - gapWidth - centralPanePadding - gapWidth - rightSideElementWidth}

What alternative solutions did you explore? (Optional)

Since we don't show scroll indicator for scroll page,

showScrollIndicator={false}

we can just hide it for the skeleton too by passing the showsVerticalScrollIndicator to EmptyStateComponent.

<EmptyStateComponent
SkeletonComponent={SearchRowSkeleton}
headerMediaType={CONST.EMPTY_STATE_MEDIA.ANIMATION}
headerMedia={content.headerMedia}
headerStyles={[content.headerStyles, styles.emptyStateCardIllustrationContainer]}
title={content.title}
titleStyles={content.titleStyles}
subtitle={content.subtitle}
buttons={content.buttons}
headerContentStyles={[styles.h100, styles.w100, content.headerContentStyles]}
lottieWebViewStyles={styles.emptyStateFolderWebStyles}
/>

and then use it here.

<ScrollView
contentContainerStyle={[{minHeight: minModalHeight}, styles.flexGrow1, styles.flexShrink0, containerStyles]}
style={styles.flex1}
>

@jliexpensify
Copy link
Contributor

Hi @deetergp and @Pujan92 - this is related to #51913, so assigning you to review.

@melvin-bot melvin-bot bot added the Overdue label Dec 2, 2024
Copy link

melvin-bot bot commented Dec 2, 2024

@deetergp, @Pujan92, @jliexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@jliexpensify
Copy link
Contributor

Waiting on @deetergp and @Pujan92 to weigh in

@melvin-bot melvin-bot bot removed the Overdue label Dec 2, 2024
@deetergp
Copy link
Contributor

deetergp commented Dec 4, 2024

@bernhardoj's proposal sounds reasonable to me. What do you think @Pujan92?

@Pujan92
Copy link
Contributor

Pujan92 commented Dec 5, 2024

Yes @deetergp but I am not able to reproduce it in my system.

Screen.Recording.2024-12-05.at.17.48.00.mov

@bernhardoj
Copy link
Contributor

@Pujan92 please try to always show the scrollbar
image

@Pujan92
Copy link
Contributor

Pujan92 commented Dec 6, 2024

I am 👍 with the @bernhardoj's alternative solution of passing showsVerticalScrollIndicator prop to the ScrollView.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Dec 6, 2024

Current assignee @deetergp is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@deetergp deetergp added Design External Added to denote the issue can be worked on by a contributor and removed Design labels Dec 6, 2024
@melvin-bot melvin-bot bot changed the title Web - Search - Empty state page skeleton is cut off from the right [$250] Web - Search - Empty state page skeleton is cut off from the right Dec 6, 2024
Copy link

melvin-bot bot commented Dec 6, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 6, 2024
Copy link

melvin-bot bot commented Dec 6, 2024

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

Copy link

melvin-bot bot commented Dec 6, 2024

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

@deetergp
Copy link
Contributor

deetergp commented Dec 6, 2024

Please disregard @shawnborton — I accidentally clicked the wrong tag :ohnothing:

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Dec 7, 2024
@bernhardoj
Copy link
Contributor

PR is ready
cc: @Pujan92

@shawnborton
Copy link
Contributor

Is this some kind of regression or was it always set up this way? cc @filip-solecki

@filip-solecki
Copy link
Contributor

Tbh it's hard to say, maybe there were some changes that broke up the calculations or it is uncovered scenario.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 12, 2024
@melvin-bot melvin-bot bot changed the title [$250] Web - Search - Empty state page skeleton is cut off from the right [HOLD for payment 2024-12-19] [$250] Web - Search - Empty state page skeleton is cut off from the right Dec 12, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 12, 2024
Copy link

melvin-bot bot commented Dec 12, 2024

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

Copy link

melvin-bot bot commented Dec 12, 2024

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

If no regressions arise, payment will be issued on 2024-12-19. 🎊

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

  • @Pujan92 requires payment through NewDot Manual Requests
  • @bernhardoj requires payment through NewDot Manual Requests

Copy link

melvin-bot bot commented Dec 12, 2024

@Pujan92 @jliexpensify @Pujan92 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@jliexpensify
Copy link
Contributor

jliexpensify commented Dec 16, 2024

Payment Summary

@Pujan92
Copy link
Contributor

Pujan92 commented Dec 16, 2024

@jliexpensify this isn't a regression of the mentioned PR, and it is a new fresh issue. So I am also eligible for the payment.

@jliexpensify
Copy link
Contributor

Cool, thanks for letting me know - will add you in.

@jliexpensify
Copy link
Contributor

Hi @deetergp - I'm headed OOO from the 18th - 29th, do you mind closing this GH when the payment date passes?

Payment Summary is here.

@Pujan92 - is there a need for a checklist?

@deetergp
Copy link
Contributor

Hi @deetergp - I'm headed OOO from the 18th - 29th, do you mind closing this GH when the payment date passes?

Will do @jliexpensify! Enjoy our OOO time 🎉

@jliexpensify
Copy link
Contributor

Thanks Scott, have a great Holiday Break!

@Pujan92 since it's my last day today, I want to confirm that there's no checklist?

@Pujan92
Copy link
Contributor

Pujan92 commented Dec 18, 2024

@jliexpensify I think checklist isn't needed here.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 18, 2024
@jliexpensify
Copy link
Contributor

Great, thanks - in that case, Scott can simply close this on the 19th if there are no regressions.

Copy link

melvin-bot bot commented Dec 19, 2024

Payment Summary

Upwork Job

BugZero Checklist (@jliexpensify)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1865178692528155887/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@garrettmknight
Copy link
Contributor

$250 approved for @Pujan92

@bernhardoj
Copy link
Contributor

Requested in ND.

@JmillsExpensify
Copy link

$250 approved for @bernhardoj based on this summary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

10 participants