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

fix: create stablecoin form validation triggered on opening the form #523

Merged

Conversation

snigdha920
Copy link
Contributor

@snigdha920 snigdha920 commented Jan 18, 2025

On opening the form, the validation was getting triggered and I was getting errors without having touched any fields. We were manually triggering the form validation when the component was rendered. Now:

  1. Don't trigger react hook form validation manually
  2. To check if we can go to the next step, use zod schema validation on the fields of the form step

Something like isFieldValid = !field.invalid && field.isDirty works when a default value is not provided, but when a default value is provided isDirty is false so isFieldValid is always false even though there is a valid value. So using the zod schema covers all the cases here

Summary by Sourcery

Remove manual form validation and use Zod schema validation to determine if a form step is valid.

Bug Fixes:

  • Fix form validation being triggered on opening the form.

Enhancements:

  • Use Zod schema validation for form steps.

Copy link

linear bot commented Jan 18, 2025

ENG-2229 Fix create stablecoin form

  • the confirm button is cut off and i cannot scroll

CleanShot 2025-01-16 at 18.03.482x.png

  • confirm button is disabled in review
  • the toast does not close when the transaction is reverted
  • the alignment between the texts is not quite right

Copy link

sourcery-ai bot commented Jan 18, 2025

Reviewer's Guide by Sourcery

This pull request addresses an issue where form validation was triggered on form opening, leading to errors before any user interaction. The solution involves removing the manual trigger of form validation and implementing a Zod schema validation to determine the validity of the form step.

Sequence diagram: Form validation flow before and after changes

sequenceDiagram
    participant U as User
    participant F as Form Component
    participant V as Validation

    rect rgb(240, 240, 240)
    Note over U,V: Before Changes
    F->>V: Trigger validation on mount
    V-->>F: Return validation errors
    F->>U: Show validation errors
    end

    rect rgb(200, 255, 200)
    Note over U,V: After Changes
    F->>F: Mount component
    U->>F: Input field value
    F->>V: Validate with Zod schema
    V-->>F: Return validation result
    F->>U: Update UI based on validation
    end
Loading

Class diagram: Form component structure changes

classDiagram
    class FormStep {
        +form: UseFormReturn
        +fields: TName[]
        +validatePage(fields, formValues)
        +title: string
        +withSheetClose: boolean
        -getIsValidPage()
    }

    class CreateTokenSchema {
        +tokenName: string
        +tokenSymbol: string
        +decimals: number
        +isin: string?
        +admin: string
        +collateralProofValidity: number
        +validateCreateTokenSchemaFields()
    }

    FormStep --> CreateTokenSchema: uses
Loading

State diagram: Form validation states

stateDiagram-v2
    [*] --> FormMounted
    FormMounted --> AwaitingInput
    AwaitingInput --> Validating: User inputs data
    Validating --> Valid: Zod schema validates
    Validating --> Invalid: Zod schema fails
    Valid --> NextStepEnabled
    Invalid --> NextStepDisabled
    NextStepEnabled --> [*]: User proceeds
    NextStepDisabled --> AwaitingInput: User corrects input
Loading

File-Level Changes

Change Details Files
Removed manual form validation trigger on component render.
  • Removed the useEffect hook that was manually triggering form validation.
  • Removed the isNavigate state and related logic.
  • Removed the isValid state and related logic.
kit/dapp/src/components/blocks/form/form-step.tsx
Implemented Zod schema validation for form step validity.
  • Added a validatePage prop to the FormStep component.
  • The validatePage prop uses a zod schema to validate the form fields.
  • Created a validateCreateTokenSchemaFields function to validate the form fields against the zod schema.
  • Added a getMask function to help with the zod schema validation.
  • The isin field in the CreateTokenSchema is now optional.
kit/dapp/src/components/blocks/form/form-step.tsx
kit/dapp/src/app/(private)/admin/tokens/_components/create-token-form/create-token-form-schema.ts
Updated the CreateTokenForm component to use the new validation logic.
  • The FormStep components now use the validatePage prop to validate the form fields.
kit/dapp/src/app/(private)/admin/tokens/_components/create-token-form/create-token-form.tsx
Updated the styling of the form components.
  • Added margin bottom to the input components in the TokenBasics step.
  • Added margin top to the description in the TokenPermissions step.
  • Added a className prop to the FormItem component in the NumericInput and TextInput components.
kit/dapp/src/app/(private)/admin/tokens/_components/create-token-form/steps/1-token-basics.tsx
kit/dapp/src/app/(private)/admin/tokens/_components/create-token-form/steps/2-token-permissions.tsx
kit/dapp/src/components/blocks/form/controls/numeric-input.tsx
kit/dapp/src/components/blocks/form/controls/text-input.tsx
Updated the createTokenAction to throw an error if the token creation fails.
  • The createTokenAction now throws an error if the token creation fails.
kit/dapp/src/app/(private)/admin/tokens/_components/create-token-form/create-token-action.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions github-actions bot added the fix label Jan 18, 2025
@@ -152,7 +85,7 @@ export const FormStep = <
<Button
type="button"
className={cn({ hidden: currentStep === totalSteps })}
disabled={(!isValid && isNavigate) || (!isValidPage && !isNavigate)}
disabled={!getIsValidPage()}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pmualaba I removed the isNavigate field because I couldn't find where it was used, please let me know if i need to add it back? and where it is used?

@@ -1,11 +1,10 @@
'use client';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was getting Props must be serializable for components in the "use client" entry file... Removed the 'use client' here because we use this component in a client component, related issue: vercel/next.js#46795

@snigdha920 snigdha920 marked this pull request as ready for review January 18, 2025 14:12
@snigdha920 snigdha920 requested a review from pmualaba January 18, 2025 14:54
@roderik roderik merged commit b794219 into main Jan 19, 2025
2 checks passed
@roderik roderik deleted the snigdha/eng-2229-cannot-scroll-in-the-design-stablecoin-panel branch January 19, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants