Fix: User hub UI/UX updates for crypto to fiat tab #2980
Conversation
There was a problem hiding this comment.
Nice job with this! Thank you for tidying up the rushed hackathon code!
Tooltips look great:
Also looking good on dark mode:
The only requirement missing from the issue is the max button not setting the correct value (I think you can just remove the decimals value from the Numeral component in the Withdraw Card so that it shows the full balance)
It also doesn't correctly format the number when it is set via the max button, it should be comma separated.
mmioana
force-pushed
the
ui/2665-user-hub-updates
branch
from
7875427 to
ac0698e
Compare
There was a problem hiding this comment.
Great job fixing up the 'max' button! It is working as intended now!
Although I realised whilst re-reviewing this that the 'receive' currency should only ever be in EUR/USD dependant on the bank details entered when the user signs up to crypto to fiat rather than the currency selected in the currency context. (I'm not sure if that detail is currently stored in another context or not, whether it is a simple change to make here? Otherwise that may increase the scope so I think just leaving the currency hardcoded here to avoid confusion might be preferred - let me know what you think! :) )
src/components/common/Extensions/UserHub/partials/CryptoToFiatTab/partials/ReceiveCard.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Just seen that you have fixed this on #2988 - so approving here!
There was a problem hiding this comment.
The devil's in the details, really nice job tackling this bunch of different issues and formatting values like they should.
It's kinda lame that there aren't many options to format a value jsut for display purposes, but keeping it as a number under the hood. Now we have to do double parsing, but hey that's life in React right now :D
Max works
Hover:
The icons are changed and the font weights are in line with Figma ✔️
Validation:
Tooltips:
And the fee texts are also a lighter shade of gray, LGTM 😎
| @@ -35,20 +41,35 @@ const WithdrawCard: FC<WithdrawCardProps> = ({ | |||
| balance, | |||
| handleSetMax, | |||
| }) => { | |||
| const name = 'amount'; | |||
| const name = TransferFields.AMOUNT; | |||
There was a problem hiding this comment.
for clarity's sake maybe we could name this with UPPERCASE, maybe even move it outside of the component since it never changes?
mmioana
force-pushed
the
ui/2665-user-hub-updates
branch
2 times, most recently
from
51aaa23 to
8d891bb
Compare
Fix: User hub UI/UX updates for crypto to fiat tab
mmioana
force-pushed
the
ui/2665-user-hub-updates
branch
from
8d891bb to
092835a
Compare
Get fee values and conversion rates Compute receive amount in user hub tab Update selected currency based on bank account details
|
I rebased this branch and I have lost your approvals @bassgeta @iamsamgibbs 😢 |
There was a problem hiding this comment.
Given this another test after the rebase and looks good!
Although, now that #2956 has been merged in, I've spotted this hover state issue with the withdraw / receive cards. When disabled, the border shouldn't change on hover:
I think it makes sense to fix this up here as it will be a tiny change and there are already changes to the withdraw / receive cards.
…eived-amount Feat/2952: Calculate receive amount in user hub tab
|
@iamsamgibbs thank you for noticing this issue regarding the hovering state! I'm on to solving it 🚀 |
Fix: Updates for CryptoToFiatTab components disabled state
|
@rdig I tested it now and can confirm we get the USD conversion and the max value updates properly
Screen.Recording.2024-08-29.at.12.39.42.mov |
|
I believe I do, but will re-test to make sure. |
Description
As part of the hackathon, the following updates are required to the new user hub functionality
Figma link
Testing
TODO: Please check the above mentioned changes are in place (or any other Figma mismatch)
Important
The values for the summary card will be updated in another PR related to #2952
Resolves #2665