Fix: Crypto-to-fiat account area UI fixes #2798

mmioana · 2024-07-29T10:40:00Z

Description

This PR contains UI styling changes for the Crypto-to-fiat user account page
Figma link

KYC Personal details modal

Additional space is required between the close X and the stepper component to not only match Figma but make this modal consistent with global modals.
Padding needs to be adjusted between the last field of the form and the button row. Also, padding needs to be adjusted between the CTA's and bottom of the modal (again, to align it globally).

Error states - Can we align these closer to the copy provided in Figma for a nicer UX?

Figma:

The p copy after the heading should be text-gray-600

KYC Terms modal

Can you please kindly adjust the modal height to accommodate the Bridge embed, so the scrollbar isn't triggered inside the modal.

Completing the KYC modal

Upon completion of the KYC steps and after closing the bridge modal, the UI doesn't give any feedback at all about the status of my verification or that, in fact, I've completed it. This is confusing because I think I need to go back into the KYC flow because nothing is returned. Can we have a solution here that provides the issue with an update upon the bridge modal closing without making the user refresh manually.

C2F page

Can we add a capital O to 'Optional'? This is an error in Figma, so apologises on this.

The loading indicator is no longer centre aligned in the user account area (happy for this to be raised in a sep issue)

Approved pill is using the wrong colour tokens (looks like it's using team colours and not primary colours)

'Bank, address and currency information' title should be a medium font weight.

Bank details modal

Same issues with regards to p colour as the KYC modal, should match Figma and be text-gray-600

Padding between inputs should align with Figma.

Padding between input and the bottom of the modal (same as the KYC) should also match Figma.

For this modal only, please remove the stepped navigation pills and match the padding between the 'X' close and the heading to this version of the modal here: https://www.figma.com/design/C2grfQysdsYXz0j4rADR6K/Crypto-to-Fiat?node-id=5245-21254&t=bCVgC5ImOijNpbQW-4

This selected state for the input should match the design system component (no blue text and incorrect font size)

Component link: https://www.figma.com/design/l1dOM5qiQYwF0ElvKDqqjg/Design-System---Colony-v3?node-id=2706-1485&t=MkbiMKMM96kcL0sB-4

Errors on empty states should match the Figma copy. The invalid error should show when an invalid input is provided by the user, but the empty state should tell the user the field is required.

Examples of empty inputs:

Toast should not display behind the bg overlay when the modal is active

Can the toast please provide more contextual information with both the toast header and text included. Please remove the :(

Heading: Something went wrong
Text: Your bank details could not be updated at this time

Testing

Make sure to have the POSTHOG_ variables set in .env.local.secrets (if you don't have them please PING me)
Connect your wallet to leela
Go to http://localhost:9091/account/crypto-to-fiat
Check page sections and modals styling

Resolves #2741
Resolves #2742 - with special thanks to @rumzledz

melyndav

Hey @mmioana nice start on all the UI updates. Some feedback on this one below.

Number 3 from my issue still requires updates - this is removing the {fieldName} and using the label for nicer copy and better UX.

When I input an email with incorrect format, I receive no error validation error.

The stepped navigation should not be intractable. It's a visual indicator, with no navigation here.

Right now, I can skip through and bypass the terms.

Video:

Can you kindly change the 'KYC form' in the stepped pill to be 'KYC'.

I can't submit the first CTA in the KYC modal. So I can't test the rest of the PR such as the bank details modal andother status pill updates. :(

mmioana · 2024-08-05T19:17:39Z

Hey @melyndav thank you for reviewing this PR 🌟

Some of the error messages I didn't properly update, but now they should be in place ✨

Regarding your point 2. When I input an email with incorrect format, I receive no error validation error.
we use the same email validation function from the Yup library (as in the form from the onboarding wizard) which considers the email you have entered to be valid

Let me know if we need to impose stricter rules when it comes to allowed emails and what format should be acceptable

For further testing the flow, you'll need to manually update a key, but let me know when you plan to re-test this PR so we can sync 🙏

mmioana · 2024-08-07T17:21:24Z

Hey @melyndav 🌻 how would you like to proceed for the email validation?

melyndav · 2024-08-08T08:55:18Z

Hey @mmioana this is fine to continue using the Yup library for this. You can ignore point 2 of my feedback.

mmioana · 2024-08-08T09:34:07Z

Cool @melyndav! Either way if you want to enforce stricter validation, just let me know.
Otherwise this PR is ready for your review 🙏

rumzledz

Awesome work on this @mmioana ! ✨

The badge colour, font weights, spacings and most importantly, the error messages are now much more pleasant to read 😄

Fix: Crypto-to-fiat account area UI fixes

…form stepper Fix: Changes for KYC form error messages and removal of bank details form stepper

Fix: Changes after rebase and minor improvements

Fix: Changes after review

Fix: Updates after rebase and update EUR required fields errors

melyndav

Hey @mmioana thanks for the further updates on this PR! Everything is looking great and all the issues have been resolved. Thanks for your effort on this one 🥇

Nortsova

Hey @mmioana ✨! Thanks for this beautiful changes 🤩

Validation and paddings looks great!

There is no scroll on Bridge embed tab:

Select options looks great!

"Approve" pill has correct color, "optional" has correct "O" capital, 'Bank, address and currency information' title has correct medium font weight.

Nortsova · 2024-08-12T12:32:14Z

src/utils/intl.ts

+export function formErrorMessage(
+  fieldName: MessageDescriptor,
+  validationMessage: 'required' | 'min' | 'max' | 'invalid' | 'length',
+  length?: number,
+) {
+  if (
+    (validationMessage === 'min' ||
+      validationMessage === 'max' ||
+      validationMessage === 'length') &&
+    length === undefined
+  ) {
+    throw new Error(
+      'length is required when validationMessage is min, max or length',
+    );
+  }


good point! thanks ✨

@rumzledz is the master of disaster for this 🥇

mmioana self-assigned this Jul 29, 2024

rumzledz mentioned this pull request Jul 29, 2024

Fix: update Bank Details Form copies #2791

Closed

mmioana force-pushed the ui/2741-user-account branch 2 times, most recently from e65aa3d to b0db6b9 Compare August 1, 2024 08:21

mmioana marked this pull request as ready for review August 1, 2024 08:21

mmioana requested review from a team as code owners August 1, 2024 08:21

mmioana marked this pull request as draft August 1, 2024 10:08

mmioana force-pushed the ui/2741-user-account branch 2 times, most recently from 724adc3 to fe2693e Compare August 2, 2024 07:20

mmioana marked this pull request as ready for review August 2, 2024 07:20

mmioana force-pushed the ui/2741-user-account branch from fe2693e to f087494 Compare August 2, 2024 07:26

melyndav requested changes Aug 2, 2024

View reviewed changes

mmioana marked this pull request as draft August 2, 2024 13:21

mmioana marked this pull request as ready for review August 5, 2024 19:17

mmioana requested a review from melyndav August 6, 2024 11:43

rumzledz previously approved these changes Aug 8, 2024

View reviewed changes

mmioana added 5 commits August 8, 2024 15:01

Fix: Crypto-to-fiat account area UI fixes

95e47e5

Fix: Crypto-to-fiat account area UI fixes

Fix: Changes for KYC form error messages and removal of bank details …

4360f7a

…form stepper Fix: Changes for KYC form error messages and removal of bank details form stepper

Fix: Changes after rebase and minor improvements

7e22a9a

Fix: Changes after rebase and minor improvements

Fix: Changes after review

06670e8

Fix: Changes after review

Fix: Updates after rebase and update EUR required fields errors

56ad214

Fix: Updates after rebase and update EUR required fields errors

mmioana dismissed rumzledz’s stale review via 56ad214 August 8, 2024 13:49

mmioana force-pushed the ui/2741-user-account branch from 72d658e to 56ad214 Compare August 8, 2024 13:49

rumzledz approved these changes Aug 8, 2024

View reviewed changes

melyndav approved these changes Aug 12, 2024

View reviewed changes

Nortsova approved these changes Aug 12, 2024

View reviewed changes

mmioana merged commit fd3c2bb into master Aug 12, 2024
4 of 6 checks passed

mmioana deleted the ui/2741-user-account branch August 12, 2024 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Crypto-to-fiat account area UI fixes #2798

Fix: Crypto-to-fiat account area UI fixes #2798

mmioana commented Jul 29, 2024 •

edited

Loading

melyndav left a comment

mmioana commented Aug 5, 2024

mmioana commented Aug 7, 2024

melyndav commented Aug 8, 2024

mmioana commented Aug 8, 2024

rumzledz left a comment

melyndav left a comment

Nortsova left a comment

Nortsova Aug 12, 2024

mmioana Aug 12, 2024

Fix: Crypto-to-fiat account area UI fixes #2798

Fix: Crypto-to-fiat account area UI fixes #2798

Conversation

mmioana commented Jul 29, 2024 • edited Loading

Description

KYC Personal details modal

KYC Terms modal

Completing the KYC modal

C2F page

Bank details modal

Testing

melyndav left a comment

Choose a reason for hiding this comment

mmioana commented Aug 5, 2024

mmioana commented Aug 7, 2024

melyndav commented Aug 8, 2024

mmioana commented Aug 8, 2024

rumzledz left a comment

Choose a reason for hiding this comment

melyndav left a comment

Choose a reason for hiding this comment

Nortsova left a comment

Choose a reason for hiding this comment

Nortsova Aug 12, 2024

Choose a reason for hiding this comment

mmioana Aug 12, 2024

Choose a reason for hiding this comment

mmioana commented Jul 29, 2024 •

edited

Loading