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

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

Closed
6 tasks done
IuliiaHerets opened this issue Aug 7, 2024 · 43 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Aug 7, 2024

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 OwnerCurrent Issue Owner: @jayeshmangwani
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels Aug 7, 2024
Copy link

melvin-bot bot commented Aug 7, 2024

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

Copy link

melvin-bot bot commented Aug 7, 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.

@IuliiaHerets
Copy link
Author

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Aug 7, 2024
Copy link
Contributor

github-actions bot commented Aug 7, 2024

👋 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
Copy link
Contributor

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
Copy link
Contributor

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

@Krishna2323
Copy link
Contributor

Krishna2323 commented Aug 7, 2024

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?

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.
    <EmptyStateComponent

What alternative solutions did you explore? (Optional)

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

<SearchRowSkeleton shouldAnimate />

<SearchRowSkeleton

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

@trjExpensify
Copy link
Contributor

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

@trjExpensify trjExpensify added the External Added to denote the issue can be worked on by a contributor label Aug 7, 2024
@melvin-bot melvin-bot bot changed the title Search-Skeleton during loading & in empty state are aligned differently compared to production [$250] Search-Skeleton during loading & in empty state are aligned differently compared to production Aug 7, 2024
Copy link

melvin-bot bot commented Aug 7, 2024

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

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

melvin-bot bot commented Aug 7, 2024

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

@trjExpensify trjExpensify removed the DeployBlocker Indicates it should block deploying the API label Aug 7, 2024
@trjExpensify
Copy link
Contributor

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

@Kicu
Copy link
Contributor

Kicu commented Aug 7, 2024

@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
Copy link
Contributor

Proposal Updated

  • Added alternative

@shawnborton
Copy link
Contributor

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

@shawnborton
Copy link
Contributor

Take a look at this comment @trjExpensify

Copy link

melvin-bot bot commented Aug 7, 2024

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

@Guccio163
Copy link
Contributor

Hi @IuliiaHerets, can you let me know if I understand the issue correctly?

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

Loading pages are exactly the same in staging and production, the problem is the alignment of Empty page. Loading page is mentioned, because we measure the Empty page's alignment towards it.

Production:

  • Empty page vary in both vertical and horizontal alignment

Staging:

  • Empty page is vertically aligned with Loading page, it vary only in (smaller than production, but not perfect) horizontal alignment

What we aim to achieve:

  • Perfectly aligning Empty page to the Loading page so that there's no 'glitch' between them

Comparison of Loading pages in staging and production (uploaded screenshot instead of recording because it is easier to capture):

vertical alignment:
Screenshot 2024-08-08 at 13 10 16

horizontal alignment:
Screenshot 2024-08-08 at 13 12 07

Comparison of Empty pages in staging and production:

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

cc @blazejkustra

@shawnborton
Copy link
Contributor

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

@Guccio163
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

Awesome, thanks for digging into this one!

@Guccio163
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

PR #47164 up

@melvin-bot melvin-bot bot removed the Overdue label Aug 12, 2024
@rayane-djouah
Copy link
Contributor

PR is merged

@rayane-djouah
Copy link
Contributor

PR on staging

@rayane-djouah
Copy link
Contributor

PR deployed to production yesterday #47164 (comment)

@melvin-bot melvin-bot bot added the Overdue label Aug 19, 2024
@jliexpensify jliexpensify changed the title [$250] Search-Skeleton during loading & in empty state are aligned differently compared to production [PAY AUG 22ND][$250] Search-Skeleton during loading & in empty state are aligned differently compared to production Aug 19, 2024
@jliexpensify
Copy link
Contributor

Thanks for the heads up!

Payment Summary:

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

@melvin-bot melvin-bot bot removed the Overdue label Aug 19, 2024
Copy link

melvin-bot bot commented Aug 21, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Aug 21, 2024
@jliexpensify
Copy link
Contributor

@rayane-djouah is a checklist needed for this?

@melvin-bot melvin-bot bot removed the Overdue label Aug 21, 2024
@rayane-djouah
Copy link
Contributor

rayane-djouah commented Aug 22, 2024

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
Copy link
Contributor

Paid and job closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
Status: Done
Development

No branches or pull requests