Skip to content
This repository has been archived by the owner on Nov 5, 2024. It is now read-only.

feat: Add "Total Remaining Budget" display #3368

Merged
merged 4 commits into from
Jul 26, 2024
Merged

feat: Add "Total Remaining Budget" display #3368

merged 4 commits into from
Jul 26, 2024

Conversation

akashs056
Copy link
Contributor

@akashs056 akashs056 commented Jul 25, 2024

Pull request (PR) checklist

Please check if your pull request fulfills the following requirements:

  • I've read the Contribution Guidelines and my PR doesn't break the rules.
  • I've read and understand the Developer Guidelines.
  • I confirm that I've run the code locally and everything works as expected.
  • My PR includes only the necessary changes to fix the issue (i.e., no unnecessary files or lines of code are changed).
  • 🎬 I've attached a screen recording of using the new code to the next paragraph (if applicable).

Screen recording of interacting with your changes:

WhatsApp.Video.2024-07-25.at.18.39.48_d1b07eed.mp4

What's changed?

  • Added display for "Total Remaining Budget" beneath budget information.
  • Calculated remaining budget for categorized and multi-categorized budgets.
  • Included multi-language support for the "Total Remaining Budget" text.
  • Enhanced user experience by providing a clear overview of remaining budget without manual summarization.

Does this PR close any GitHub issues? (do not delete)

Added display for "Total Remaining Budget" beneath budget information.
Calculated remaining budget for categorized and multi-categorized budgets.
Included multi-language support for the "Total Remaining Budget" text.
Enhanced user experience by providing a clear overview of remaining budget without manual summarization.
Copy link
Collaborator

@ILIYANGERMANOV ILIYANGERMANOV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, a few changes:

  • use String? for the UI state of the new text
  • do the calculation and formatting in the VM

This is needed for performance and architecture reasons.

@ILIYANGERMANOV
Copy link
Collaborator

Ping me for re-review when you've applied the changes

@ILIYANGERMANOV ILIYANGERMANOV added the requested changes Changes are needed. Please, apply them label Jul 25, 2024
@akashs056
Copy link
Contributor Author

@ILIYANGERMANOV applied the changes. please re-review it

@akashs056 akashs056 requested a review from ILIYANGERMANOV July 26, 2024 09:00
Copy link
Collaborator

@ILIYANGERMANOV ILIYANGERMANOV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unnecessary state, fix Detekt and we can merge

@akashs056 akashs056 requested a review from ILIYANGERMANOV July 26, 2024 09:42
@akashs056
Copy link
Contributor Author

its ready to merge

@ILIYANGERMANOV ILIYANGERMANOV merged commit a887286 into Ivy-Apps:main Jul 26, 2024
9 checks passed
@akashs056 akashs056 deleted the fix-issue-3365 branch July 26, 2024 10:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
requested changes Changes are needed. Please, apply them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TOTAL REMAINING BUDGET]
2 participants