-
Notifications
You must be signed in to change notification settings - Fork 76
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
Page to add/edit transactions and account page #118
Conversation
Diplay icon background color based on category. Show amount in the correct color based on type. Totals of the day consider only income and expenses.
Rename to DetailsListTile. Remove redundant InkWell. Use named instead of positional arguments.
Break it down into separate widgets for legibility. Display the correct switch for recurring payments based on the platform. Rename "Notes" to "Label" and move it to the top (fixes RIP-Comm#98).
@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 |
@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. |
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 |
I noticed some strange behaviour when changing amount field (in update, but something broken also when creating new transactions), error.mp4 |
@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.
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 |
I've tried it and LGTM! Great work |
@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 |
I've tried with 100 transactions, and still not noticed any remarkable delay. |
LGTM, except known bugs it works fine. |
Cool! |
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.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.