Skip to content

[HOLD for payment 2024-05-20] [$250] Skeleton loader uses lighter color for page header #40244

Closed

Description

@m-natarajan

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: 1.4.62-4
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1713166267469379

Action Performed:

  1. Open a chat which will show a skeleton loader
  2. Look at the header of the chat report

Expected Result:

The skeleton loader for the top page/chat header should use the same color background as the skeleton loader for the chat messages in the main content screen.

Actual Result:

The skeleton loader used in the top page/chat header is using slightly lighter colors

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

Add any screenshot/video evidence

CleanShot 2024-04-15 at 09 29 33@2x
Untitled

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0199d049e1997c5cb7
  • Upwork Job ID: 1779899140122308608
  • Last Price Increase: 2024-04-15
  • Automatic offers:
    • abdulrahuman5196 | Reviewer | 0
    • GandalfGwaihir | Contributor | 0
Issue OwnerCurrent Issue Owner: @twisterdotcom

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Apr 15, 2024
  2. melvin-bot commented on Apr 15, 2024

    @melvin-bot

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

  3. allgandalf commented on Apr 15, 2024

    @allgandalf
    Contributor

    Proposal

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

    Loading lines colour of header and report doesn't match

    What is the root cause of that problem?

    Currently we have the lines colour as theme.highlightBG this gives us faint lines

    <SkeletonViewContentLoader
    animate={shouldAnimate}
    width={styles.w100.width}
    height={height}
    backgroundColor={theme.highlightBG}
    foregroundColor={theme.border}
    >

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

    We need to use skeletonLHNIn as bg colour and theme.skeletonLHNOut as foreground colour.

    We do the same for report screen:

    <SkeletonViewContentLoader
    animate={shouldAnimate}
    height={CONST.CHAT_SKELETON_VIEW.HEIGHT_FOR_ROW_COUNT[numberOfRows]}
    backgroundColor={theme.skeletonLHNIn}
    foregroundColor={theme.skeletonLHNOut}
    style={styles.mr5}
    >

    The colour of these styles for dark mode is :

    skeletonLHNIn: colors.productDark400,
    skeletonLHNOut: colors.productDark600,

    And for light mode is:

    skeletonLHNIn: colors.productLight400,
    skeletonLHNOut: colors.productLight600,

    This should make both the report as well as the report header consistent in loading colour

    What alternative solutions did you explore? (Optional)

  4. twisterdotcom commented on Apr 15, 2024

    @twisterdotcom
    Contributor

    Ahh:

    Issue reported by: @shawnborton

    Gonna make external.

  5. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Apr 15, 2024
  6. changed the title [-]Skeleton loader uses lighter color for page header[/-] [+][$250] Skeleton loader uses lighter color for page header[/+] on Apr 15, 2024
  7. melvin-bot commented on Apr 15, 2024

    @melvin-bot
  8. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Apr 15, 2024
  9. melvin-bot commented on Apr 15, 2024

    @melvin-bot

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

  10. mananjadhav commented on Apr 15, 2024

    @mananjadhav
    Collaborator

    I was too slow in posting a proposal. 🐌

  11. allgandalf commented on Apr 15, 2024

    @allgandalf
    Contributor

    I was too slow in posting a proposal. 🐌

    😆 godspeed

  12. dragnoir commented on Apr 15, 2024

    @dragnoir
    Contributor

    Proposal

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

    Skeleton loader uses lighter color for page header.

    What is the root cause of that problem?

    On the hedare we are using ReportHeaderSkeletonView for the loading animation
    But on the report we use ReportActionsSkeletonView

    Those boath components uses the same SkeletonViewContentLoader with different parameters
    here:

    <SkeletonViewContentLoader
    animate={shouldAnimate}
    width={styles.w100.width}
    height={height}
    backgroundColor={theme.highlightBG}
    foregroundColor={theme.border}

    and here:

    <SkeletonViewContentLoader
    animate={shouldAnimate}
    height={CONST.CHAT_SKELETON_VIEW.HEIGHT_FOR_ROW_COUNT[numberOfRows]}
    backgroundColor={theme.skeletonLHNIn}
    foregroundColor={theme.skeletonLHNOut}
    style={styles.mr5}

    Also there's an issue with the alignment of the animation

    image

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

    1- to fix the colors:
    we shoudl use the same backgroundColor and foregroundColor

    backgroundColor={theme.skeletonLHNIn}
    foregroundColor={theme.skeletonLHNOut}

    2- align the header animation same as report animation
    we should remove the padding left here

    <View style={[styles.appContentHeaderTitle, !isSmallScreenWidth && styles.pl5]}>

    POC:
    image

  13. 40 remaining items

  14. melvin-bot commented on Apr 29, 2024

    @melvin-bot

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

  15. allgandalf commented on May 3, 2024

    @allgandalf
    Contributor

    Friendly bump for PR review @marcaaron

  16. allgandalf commented on May 6, 2024

    @allgandalf
    Contributor

    friendly bump @marcochavezf for PR review

  17. added
    Awaiting PaymentAuto-added when associated PR is deployed to production
    and removed on May 13, 2024
  18. changed the title [-][$250] Skeleton loader uses lighter color for page header[/-] [+][HOLD for payment 2024-05-20] [$250] Skeleton loader uses lighter color for page header[/+] on May 13, 2024
  19. melvin-bot commented on May 13, 2024

    @melvin-bot

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

  20. melvin-bot commented on May 13, 2024

    @melvin-bot

    The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.72-1 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-05-20. 🎊

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

  21. melvin-bot commented on May 13, 2024

    @melvin-bot

    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:

    • [@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR:
    • [@abdulrahuman5196] 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:
    • [@abdulrahuman5196] 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:
    • [@abdulrahuman5196] Determine if we should create a regression test for this bug.
    • [@abdulrahuman5196] 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.
    • [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
  22. twisterdotcom commented on May 20, 2024

    @twisterdotcom
    Contributor

    Payment Summary:

    I dont' think this was actually an introduced bug right, but it was actually a new feature. It never looked this way before right? We just got our greys wrong originally. I'm also not sure we can do regression testing for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions