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] Request a call page #9575

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

[Form Refactor] Request a call page #9575

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

Comments

@luacmartins
Copy link
Contributor

luacmartins commented Jun 27, 2022

Coming from the New Expensify Forms design doc, we should refactor RequestCallPage 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 luacmartins self-assigned this Jun 27, 2022
@luacmartins luacmartins changed the title Refactor Request a call page to use Form [Form Refactor] Request a call page Jun 27, 2022
@luacmartins luacmartins changed the title [Form Refactor] Request a call page [HOLD] [Form Refactor] Request a call page Jun 29, 2022
@luacmartins
Copy link
Contributor Author

Holding on push to page discussion as per this comment

@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 luacmartins changed the title [HOLD] [Form Refactor] Request a call page [Form Refactor] Request a call page Aug 8, 2022
@luacmartins
Copy link
Contributor Author

No longer on hold!

@melvin-bot
Copy link

melvin-bot bot commented Aug 26, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@ntdiary
Copy link
Contributor

ntdiary commented Aug 26, 2022

Oh sorry, I think the bot's comment shouldn't be here (maybe it's my fault), I just wanted to cite this issue.

@luacmartins
Copy link
Contributor Author

@ntdiary no worries! these comments have been all over the place.

@melvin-bot
Copy link

melvin-bot bot commented Sep 16, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

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

2 participants