[PAY AUG 22ND][$250] Search-Skeleton during loading & in empty state are aligned differently compared to production

[IuliiaHerets]

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: 9.0.17-1
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Create a new workspace.
3. Go back to Inbox.
4. Open workspace switcher and select the new workspace.
5. Go to Search.
6. Navigate through the empty Search sections to check how the skeleton during loading and the skeleton in empty state are aligned differently on staging and production.

Expected Result:

The skeleton during loading and the skeleton in empty state should have similar alignment (only slight deviation on production).

Actual Result:

The skeleton during loading and the skeleton in empty state has larger deviation in alignment compared to production.

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6564170_1723020514064.20240807_164018.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011af2fbb0ab06fc24
• Upwork Job ID: 1821143733082880779
• Last Price Increase: 2024-08-07
• Automatic offers:
• rayane-djouah | Contributor | 103432889

Issue Owner

Current Issue Owner: @jayeshmangwani

[melvin-bot]

Triggered auto assignment to @luacmartins (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

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.

[IuliiaHerets]

We think that this bug might be related to #vip-vsb

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[nyomanjyotisa]

Proposal

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

Skeleton during loading & in empty state are aligned differently compared to production

What is the root cause of that problem?

The margin of the skeleton view not properly set

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

Add proper margin here to make it consistent to the empty state

style={[themeStyles.flex1, themeStyles.overflowHidden, themeStyles.m2, themeStyles.mt3]}

OR

style={[themeStyles.flex1, themeStyles.overflowHidden, themeStyles.mt3, themeStyles.mb2, themeStyles.mr2, themeStyles.ml2]}

RESULT

New-Expensify.27.mp4

What alternative solutions did you explore? (Optional)

[trjExpensify]

@shawnborton confirming with you the correct expectation here so we can move this forward.

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-08-07 11:26:51 UTC.

Proposal

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

Search-Skeleton during loading & in empty state are aligned differently compared to production

What is the root cause of that problem?

• The correct margin for a row is 20px and the empty state spacing seems correct.
  

  App/src/components/SelectionList/Search/ReportListItem.tsx

  Line 74 in f9306d5

  styles.selectionListPressableItemWrapper,

  
  

  App/src/styles/index.ts

  Line 4631 in f9306d5

  marginHorizontal: 20,

• The margin horizontal applied to ItemListSkeletonView is incorrect, the margin should be 20px. But we
  

  App/src/components/Skeletons/SearchRowSkeleton.tsx

  Line 120 in f9306d5

  itemViewStyle={[styles.highlightBG, styles.mb3, styles.br3, styles.mr3, styles.ml3]}

  
  

  App/src/components/Skeletons/SearchRowSkeleton.tsx

  Line 38 in f9306d5

  itemViewStyle={[styles.highlightBG, styles.mb3, styles.br3, styles.mr3, styles.ml3]}

• We can change styles.mr3, styles.ml3 to styles.mh5 but now the empty state will have 8px extra padding because in EmptyStateComponent, the SkeletonComponent is wrapped with a view which hap 8px padding.
  

  App/src/components/EmptyStateComponent/index.tsx

  Lines 82 to 87 in f9306d5

  	<View style={styles.skeletonBackground}>
  	<SkeletonComponent
  	gradientOpacityEnabled
  	shouldAnimate={false}
  	/>
  	</View>

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

• We should change the horizontal margin to styles.mh5.
• We need to introduce a new prop in EmptyStateComponent to overwrite skeletonBackground style which has 8px of padding.
• In EmptySearchView use the new prop to remove the horizontal padding.
  

  App/src/pages/Search/EmptySearchView.tsx

  Line 14 in f9306d5

  <EmptyStateComponent

What alternative solutions did you explore? (Optional)

• Wrap SearchRowSkeleton with view which has 8px of horizontal padding.

App/src/components/Search/index.tsx

Line 226 in f9306d5

<SearchRowSkeleton shouldAnimate />

App/src/components/Search/index.tsx

Line 367 in f9306d5

<SearchRowSkeleton

                <View style={[styles.ph2, styles.flex1]}>
                    <SearchRowSkeleton shouldAnimate />
                </View>

[trjExpensify]

either way, this will be external. Adding the label to get a C+ assigned to move the deploy blocker along.

[melvin-bot]

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

[melvin-bot]

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

[trjExpensify]

^^ Not a web deploy blocker either, removed that label.

[Kicu]

@trjExpensify almost everything related to Search was done by me @WojtekBoman from SWM, and the newest iteration of skeleton empty views was implemented in here by @filip-solecki also from SWM.

So if you don't mind we could assign one our devs to this.
It doesn't look like a big bug, and in addition we can make sure empty "skeleton" views have correct padding in other places as well.

CC @blazejkustra

[Krishna2323]

Proposal Updated

• Added alternative

[shawnborton]

I had already reported this one to @filip-solecki and I believe @filip-solecki was planning on fixing it already?

[shawnborton]

Take a look at this comment @trjExpensify

[shawnborton]

Can we also address this particular comment too? I think that's the biggest visual glitch we need to solve.

[Guccio163]

Actually it is interesting case, because on your video @IuliiaHerets there is exactly the opposite situation: the staging has both horizontal and vertical switches and production has only horizontal ones. On @shawnborton 's video in this comment there is only horizontal variation too, I suppose that using staging app.

Staging:

Screen.Recording.2024-08-08.at.13.25.18.mov

Production:

Screen.Recording.2024-08-08.at.13.25.49.mov

I'll to dig right into it, but it's interesting that we have exactly opposite situations

[Guccio163]

Can we also address #44137 (comment)? I think that's the biggest visual glitch we need to solve.

@shawnborton you mean the horizontal alignment of Empty page right?

[shawnborton]

Yeah, when you jump from loading rows to the empty state, there is a difference in horizontal padding which causes the jumpiness. Both of your videos directly above illustrate this.

[Guccio163]

@shawnborton Yeah, I think this is exactly the case that I have to solve here, as a matter of fact, this was exactly what I was trying to confirm with @IuliiaHerets, though there's also the vertical jump (shows on production here and on staging in @IuliiaHerets's case) which makes a big difference and can mean smth more 🤔

