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

[PAYMENT 12/25 regression] [$250] Feature Request: Implement to use a 👍icon next to approved report preview #49847

Open
6 tasks done
m-natarajan opened this issue Sep 27, 2024 · 48 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Sep 27, 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: n/a
Reproducible in staging?: n/a
Reproducible in production?: n/a
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: @youssef-lr
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1727409632797909

Action Performed:

  1. Have an approved expense report
  2. Open the report

Expected Result:

Should have some indication with icon for the report is approved

Actual Result:

Implement to use a 👍:skin-tone-3: icon next to approved report preview, similar to how we display a checkmark next to paid reports.

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

Screenshot 2024-07-12 at 18 10 44

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021839674890714199459
  • Upwork Job ID: 1839674890714199459
  • Last Price Increase: 2024-09-27
  • Automatic offers:
    • situchan | Reviewer | 104253538
    • Krishna2323 | Contributor | 104253539
    • DylanDylann | Contributor | 104775457
Issue OwnerCurrent Issue Owner: @slafortune
@m-natarajan m-natarajan added Weekly KSv2 NewFeature Something to build that is a new item. labels Sep 27, 2024
Copy link

melvin-bot bot commented Sep 27, 2024

Triggered auto assignment to @slafortune (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

Copy link

melvin-bot bot commented Sep 27, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

Copy link

melvin-bot bot commented Sep 27, 2024

Triggered auto assignment to Design team member for new feature review - @dannymcclain (NewFeature)

@m-natarajan m-natarajan changed the title Feature Request: Implement to use a 👍:skin-tone-3: icon next to approved report preview Feature Request: Implement to use a :+1 icon next to approved report preview Sep 27, 2024
@m-natarajan m-natarajan changed the title Feature Request: Implement to use a :+1 icon next to approved report preview Feature Request: Implement to use a 👍icon next to approved report preview Sep 27, 2024
@dannymcclain
Copy link
Contributor

Adding some design specs here for easier implementation:

  • The icon we want to use is our thumbs up icon—it can be found here: App/assets/images/thumbs-up.svg
  • The layout should copy what we already do for paid reports, just instead of a green checkmark icon, we want to use an icons colored thumbs up icon

Mockup:
CleanShot 2024-09-27 at 08 44 10@2x

cc @dubielzyk-expensify

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Sep 27, 2024
@JmillsExpensify
Copy link

Thanks @dannymcclain! That looks solid.

@Krishna2323
Copy link
Contributor

Krishna2323 commented Sep 27, 2024

Edited by proposal-police: This proposal was edited at 2024-09-27 14:42:56 UTC.

Proposal


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

Feature Request: Implement to use a 👍icon next to approved report preview

What is the root cause of that problem?

New feature

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


  • First we need to use isReportApproved to check if the report is approved or not. We are already using isReportApproved in ReportPreview, so we don't need to create a new variable.

    App/src/libs/ReportUtils.ts

    Lines 892 to 901 in 0856034

    /**
    * Checks if the supplied report has been approved
    */
    function isReportApproved(reportOrID: OnyxInputOrEntry<Report> | string, parentReportAction: OnyxEntry<ReportAction> = undefined): boolean {
    const report = typeof reportOrID === 'string' ? ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportOrID}`] ?? null : reportOrID;
    if (!report) {
    return parentReportAction?.childStateNum === CONST.REPORT.STATE_NUM.APPROVED && parentReportAction?.childStatusNum === CONST.REPORT.STATUS_NUM.APPROVED;
    }
    return report?.stateNum === CONST.REPORT.STATE_NUM.APPROVED && report?.statusNum === CONST.REPORT.STATUS_NUM.APPROVED;
    }
  • Then we can use Expensicons.ThumbsUp with specified color code and use it just like we have implemented for settled report
    {iouSettled && (
    <Animated.View style={checkMarkStyle}>
    <Icon
    src={Expensicons.Checkmark}
    fill={theme.iconSuccessFill}
    />
    </Animated.View>
    )}
  • We can create a new animation or we can use the similar animation used for paid icon.
  • We also need to make similar code changes in MoneyReportView & MoneyRequestPreview. We can also consider using checks like !isPartialHold & !isBillSplit
    {isSettled && !isPartiallyPaid && (
    <View style={[styles.defaultCheckmarkWrapper, styles.mh2]}>
    <Icon
    src={Expensicons.Checkmark}
    fill={theme.success}
    />
    </View>
    )}

    {ReportUtils.isSettled(iouReport?.reportID) && !isPartialHold && !isBillSplit && (
    <View style={styles.defaultCheckmarkWrapper}>
    <Icon
    src={Expensicons.Checkmark}
    fill={theme.iconSuccessFill}
    />
    </View>
    )}

What alternative solutions did you explore? (Optional)

Result

@Nodebrute
Copy link
Contributor

Nodebrute commented Sep 27, 2024

Edited by proposal-police: This proposal was edited at 2024-09-27 15:15:07 UTC.

Proposal

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

Implement to use a 👍icon next to approved report preview

What is the root cause of that problem?

Feature request

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

If we want to show animation for thumbsUp similar to CheckMark we can do something like this in ReportPreview.tsx

   const ThumbsUpScale = useSharedValue(isApproved ? 1 : 0);
    const ThumbsUpStyle = useAnimatedStyle(() => ({
        ...styles.defaultCheckmarkWrapper,
        transform: [{scale: ThumbsUpStyle.value}],
    }));

And then we need to create a useEffect to give it spring-based Animation. We can pass the duration of our choice. I am passing 200 here the default is 2000

    useEffect(() => {
        if (!isApproved) {
            return;
        }

        checkMarkScale1.value = withSpring(1,{duration: 200});

    }, [isApproved]);

Lastly, here We can use the thumbsUp icon

 {isApproved && (
                                                    <Animated.View style={checkMarkStyle1}>
                                                        <Icon
                                                            src={Expensicons.ThumbsUp}
                                                            fill={theme.icon}
                                                        />
                                                    </Animated.View>
                                                )}

Other minor styling can be adjusted in the PR

Screen.Recording.2024-09-30.at.3.29.14.PM.mov

What alternative solutions did you explore? (Optional)

@slafortune slafortune added the External Added to denote the issue can be worked on by a contributor label Sep 27, 2024
@melvin-bot melvin-bot bot changed the title Feature Request: Implement to use a 👍icon next to approved report preview [$250] Feature Request: Implement to use a 👍icon next to approved report preview Sep 27, 2024
Copy link

melvin-bot bot commented Sep 27, 2024

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

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

melvin-bot bot commented Sep 27, 2024

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

@Krishna2323
Copy link
Contributor

Proposal Updated

  • Proposal updated to include similar places which requires similar code changes.

@situchan
Copy link
Contributor

Should we animate 👍 similar to ✔️ in #49438?
cc: @Expensify/design

@Nodebrute
Copy link
Contributor

Proposal Updated
Added Implementation if we want to Animate

@dannymcclain
Copy link
Contributor

Yeah I think we should animate the 👍 if possible! The transition to the approved state I think will typically be pretty subtle, so a little animation would be good IMO.

cc @dubielzyk-expensify in case he disagrees.

@dubielzyk-expensify
Copy link
Contributor

100% agree. That'd be great!

@situchan
Copy link
Contributor

situchan commented Oct 1, 2024

@Nodebrute can you please elaborate the meaningful difference from previous proposal?
And now that we want to animate, please remove deprecated proposal which is confusing.

@Nodebrute
Copy link
Contributor

@situchan I've removed the outdated code. Since this is a feature request, I believe the selected proposal should be more thoroughly researched. Ultimately, it’s your decision, and I'm happy with whoever you choose.

@DylanDylann
Copy link
Contributor

@youssef-lr @slafortune I can help to complete the PR

Copy link

melvin-bot bot commented Nov 6, 2024

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

@slafortune
Copy link
Contributor

Thanks for volunteering @DylanDylann

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 22, 2024
@melvin-bot melvin-bot bot changed the title [$250] Feature Request: Implement to use a 👍icon next to approved report preview [HOLD for payment 2024-11-29] [$250] Feature Request: Implement to use a 👍icon next to approved report preview Nov 22, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 22, 2024
Copy link

melvin-bot bot commented Nov 22, 2024

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

Copy link

melvin-bot bot commented Nov 22, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.65-5 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-11-29. 🎊

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

Copy link

melvin-bot bot commented Nov 22, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@DylanDylann] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@slafortune] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@Krishna2323
Copy link
Contributor

Krishna2323 commented Nov 25, 2024

The PR was reverted due to causing #52873. I will open a new PR by the end of tomorrow.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 29, 2024
@DylanDylann
Copy link
Contributor

@Krishna2323 Let's prioritize this PR before creating new proposals

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

I tried yesterday but failed to build the app on my physical device and the bug only appears on physical android device. Will try again in a moment.

Copy link

melvin-bot bot commented Dec 2, 2024

@dannymcclain, @youssef-lr, @slafortune, @Krishna2323, @DylanDylann Whoops! This issue is 2 days overdue. Let's get this updated quick!

@slafortune slafortune changed the title [HOLD for payment 2024-11-29] [$250] Feature Request: Implement to use a 👍icon next to approved report preview [HOLD for payment ?regression] [$250] Feature Request: Implement to use a 👍icon next to approved report preview Dec 3, 2024
Copy link

melvin-bot bot commented Dec 4, 2024

@dannymcclain, @youssef-lr, @slafortune, @Krishna2323, @DylanDylann Eep! 4 days overdue now. Issues have feelings too...

@slafortune
Copy link
Contributor

not overdue, waiting on PR

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Overdue Daily KSv2 labels Dec 4, 2024
@slafortune
Copy link
Contributor

deployed to production today

@slafortune slafortune changed the title [HOLD for payment ?regression] [$250] Feature Request: Implement to use a 👍icon next to approved report preview [PAYMENT 12/25 regression] [$250] Feature Request: Implement to use a 👍icon next to approved report preview Dec 18, 2024
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 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Status: Polish
Development

No branches or pull requests

10 participants