RIP-Comm / sossoldi

"Sossoldi" is a wealth management / personal finance / Net Worth tracking app, made with Flutter.
MIT License
262 stars 73 forks source link

Page to add/edit transactions and account page #118

Closed GBergatto closed 8 months ago

GBergatto commented 10 months ago

I've transformed the modal used to create/add transactions into a page (fix #90), called AddPage. I've also put the text for the label right after the price to fix #98. Finally, I've made some improvements to the list of transactions in the "List" tab of the "Transactions" page: the daily totals are now correct and the icons are colored based on the category of the transaction.

Screenshot_20231005_085710.jpg

In addition, I've transformed the modal that used to open when clicking on the cards in the "Your accounts" section of the homepage into a page. However, I haven't added the list of transactions to it, yet, and the total amount displayed on the page doesn't match the one on the card.

WARNING: Clicking on the transactions in the "Last transactions" section of the page currently opens a blank modal.

I haven't edited this section because my plan is to replace it with the lib/pages/transactions_page/widgets/list_tab.dart widget, after having made it more versatile so that it can be reused into different parts of the app. This include adding a way to filter transactions by account (to use it in the account page) and to limit the number of transactions displayed (to use it in "Last transactions" on the homepage). This is in line with #108 about reusing widgets.

If that's fine with you, I would like to merge this PR to keep momentum and avoid big merge conflicts down the line. Next, I'll work on fixing the issues mentioned.

lucaantonelli commented 9 months ago

@GBergatto I've checked the PR, the new add transaction page looks good, but update fail since use the old modal page, we should update the onTap at row 189 showModalBottomSheet(... in _custom_widgets\transactionslist.dart , just changing row 188 to that should do the work (removing the modal code block): ref.read(transactionsProvider.notifier).transactionUpdateState().whenComplete(() => Navigator.of(context).pushNamed("/add-page"));

GBergatto commented 9 months ago

@lucaantonelli I've just applied the changes you suggested. Now, clicking on a transaction list tile on the homepage will open the page to edit it instead of the empty modal.

GBergatto commented 9 months ago

There's a noticeable delay when trying to switch to the homepage from another page. My guess is that having to load data for the graph, account sums, and transactions slows things down. Maybe this should be investigated in more details in the future

lucaantonelli commented 9 months ago

I noticed some strange behaviour when changing amount field (in update, but something broken also when creating new transactions), Apart from that LGTM.

https://github.com/RIP-Comm/sossoldi/assets/45290704/3f0eb3ea-abcb-4445-aa44-3433a9748a4e

GBergatto commented 9 months ago

@lucaantonelli It has something to do with the decimal separator switching from comma to dot. I'll look into it

mikev-cw commented 9 months ago

@lucaantonelli It has something to do with the decimal separator switching from comma to dot. I'll look into it

@rcasula worked (hardly 🤣) on that months ago. See if he can possibly help, if you need.

There's a noticeable delay when trying to switch to the homepage from another page. My guess is that having to load data for the graph, account sums, and transactions slows things down. Maybe this should be investigated in more details in the future

Not detected honestly. Please explain what "noticeable" means to you.

PS: If you're gonna add other commits, edit this line to "3.13" for passing tests

rcasula commented 9 months ago

I've tried it and LGTM! Great work

GBergatto commented 9 months ago

@mikev-cw I checked and I don't think it has something to do with the decimal separator (fortunately) because it happens even when the separator doesn't change. I guess it's caused by the way we update the provider.

About the delay, it's last just a split second. If you want I can send you a screen recording of it. Consider that I've edited lib/providers/transactions_provider.dart to change the hard-coded limit of transactions from 5 to 50. So, maybe that has an impact on it.

mikev-cw commented 8 months ago

About the delay, it's last just a split second. If you want I can send you a screen recording of it. Consider that I've edited lib/providers/transactions_provider.dart to change the hard-coded limit of transactions from 5 to 50. So, maybe that has an impact on it.

I've tried with 100 transactions, and still not noticed any remarkable delay. If you want, after fixing that strange behavior with the amount field (and resolve new conflicts, sorry for that!), send us a screen recording, and we'll try to go deeper on that!

lucaantonelli commented 8 months ago

LGTM, except known bugs it works fine.

mikev-cw commented 8 months ago

Cool! Let's merge