[shawnborton]

Cool, I agree with you we should align on both the horizontal and vertical padding of both the loading rows and the empty state.

[Guccio163]

I just found out that in narrow view both staging and production has mismatched components' widths between Empty and Loading pages, I'll try to handle it in this PR too!

Staging narrow window:

Screen.Recording.2024-08-08.at.15.26.55.mov

Production narrow window:

Screen.Recording.2024-08-08.at.15.26.24.mov

[shawnborton]

Awesome, thanks for digging into this one!

[Guccio163]

@shawnborton @IuliiaHerets Also when I'm on it, what do you think about aligning Loading page with Results page (not empty)? Vertically it's normal that they will vary (Loading page doesn't have f.ex. filters), but I could try to match them to align horizontally. Then it would be about tweaking both Loading and Empty pages' widths to match Results page.

Here's video of the horizontal difference between Loading and Results pages.

Screen.Recording.2024-08-08.at.16.55.26.mov

[shawnborton]

Yes, I think ideally we should use the same horizontal padding between loading/empty state/results. Whatever we currently have for results should be the guide here I would think? Which I think is just 20px of horizontal padding around the rows.

[rayane-djouah]

PR #47164 up

[rayane-djouah]

PR is merged

[rayane-djouah]

PR on staging

[rayane-djouah]

PR deployed to production yesterday #47164 (comment)

[jliexpensify]

Thanks for the heads up!

Payment Summary:

• C+: @rayane-djouah $250

https://www.upwork.com/jobs/~011af2fbb0ab06fc24

[melvin-bot]

@luacmartins @jliexpensify @rayane-djouah @Guccio163 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!

[jliexpensify]

@rayane-djouah is a checklist needed for this?

[rayane-djouah]

Checklist

• The PR that introduced the bug has been identified. Link to the PR: Empty State Component for Search #44137
• 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: Author and reviewers are already aware: Empty State Component for Search #44137 (comment)
• 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: N/A
• Determine if we should create a regression test for this bug. Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Create a new workspace
2. Submit an expense in your workspace report
3. Go to settings > troubleshoot > clear cache and restart
4. Use a slow internet connection
5. Open the search page
6. Verify that the loading view skeleton has a 20px horizontal padding
7. When the results load, Verify that the results view has a 20px horizontal padding
8. Change the search filter to get an empty state view
9. Verify that the loading empty state view skeleton has a 20px horizontal padding
10. The horizontal padding should be consistent across the three views

Do we agree 👍 or 👎

cc @jliexpensify

[jliexpensify]

Paid and job closed