Skip to content
This repository was archived by the owner on Jul 27, 2025. It is now read-only.

Conversation

@D-Gaspa
Copy link
Contributor

@D-Gaspa D-Gaspa commented Apr 11, 2025

Closes #2016

This PR fixes a bug where transaction forms were showing all categories regardless of transaction type (income or expense).

Changes

  • Fixed the category dropdown to only show relevant categories:
    • Income categories when creating income transactions
    • Expense categories when creating expense transactions
  • Created a shared transaction type tabs component for consistent navigation between expense, income, and transfer forms
  • Fixed issue where navigating from the transfer form to the income form wasn't working correctly
  • Enhanced Account::TransactionsController to filter categories appropriately
  • Fixed minor formatting and whitespace issues in some files

Implementation Details

The implementation filters categories in the controller layer while maintaining a clean user interface for navigating between different transaction types. By moving the filtering logic to the controller and creating a shared navigation component, we've addressed the issue while also maintaining code quality.

Testing

I've manually tested the changes to verify:

  • Only income categories appear when creating income transactions
  • Only expense categories appear when creating expense transactions
  • Navigation between the different forms works correctly
separate-income-expense_proof.mp4

Note: There were some failing tests in the suite when running the tests on my dev container, but these failures are also present on the main branch before doing any changes; therefore I assume they are unrelated to the changes in this PR.

For Discussion: Potential UI Improvements

While working on this fix, I noticed some opportunities for further improving the transaction forms:

  1. Enhanced Category Dropdown: Currently, the category dropdown shows plain text names. Would it make sense to display categories with their corresponding colors and icons (similar to how they appear in the categories management section)? This would provide visual consistency throughout the app and make it more visually appealing. I noticed that in the Figma mobile designs we already have something similar:

Screenshot 2025-04-10 182421

  1. Tags Selection on New Transactions: Currently, users can only add tags when editing an existing transaction. Would it be valuable to add tag selection capability when creating new income/expense transactions?

I'd be happy to implement these improvements in a follow-up PR if you think they'd be beneficial. Great work on the app by the way!

@zachgoll
Copy link
Collaborator

Hey @D-Gaspa thanks for the PR! Sorry for the delayed response; we've been pretty busy prepping for launch coming up.

I think this code looks good. If you can fix those merge conflicts and get the tests passing I can get this merged.

As far as the additional improvements with the badges in the form fields, we're not quite ready for that yet. That will come down the road when we starting introducing ViewComponent into the codebase.

Changed transaction form to only display relevant categories based on transaction type (income or expense), improving usability and preventing misclassification. Created a shared transaction type tabs component for consistent navigation between expense, income, and transfer forms, providing a better user experience and reducing code duplication.
@D-Gaspa D-Gaspa force-pushed the fix/separate-income-expense-categories branch from b23c56a to a690842 Compare April 22, 2025 18:06
@D-Gaspa
Copy link
Contributor Author

D-Gaspa commented Apr 22, 2025

Hey @zachgoll! I've resolved the merge conflicts and adapted my changes to the new codebase structure.

Note: Noticed that there might need to be some tweaking done for dark mode support, but I didn't want to introduce more changes for now.

Signed-off-by: Zach Gollwitzer <zach@maybe.co>
@zachgoll zachgoll merged commit 71bc51c into maybe-finance:main Apr 25, 2025
5 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Income category is present in Expense transaction. Also Vice versa

2 participants