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

Feature/budget #158

Merged
merged 11 commits into from
Mar 27, 2024
Merged

Feature/budget #158

merged 11 commits into from
Mar 27, 2024

Conversation

K-w-e
Copy link
Contributor

@K-w-e K-w-e commented Feb 20, 2024

I'm sending you the pull request for the implementation of the Budgets feature, just to keep you informed about the direction I took during development.

Some points of attention already known:

  • I removed the "Add category budget" button from the planning page. Now, if you want to modify the budgets, you need to press "Manage." (I did this for both simplicity and because the flow to follow by pressing that button was not clear).
  • I added the ability to delete the budget via swipe. In the UI, there was both that option and the option through a "-" button next to the budget. We need to decide which one to keep.
  • When you add a new budget or modify an existing one, provide the option to select only the categories that are not already present. This should be easy to fix, but I haven't found a clean way to do it yet.

For any doubts, questions, or suggestions, feel free to reach out!

Copy link
Collaborator

@mikev-cw mikev-cw left a comment

Choose a reason for hiding this comment

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

Ok, that's very cool!!!
I just made some minor notes, but I've nothing to say about the overall approach.
Also keep an eye to those brand new conflicts.

About this:

I added the ability to delete the budget via swipe. In the UI, there was both that option and the option through a "-" button next to the budget. We need to decide which one to keep.

It seems that in the design file @federicopozzato has planned kind of "edit mode", where user can tap the "-" button to delete a budget, but since you can in fact edit all about budgets when you press "Manage", actually having an extra edit mode can be a bit redundant at this stage?

lib/model/budget.dart Outdated Show resolved Hide resolved
lib/pages/planning_page/manage_budget_page.dart Outdated Show resolved Hide resolved
lib/pages/planning_page/planning_page.dart Outdated Show resolved Hide resolved
lib/pages/planning_page/widget/budget_card.dart Outdated Show resolved Hide resolved
@mikev-cw mikev-cw marked this pull request as draft February 26, 2024 18:35
@federicopozzato
Copy link
Contributor

@mikev-cw @K-w-e thanks for pointing out those things!
The redundancy of the actions has been done on purpose, to give the user more freedom, even though I understand what you say about too many actions, and you may be right.
For semplicity let's keep it in the way you did, it's fine!

@K-w-e K-w-e marked this pull request as ready for review March 14, 2024 21:14
Copy link
Collaborator

@theperu theperu left a comment

Choose a reason for hiding this comment

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

I really like the implementation, good job! I left a couple of comments for minor changes but apart from that it seems fine to me.
Since you have already done the work on the budgets could you also work on the graphs that are at the bottom of the Dashboard next?

FYI I just noticed that there are also some conflicts that need to be resolved before we can merge

lib/pages/planning_page/manage_budget_page.dart Outdated Show resolved Hide resolved
@K-w-e
Copy link
Contributor Author

K-w-e commented Mar 17, 2024

PR updated!
That file with conflicts is caused by the fact that that widget has been rearranged and almost completely modified, I don't think I can resolve it myself. You should accept the changes from my branch through the PR.

Let me know if i can help

@theperu

@mikev-cw mikev-cw merged commit 2b2e877 into RIP-Comm:main Mar 27, 2024
1 check passed
@mikev-cw mikev-cw linked an issue Mar 27, 2024 that may be closed by this pull request
@theperu theperu mentioned this pull request Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Budgets feature
4 participants