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

frontend: rotate default currency on account summary #2635

Merged

Conversation

shonsirsha
Copy link
Collaborator

@shonsirsha shonsirsha commented Mar 18, 2024

to improve UX, we'd like to enable rotating default currency on account summary. This is done by:

  • creating DefaultCurrencyRotator component
  • reused that component in rates.tsx and chart.tsx
  • onClick, fires rotateFiat (RatesContext).
  • reinitialize chart on change defaultCurrency (RatesContext)

frontends/web/src/routes/account/summary/chart.tsx Outdated Show resolved Hide resolved
frontends/web/src/routes/account/summary/chart.tsx Outdated Show resolved Hide resolved
frontends/web/src/components/rates/rates.tsx Outdated Show resolved Hide resolved
@thisconnect
Copy link
Collaborator

When I rotate the fiat is a bit unstable and seems to change a few times, I haven't investigated .. do you see that too?

@shonsirsha shonsirsha force-pushed the frontend-rotate-fiat-account-summary branch from 97d0ef9 to c979515 Compare March 19, 2024 05:07
@shonsirsha
Copy link
Collaborator Author

thx for the review as usual, PTAL 🙏 @thisconnect

@shonsirsha shonsirsha force-pushed the frontend-rotate-fiat-account-summary branch from 931097b to 9f57a41 Compare March 19, 2024 07:21
@thisconnect
Copy link
Collaborator

tested qt-osx and it looks good, but I want to take a bit more time to look at the changes in frontends/web/src/contexts/RatesProvider.tsx

Copy link
Collaborator

@thisconnect thisconnect left a comment

Choose a reason for hiding this comment

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

tested LGTM, with last small pixel nit

to improve UX, we'd like to enable rotating default currency
on account summary. This is done by:
- creating DefaultCurrencyRotator component
- reused that component in rates.tsx and chart.tsx
- onClick, runs `rotateFiat` (`RatesContext`).
- reinitialize chart on change `defaultCurrency` (`RatesContext`)
@shonsirsha shonsirsha force-pushed the frontend-rotate-fiat-account-summary branch from 3001bce to 85e72c7 Compare March 19, 2024 16:27
@thisconnect thisconnect merged commit 61ddaad into BitBoxSwiss:master Mar 20, 2024
5 checks 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.

2 participants