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

feat: Show jar total amount in detail view #551

Merged
merged 3 commits into from
Dec 14, 2022
Merged

feat: Show jar total amount in detail view #551

merged 3 commits into from
Dec 14, 2022

Conversation

httpiga
Copy link
Contributor

@httpiga httpiga commented Oct 26, 2022

Closes #517.
I added total balance next to the tab selector so that it will be always visible.

Screenshot 2022-10-26 at 12 10 57

@httpiga httpiga changed the title Show jar total amount in detail view (#517) Show jar total amount in detail view Oct 26, 2022
@theborakompanioni
Copy link
Collaborator

Hey @httpiga, thanks for your contribution - nice!
There is already a place where the balance is shown if you select individual utxos. Would it make sense to move the total balance to the same position (if no utxo is selected)?

I am asking because there is also a task related to improving the header. See #528.

@editwentyone
Copy link

I like the input, edited my screen

image

@httpiga
Copy link
Contributor Author

httpiga commented Oct 26, 2022

@theborakompanioni thank you for your comment, initially I considered exactly your suggestion, but doing this would show total amount only in "UTXOs" tab, while you won't see it in "Details" tab, so in the end I decided to put the total amount info outside the "UTXOs" tab content.
I really like @editwentyone design, what do you think if we merge it with my consideration (assuming you agree with this) and we put total amount info below jar name?

@theborakompanioni
Copy link
Collaborator

I really like @editwentyone design, what do you think if we merge it with my consideration (assuming you agree with this) and we put total amount info below jar name?

👍 from me!

ping @editwentyone

@editwentyone
Copy link

@httpiga I would like to keep the header clean, the total amount is now under "details" and "utxos" visible

please check the Figma file, you can have a better understanding where we are heading: link

@httpiga
Copy link
Contributor Author

httpiga commented Oct 27, 2022

Thank for sharing the Figma with me @editwentyone , I really like the new design!
However, in your design I can't find the total amount in "Details" tab: you have the total for "External Addresses for deposits", but what if the amount is split between external and internal addresses? In this case I can't see the total amount of that jar, am I missing something?

@editwentyone
Copy link

hm you are right, its not including the internal ones. will need to rethink it a little bit more, thanks for the hint

@httpiga
Copy link
Contributor Author

httpiga commented Oct 27, 2022

Do you think that's better pausing this PR while waiting for an updated design, or merge this just to have it and refactor later?

@theborakompanioni
Copy link
Collaborator

Do you think that's better pausing this PR while waiting for an updated design, or merge this just to have it and refactor later?

Would you be okay with moving it to the place the current figma files suggest, i.e. having it only on the "Jar" tab for now, and then adding it to "Details" tab in a follow-up PR?

I think it is fine either way, we can also merge it now if you prefer that!

@httpiga
Copy link
Contributor Author

httpiga commented Oct 28, 2022

Okay, next week I'm going to move the total amount to the "UTXOs" tab like the Figma suggest!

@theborakompanioni
Copy link
Collaborator

Okay, next week I'm going to move the total amount to the "UTXOs" tab like the Figma suggest!

Hey @httpiga, still motivated to get this feature in? 🧡

@httpiga
Copy link
Contributor Author

httpiga commented Dec 9, 2022

Hey @httpiga, still motivated to get this feature in? 🧡

Hey, sorry for having disappeared but had un unexpected busy period - I'll be glad to implement this as soon as I can!

@theborakompanioni
Copy link
Collaborator

Hey, sorry for having disappeared but had un unexpected busy period - I'll be glad to implement this as soon as I can!

Don't worry. Glad you are still interested. Please know, that there is absolutely no urgency involved. Take your time!

@httpiga
Copy link
Contributor Author

httpiga commented Dec 14, 2022

Thank you for your patience, what about this?

image

Bitcoin amount is not bold since the font weight in BalanceComponent is forced at 400 by slashed-zeroes class name, which is used also by other components. Do we want to refactor BalanceComponent to use Bootstrap classes?

@theborakompanioni
Copy link
Collaborator

Thank you for your patience, what about this?

Nice! 🧡

Bitcoin amount is not bold since the font weight in BalanceComponent is forced at 400 by slashed-zeroes class name, which is used also by other components. Do we want to refactor BalanceComponent to use Bootstrap classes?

What do you think about merging it and doing the refactoring in a follow-up PR?
Of course, only if you are motivated to do so!

@httpiga httpiga marked this pull request as ready for review December 14, 2022 09:22
@httpiga
Copy link
Contributor Author

httpiga commented Dec 14, 2022

Let's do this 🧡
I opened this PR to review, and will merge it once approved!

@theborakompanioni theborakompanioni self-requested a review December 14, 2022 12:34
@theborakompanioni theborakompanioni added enhancement New feature or request UI/UX Issue related to cosmetics, design, or user experience labels Dec 14, 2022
@theborakompanioni theborakompanioni changed the title Show jar total amount in detail view feat: Show jar total amount in detail view Dec 14, 2022
@theborakompanioni theborakompanioni merged commit 90f22e5 into joinmarket-webui:master Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request UI/UX Issue related to cosmetics, design, or user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jar detail view: show total amount in jar
3 participants