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

Refactor Modal Component and Form Handling #652

Open
jtucholski opened this issue Jan 13, 2025 · 0 comments · May be fixed by #727
Open

Refactor Modal Component and Form Handling #652

jtucholski opened this issue Jan 13, 2025 · 0 comments · May be fixed by #727
Assignees

Comments

@jtucholski
Copy link
Contributor

Description:
During the review of PR #645, we identified opportunities to refactor the Modal component and the Add, Edit, and Delete forms to address some recurring problems and improve maintainability.

Existing Problems:

  1. CloseX Component:
    • Each form currently implements its own CloseX component and a closeAndReset function to reset the form.
  2. Form Reset on Modal Close (ESC):
    • Closing a modal using the ESC key happens at the Modal/HTMLDialogElement level. However, form values inside the modal were not reset when the modal was closed.
    • To address this, a useEffect hook was added to each form to wipe the values when the dependent object changes. While this works as a temporary fix, it’s not a long-term solution.

Proposed Solution:

  • Move the responsibility for resetting the form to the Modal component.
  • Refactor FormProps so they can be passed into the Modal along with the form itself, enabling the Modal to invoke reset() on any form.

This refactor would simplify form handling, reduce redundant code, and improve the maintainability of the Modal and related components.

@calisio calisio self-assigned this Jan 15, 2025
@calisio calisio added the added after sprint This label reflects tasks that were added after the sprint started. label Jan 16, 2025
@calisio calisio linked a pull request Feb 5, 2025 that will close this issue
@calisio calisio removed the added after sprint This label reflects tasks that were added after the sprint started. label Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants