Skip to content

Ieee 272 implement item incident form #503

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

Merged
merged 43 commits into from
Aug 27, 2023

Conversation

natapokie
Copy link
Contributor

@natapokie natapokie commented Jun 18, 2023

Overview

  • Resolves 272

  • Created incident form with form validation

    • displays error messages on invalid submission
    • logs the values on correct submission (later replaced with the actual backend stuff)
      • upon successful submission resets the form
  • back button takes you back to the previous page

  • broken link button on checked out tables will navigate user to the incident form page

Some General Questions...

  • Qty seemed like it was a dropdown, so I just put an arbitrary selection of numbers 1-8 (not sure if there is a specific number?)
  • Is the link implementation to the incident form ok? i worked off of a file that was already in the code base
  • What should users be submitting in the text fields (what, where when)? Is there an example we can provide as a placeholder instead and use the current placeholder (e.g., "What happened to the hardware?") as a label instead?
  • How to handle the submit? for example, display an success message, or disable the submit button after?

Preview...

broken link in CheckedOutTables component (uncommented previous code),
image

incident form
image

invalid submission
image

Unit Tests Created

  • Unit test ensures that components are rendered properly, yarn test IncidentForm.test.tsx
    • note: didn't make any tests to emulate submission since formik in the frontend takes care of it...

Steps to QA

  • navigate to /incident-form
  • in order to submit, all fields must be entered
  • an field that is empty will prompt an error message
  • pressing submit will print all values to the console

@natapokie natapokie marked this pull request as ready for review June 21, 2023 03:06
@Mustaballer
Copy link
Contributor

Some General Questions...

  • Qty seemed like it was a dropdown, so I just put an arbitrary selection of numbers 1-8 (not sure if there is a specific number?)
  • Is the link implementation to the incident form ok? i worked off of a file that was already in the code base
  • What should users be submitting in the text fields (what, where when)? Is there an example we can provide as a placeholder instead and use the current placeholder (e.g., "What happened to the hardware?") as a label instead?
  • How to handle the submit? for example, display an success message, or disable the submit button after?
  1. For quantity dropdown the number would range from 1 to the total quantity checked out for that hardware.
  2. Will need to test(will do soon)
  3. For submitting I was thinking of like a redirect to the dashboard page with a success snackbar. You're free to do what you feel makes most sense.

@natapokie
Copy link
Contributor Author

@Mustaballer thanks for clarifying !!

in the link to the incident page, I used query parameters(?) to pass in data -- just wanted to provide this as one of the options because I realize now that having a page for the incident form might not be the best idea since you probably want to pass in the team info too...
currently, i just added the hardware quantity to the link, so the dropdown will show different numbers!

i do also like the idea of a snackbar so i added that in my recent commit,

@KarandeepLubana
Copy link
Contributor

image

Empty values are considered invalid. However, you should not display an error in the very beginning, as it is a bad UX (the user clicks a button and sees various errors before having made any action). You should validate a field after its value has changed, or once the submit button is clicked

@KarandeepLubana
Copy link
Contributor

KarandeepLubana commented Jun 24, 2023

Screenshot (761)
image
image

Fix any responsiveness-related issues

Copy link
Contributor

@KarandeepLubana KarandeepLubana left a comment

Choose a reason for hiding this comment

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

Overall great start! There are some issues that need to be addressed before we can merge.

@KarandeepLubana
Copy link
Contributor

The "Report Broken/Lost" button has a discoverability issue as users may not realize it exists unless they scroll to the left. Consider redesigning the UI to address this concern, unless the current UI intentionally intends for the button to be less prominent.

image

@natapokie
Copy link
Contributor Author

The "Report Broken/Lost" button has a discoverability issue as users may not realize it exists unless they scroll to the left. Consider redesigning the UI to address this concern, unless the current UI intentionally intends for the button to be less prominent.

image

will address this in another issue! 👍

@natapokie natapokie merged commit a7222e1 into develop Aug 27, 2023
@natapokie natapokie deleted the IEEE-272-implement-item-incident-form branch August 27, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants