feat: refactor modals #727
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jtucholski
reviewed
Feb 6, 2025
Comment on lines
53
to
56
| const onSubmitHandler: SubmitHandler<T> = async (data) => { | ||
| await onSubmit(data); | ||
| reset(); | ||
| }; |
There was a problem hiding this comment.
This is good, having it automatically clear the form on submit.
jtucholski
reviewed
Feb 6, 2025
Comment on lines
49
to
51
| useEffect(() => { | ||
| reset(defaultValues); | ||
| }, [defaultValues, reset]); |
There was a problem hiding this comment.
Does this handle the case where the user presses Esc and the form resets back to it's default values?
jtucholski
reviewed
Feb 6, 2025
Comment on lines
64
to
69
| const openDeleteFacility = (facility: Facility) => { | ||
| setTargetFacility({ | ||
| action: CRUDActions.Delete, | ||
| facility: facility | ||
| }); | ||
| }; |
There was a problem hiding this comment.
It feels like these should have the word handle in the name. My initial reaction was that it's doing the same as what is in the useEffect
jtucholski
reviewed
Feb 6, 2025
Comment on lines
84
to
145
| const facilityInputs: Input[] = [ | ||
| { | ||
| type: FormInputTypes.Text, | ||
| label: 'Name', | ||
| interfaceRef: 'name', | ||
| required: true | ||
| }, | ||
| { | ||
| type: FormInputTypes.Dropdown, | ||
| label: 'Timezone', | ||
| interfaceRef: 'timezone', | ||
| required: true, | ||
| enumType: Timezones | ||
| } | ||
| revalidate(); | ||
| editFacilityModal.current?.close(); | ||
| addFacilityModal.current?.close(); | ||
| resetModal(); | ||
| } | ||
| ]; | ||
|
|
||
| const handleDeleteFacilityCancel = () => { | ||
| deleteFacilityModal.current?.close(); | ||
| resetModal(); | ||
| const addFacility: SubmitHandler<FieldValues> = async (data) => { | ||
| const response = await API.post('facilities', data); | ||
| if (!response.success) { | ||
| toaster('Failed to add facility', ToastState.error); | ||
| return; | ||
| } | ||
| toaster('Facility created successfully', ToastState.success); | ||
| void mutate(); | ||
| addFacilityModal.current?.close(); | ||
| setTargetFacility(null); | ||
| }; | ||
| const openDeleteFacility = (facility: Facility) => { | ||
| deleteFacilityModal.current?.showModal(); | ||
| setTargetFacility(facility); | ||
|
|
||
| const updateFacility: SubmitHandler<FieldValues> = async (data) => { | ||
| const response = await API.patch( | ||
| `facilities/${targetFacility?.facility.id}`, | ||
| data | ||
| ); | ||
| if (!response.success) { | ||
| toaster('Failed to update facility', ToastState.error); | ||
| return; | ||
| } | ||
| toaster('Facility updated successfully', ToastState.success); | ||
| void mutate(); | ||
| editFacilityModal.current?.close(); | ||
| setTargetFacility(null); | ||
| }; | ||
| const handleDeleteFacility = async () => { | ||
| if (targetFacility?.id == 1) { | ||
|
|
||
| const deleteFacility = async () => { | ||
| if (targetFacility?.facility.id == 1) { | ||
| toaster('Cannot delete default facility', ToastState.error); | ||
| deleteFacilityModal.current?.close(); | ||
| return; | ||
| } | ||
|
|
||
| await API.delete('facilities/' + targetFacility?.id) | ||
| .then((response) => { | ||
| if (response.success) { | ||
| toaster( | ||
| 'Facility successfully deleted', | ||
| ToastState.success | ||
| ); | ||
| revalidate(); | ||
| } else { | ||
| toaster('Error deleting facility', ToastState.success); | ||
| } | ||
| }) | ||
| .catch(() => { | ||
| toaster('Error deleting facility', ToastState.error); | ||
| }); | ||
|
|
||
| const response = await API.delete( | ||
| 'facilities/' + targetFacility?.facility.id | ||
| ); | ||
| if (response.success) { | ||
| toaster('Facility successfully deleted', ToastState.success); | ||
| void mutate(); | ||
| } else { | ||
| toaster('Error deleting facility', ToastState.error); | ||
| } | ||
| deleteFacilityModal.current?.close(); | ||
| resetModal(); | ||
| void mutate(); | ||
| setTargetFacility(null); | ||
| return; | ||
| }; |
There was a problem hiding this comment.
What do you think about something like putting the form inputs and the handlers into their own file so that we have a reusable form module for managing facilities?
It cleans up the FacilityManagement page a bit more and adds an element of reusability (not that we have a use case for it yet).
I could be overthinking it and I am open to you pushing back on this.
jtucholski
reviewed
Feb 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the change
Refactors modal into a new modal component. This will standardize the way that we implement form modals by passing an array of inputs, which will then create the form automatically. This will eliminate the need to create a new file included in the
/formsfolder.NOTE: will be moving the interfaces/enums to
common.ts, but just for simplicity now i am keeping them in the new modal. I also will be renaming at the very end once all is transferred to new modal.Related issues: closes #652
Additional context
Still a work in progress. Leaving as a draft for people to look at it first before implementing across most pages that include modals.