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

[Form Refactor] ValidationStep #9582

Closed
8 tasks
luacmartins opened this issue Jun 27, 2022 · 14 comments
Closed
8 tasks

[Form Refactor] ValidationStep #9582

luacmartins opened this issue Jun 27, 2022 · 14 comments
Assignees
Labels
Engineering Reviewing Has a PR in review Weekly KSv2

Comments

@luacmartins
Copy link
Contributor

Coming from the New Expensify Forms design doc, we should refactor ValidationStep to use the new form component, follow the guidelines below:

Here's an example of a Form refactor: #9056

Guidelines

  1. Replace the form component with Form.js.
  2. Create a unique Onyx key in ONYXKEYS.FORM and pass it as the formID prop to Form.
  3. Pass a validate callback prop.
  4. Pass an onSubmit callback prop that calls the API via an action.
  5. Update all inputs wrapped by Form, following the guidelines in Refactor inputs.
  6. Remove any unused code.

Testing

Verify that:

  • UI looks as it did before the refactor
  • Values can be added and edited
  • Errors are highlighted correctly (input border)
  • Error messages show up correctly
  • Draft values are saved properly
  • Form alerts are displayed correctly
  • Clicking the fix the errors link focuses the first input with error
  • No duplicate submission of the form occurs (when it's already submitting)
@luacmartins
Copy link
Contributor Author

This issue is being put on hold due to push to page discussions, as per this comment.

@luacmartins luacmartins changed the title [Form Refactor] ValidationStep [HOLD] [Form Refactor] ValidationStep Jun 29, 2022
@melvin-bot melvin-bot bot added the Overdue label Jul 7, 2022
@luacmartins
Copy link
Contributor Author

On hold!

@melvin-bot melvin-bot bot removed the Overdue label Jul 7, 2022
@melvin-bot melvin-bot bot added the Overdue label Jul 18, 2022
@luacmartins
Copy link
Contributor Author

On hold

@melvin-bot melvin-bot bot removed the Overdue label Jul 18, 2022
@melvin-bot melvin-bot bot added the Overdue label Jul 27, 2022
@luacmartins
Copy link
Contributor Author

On hold

@melvin-bot melvin-bot bot removed the Overdue label Jul 27, 2022
@melvin-bot melvin-bot bot added the Overdue label Aug 4, 2022
@luacmartins
Copy link
Contributor Author

On hold

@melvin-bot melvin-bot bot removed the Overdue label Aug 5, 2022
@luacmartins
Copy link
Contributor Author

No longer on hold!

@luacmartins luacmartins changed the title [HOLD] [Form Refactor] ValidationStep [Form Refactor] ValidationStep Aug 8, 2022
@melvin-bot melvin-bot bot added the Overdue label Aug 19, 2022
@luacmartins
Copy link
Contributor Author

I'll try to work on it this week!

@melvin-bot melvin-bot bot removed the Overdue label Aug 22, 2022
@melvin-bot melvin-bot bot added the Overdue label Aug 31, 2022
@luacmartins
Copy link
Contributor Author

I'm focused on offline first and N7 issues this week. Gonna try to get to this next week.

@melvin-bot melvin-bot bot removed the Overdue label Aug 31, 2022
@melvin-bot melvin-bot bot added the Overdue label Sep 8, 2022
@luacmartins
Copy link
Contributor Author

Prioritizing N7 issues, I'll get to this next week.

@melvin-bot melvin-bot bot removed the Overdue label Sep 8, 2022
@aldo-expensify
Copy link
Contributor

Sorry @luacmartins , didn't have enough time to get to this before the end of last week. I'll assign it back to you so you can decide on doing it yourself or finding someone else :)

@aldo-expensify
Copy link
Contributor

@luacmartins I asked in slack if someone wanted to give this a try here and @youssef-lr offered to help (just letting you know)

@youssef-lr youssef-lr added the Reviewing Has a PR in review label Sep 14, 2022
@youssef-lr
Copy link
Contributor

youssef-lr commented Sep 14, 2022

hey @luacmartins! I noticed that we're using isLoading in Form.js instead of isSubmitting as found in the DD. Which one should be use in the refactors?
Edit: same goes for error vs serverErrorMessage. Should we update the DD or the code?

@luacmartins
Copy link
Contributor Author

@youssef-lr we changed to isLoading everywhere, so you should use that key.

@youssef-lr
Copy link
Contributor

Thanks @luacmartins, I went ahead and updated the DD.

@melvin-bot melvin-bot bot closed this as completed Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

3 participants