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

[$500] Add Empty State Background To Tasks #27495

Closed
grgia opened this issue Sep 15, 2023 · 36 comments
Closed

[$500] Add Empty State Background To Tasks #27495

grgia opened this issue Sep 15, 2023 · 36 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@grgia
Copy link
Contributor

grgia commented Sep 15, 2023

We're missing the empty state background in Task reports. Follow up from #27478

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f8fb00801e6f1236
  • Upwork Job ID: 1702572656769077248
  • Last Price Increase: 2023-10-11
  • Automatic offers:
    • aimane-chnaif | Reviewer | 27346707
    • Pujan92 | Contributor | 27346708
@grgia grgia added Weekly KSv2 Internal Requires API changes or must be handled by Expensify staff labels Sep 15, 2023
@grgia grgia self-assigned this Sep 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 15, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 15, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @aimane-chnaif (Internal)

@melvin-bot melvin-bot bot added the Overdue label Sep 25, 2023
@grgia
Copy link
Contributor Author

grgia commented Sep 25, 2023

not overdue

@melvin-bot melvin-bot bot removed the Overdue label Sep 25, 2023
@melvin-bot melvin-bot bot added the Overdue label Oct 3, 2023
@grgia grgia added External Added to denote the issue can be worked on by a contributor and removed Internal Requires API changes or must be handled by Expensify staff labels Oct 4, 2023
@melvin-bot melvin-bot bot changed the title Add Empty State Background To Tasks [$500] Add Empty State Background To Tasks Oct 4, 2023
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

Current assignee @aimane-chnaif is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 Overdue labels Oct 4, 2023
@grgia
Copy link
Contributor Author

grgia commented Oct 4, 2023

@aimane-chnaif making external

@Pujan92
Copy link
Contributor

Pujan92 commented Oct 4, 2023

Proposal

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

Add empty state background for tasks report

What is the root cause of that problem?

We do not have that logic currently in the TaskView component.

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

Replicate the same logic for TaskReport in the ReportActionItem which we have for MoneyRequestView component for showing the background image.

if (ReportUtils.isTaskReport(props.report)) {
if (ReportUtils.isCanceledTaskReport(props.report, parentReportAction)) {
content = (
<>
<ReportActionItemSingle
action={parentReportAction}
showHeader={!props.draftMessage}
wrapperStyles={[styles.chatItem]}
report={props.report}
>
<RenderHTML html={`<comment>${props.translate('parentReportAction.deletedTask')}</comment>`} />
</ReportActionItemSingle>
<View style={styles.reportHorizontalRule} />
</>
);
} else {
content = (
<TaskView
report={props.report}
shouldShowHorizontalRule={!props.shouldHideThreadDividerLine}
/>
);
}
}

For that, we can create one more variable and assign that code. With that use that variable in content with both(if/else) conditions.

let emptyStateBackground = (
    <View style={[StyleUtils.getReportWelcomeContainerStyle(isSmallScreenWidth), StyleUtils.getMinimumHeight(CONST.EMPTY_STATE_BACKGROUND.MONEY_REPORT.MIN_HEIGHT)]}> 
         <AnimatedEmptyStateBackground /> 
    </View>
)
Screenshot 2023-10-04 at 18 38 43

What alternative solutions did you explore?

<View style={[StyleUtils.getReportWelcomeContainerStyle(isSmallScreenWidth), StyleUtils.getMinimumHeight(CONST.EMPTY_STATE_BACKGROUND.MONEY_REPORT.MIN_HEIGHT)]}>
<AnimatedEmptyStateBackground />
</View>

As for canceled task we are not rendering TaskView, we need to add the same snippet here before the ReportActionItemSingle.

if (ReportUtils.isCanceledTaskReport(props.report, parentReportAction)) {
content = (
<>
<ReportActionItemSingle
action={parentReportAction}
showHeader={!props.draftMessage}
wrapperStyles={[styles.chatItem]}
report={props.report}
>
<RenderHTML html={`<comment>${props.translate('parentReportAction.deletedTask')}</comment>`} />
</ReportActionItemSingle>
<View style={styles.reportHorizontalRule} />
</>
);
} else {
content = (
<TaskView
report={props.report}
shouldShowHorizontalRule={!props.shouldHideThreadDividerLine}

@aimane-chnaif
Copy link
Contributor

aimane-chnaif commented Oct 4, 2023

@Pujan92 thanks for the proposal. Let's make sure to test on various cases including subtask, thread task, cancelled/deleted task, very long description, scrolling on mobile, offline.

@Pujan92
Copy link
Contributor

Pujan92 commented Oct 5, 2023

Thanks, I will check and update based on that.

@Pujan92
Copy link
Contributor

Pujan92 commented Oct 5, 2023

@aimane-chnaif I have updated the proposal a bit and here are the screencasts.

a1.mov
Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-10-05.at.19.22.05.mp4

@melvin-bot melvin-bot bot added the Overdue label Oct 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 10, 2023

@grgia, @aimane-chnaif Huh... This is 4 days overdue. Who can take care of this?

@aimane-chnaif
Copy link
Contributor

aimane-chnaif commented Oct 10, 2023

@Pujan92 did you pass test on #27495 (comment)?

@melvin-bot melvin-bot bot removed the Overdue label Oct 10, 2023
@Pujan92
Copy link
Contributor

Pujan92 commented Oct 10, 2023

Yes @aimane-chnaif

@melvin-bot
Copy link

melvin-bot bot commented Oct 11, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@aimane-chnaif

This comment was marked as outdated.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 24, 2023
@grgia
Copy link
Contributor Author

grgia commented Oct 24, 2023

Assigned @Pujan92 !

@melvin-bot
Copy link

melvin-bot bot commented Oct 24, 2023

📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

@melvin-bot melvin-bot bot removed the Overdue label Oct 24, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 24, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Oct 26, 2023
@aimane-chnaif
Copy link
Contributor

@Pujan92 what is ETA?

@melvin-bot melvin-bot bot removed the Overdue label Oct 26, 2023
@Pujan92
Copy link
Contributor

Pujan92 commented Oct 27, 2023

@Pujan92 what is ETA?

Working on it and will raise a PR within an hour.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Oct 27, 2023
@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Nov 20, 2023
Copy link

melvin-bot bot commented Nov 20, 2023

This issue has not been updated in over 15 days. @Pujan92, @grgia, @aimane-chnaif eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@aimane-chnaif
Copy link
Contributor

@grgia can you please add Bug label and move to Daily? Thanks

@grgia
Copy link
Contributor Author

grgia commented Nov 28, 2023

@aimane-chnaif I think this was fixed and we can close

@aimane-chnaif
Copy link
Contributor

@grgia yes, only payment is left

@grgia grgia added the Bug Something is broken. Auto assigns a BugZero manager. label Nov 28, 2023
Copy link

melvin-bot bot commented Nov 28, 2023

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Monthly KSv2 labels Nov 28, 2023
Copy link

melvin-bot bot commented Nov 28, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@stephanieelliott
Copy link
Contributor

stephanieelliott commented Dec 4, 2023

Summarizing payment on this issue:

  • Issue reporter: N/A
  • Contributor: @Pujan92 $500, PAID via Upwork
  • Contributor+: @aimane-chnaif, PAID via Upwork

Upwork job is here, [X] bonus is included on final payout

@aimane-chnaif can you please complete the BZ checklist above? Once completed I will issue payment. Thanks!

@aimane-chnaif
Copy link
Contributor

I think we can skip checklist like #27477. This is not bug but kind of a small feature request

@stephanieelliott
Copy link
Contributor

Sounds good to me - all paid !

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 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

4 participants