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

Improvements to "Transactions" page #84

Merged
merged 5 commits into from Apr 18, 2023
Merged

Conversation

GBergatto
Copy link
Contributor

I improved the List and Categories tabs. Here are two screenshots of how they look now.

Currently, all transactions are queried from the database regardless of their date. We should query only the ones in the selected time frame. Also, the totals for the day and for each category are calculated in the build method of the widgets, which is not ideal. It might be better to retrieve them directly from the database with a designated query. The same goes for the groupings of transactions in their categories in the "Categories" tab.

The list of expandable categories in the "Categories" tab is wrapped in a Sizebox whose size is calculated every time a category is expanded. While a category gets collapsed, a RenderFlex overflowed is sometimes thrown, because the new size is computed before the end of the animation. After the animation is over, everything is fine.
If you can think of a better solution, please tell me.

What's missing

  • bank account and category of the transaction in the "Categories" tab are shown as numbers instead of names
  • colors of the categories for icons and pie chart (still missing from database)
  • more specific queries and more efficient calculation of totals

Calculate total for each day
Get category and bank account for each transaction
Get categories and transactions from database.
Group transactions under their category and calculate total.
Calculate percentage for each category.
Copy link
Collaborator

@FedericoBruzzone FedericoBruzzone left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, in addition to the error already shown on discord, I noticed that some comments are written with lowercase letter. In the moment when you'll update the PR, please change this too.

@GBergatto
Copy link
Contributor Author

Thanks for this PR, in addition to the error already shown on discord, I noticed that some comments are written with lowercase letter. In the moment when you'll update the PR, please change this too.

The error shown on discord is caused by a typing error at line 149 of the file lib/database/sossoldi_database.dart

(14, "Leisure", "subscriptions", '', null, '${DateTime.now()}', '${DateTime.now()}');
15, "Salary", "work", '', null, '${DateTime.now()}', '${DateTime.now()}');

The semicolon at the end of the line should be a comma.

I have fixed this on my local fork but haven't committed the change as I thought it would have been out of scope for this PR. @lucaantonelli said he was fixing a few issues with the database.

If you want me to, I can make all comments start with uppercase letters. However, I think it would be a bit overkill, considering that, as far as I know, no convention about comments has been established. (Maybe we should establish one now)

@mikev-cw
Copy link
Collaborator

Thanks for this PR, in addition to the error already shown on discord, I noticed that some comments are written with lowercase letter. In the moment when you'll update the PR, please change this too.

The error shown on discord is caused by a typing error at line 149 of the file lib/database/sossoldi_database.dart

(14, "Leisure", "subscriptions", '', null, '${DateTime.now()}', '${DateTime.now()}');
15, "Salary", "work", '', null, '${DateTime.now()}', '${DateTime.now()}');

The semicolon at the end of the line should be a comma.

I have fixed this on my local fork but haven't committed the change as I thought it would have been out of scope for this PR. @lucaantonelli said he was fixing a few issues with the database.

If you want me to, I can make all comments start with uppercase letters. However, I think it would be a bit overkill, considering that, as far as I know, no convention about comments has been established. (Maybe we should establish one now)

Agreed.
Just fixed the typo with a direct commit.

@mikev-cw mikev-cw merged commit 7ff5ee6 into RIP-Comm:main Apr 18, 2023
1 check passed
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.

None yet

3 participants