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

Add active user balance and reimbursements #155

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

araffin
Copy link
Contributor

@araffin araffin commented May 19, 2024

This is a proof of concept to display an overview in the expense page for the active user.
I had to adapt the Money component to display in read "You owes B €5" (amount is positive but need to be shown in red).

I also updated formatCurrency to display euro and kr as suffix (10€ vs €10).

Since this is the first time I am writing in typescript/jsx, I would appreciate any feedback =)

Screenshots:
user_balance1
user_balance2

Copy link
Contributor

@dcbr dcbr left a comment

Choose a reason for hiding this comment

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

Apart from a small typo this looks fine I think. I'm only wondering why you would add this to the "Expenses" page and not to the "Balances" page? To me it seems it would fit much better there. And if it were to be added to the "Expenses" page, I would put it at the bottom, after the expenses list. Because right now it's a bit confusing that the first thing you see on the "Expenses" page is a balance overview instead of showing the (expected) overview of expenses.

src/app/groups/[groupId]/expenses/active-user-balance.tsx Outdated Show resolved Hide resolved
@araffin
Copy link
Contributor Author

araffin commented Jun 5, 2024

Thanks for the review =)

I'm only wondering why you would add this to the "Expenses" page and not to the "Balances" page?

That's actually the main point of this PR to have a quick overview without having to do any additional click (I'm also thinking about adding the balance overview when showing the group list).

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.

2 participants