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

added expense detail page #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

marvelalexius
Copy link
Collaborator

  • added a href wrapper in expense card
  • installed new badge component from shadcn

image

- added a href wrapper in expense card
- installed new badge component from shadcn
@marvelalexius marvelalexius self-assigned this May 26, 2024
@marvelalexius marvelalexius requested review from a team as code owners May 26, 2024 10:09
@marvelalexius marvelalexius linked an issue May 26, 2024 that may be closed by this pull request
app/components/card-expense.tsx Show resolved Hide resolved
<h3 className="text-lg font-medium">
{new Intl.NumberFormat("id-ID", {
style: "currency",
currency: "IDR",
Copy link
Member

Choose a reason for hiding this comment

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

The currency won't always be IDR though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, what should I use then? Should I use original_currency or do we have an env for deciding which currency to show?

Copy link
Member

Choose a reason for hiding this comment

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

You should show both original currency and IDR currency though. The original transaction won't always be in IDR, but people who see this site mostly will come from Indonesia. So there's that.

</h3>
</div>
{/* TODO: use whatever we use for detail links. Eg: tx code, unique id, uuid, etc. */}
<a href={`/expenses/123`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, kenapa ga langsung pake transaction_code? atau dto nya belum final ya?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's the question, masih unclear posisinya, but I think I'll change it to transaction_code langsung

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I'd also prefer this to be a /expenses/${expense.transaction_code}

Comment on lines +76 to +79
{new Intl.NumberFormat("id-ID", {
style: "currency",
currency: "IDR",
}).format(expense.idr_amount.toNumber())}
Copy link
Contributor

Choose a reason for hiding this comment

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

kenapa ga di buat utility fungsi sendiri ya? jadi kalo misal ada yang perlu disesuaikan tinggal di satu tempat

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, will do it later

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.

Frontend: Expense detail page
3 participants