search

codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
28 stars 25 forks source link

Issue 316/delete document modal #381

Closed xscottxbrownx closed 1 year ago

xscottxbrownx commented 1 year ago

Delete Document window.confirm --> MUI modal

This PR:

1. Closes Issue #316 2. Creates a DeleteDocumentModal component to replace the window.confirm when clicking the trash icon in document table
3. Fixes a lot of small jsDoc and typedef issues

The files this PR effects:

Components

DeleteDocumentModal.jsx DocumentTable.jsx DocumentTableRow.jsx UploadDocumentModal.jsx /Pages/Profile.jsx

Tests

List file names here

Other Files

/Modals/index.js typedefs.js

Screenshots (if applicable):

https://github.com/codeforpdx/PASS/assets/71395931/beb45206-e33c-4170-885e-5548b1fa88e2

xscottxbrownx commented 1 year ago

Was thinking about this one last night...

Curious to know if anybody feels like I should refactor the codebase to have a ConfirmationModal and use that instead of DeleteDocumentModal and DeleteClientModal etc?

timbot1789 commented 1 year ago

Was thinking about this one last night...

Curious to know if anybody feels like I should refactor the codebase to have a ConfirmationModal and use that instead of DeleteDocumentModal and DeleteClientModal etc?

I like that idea a lot. Less code ≈ fewer bugs

leekahung commented 1 year ago

Was thinking about this one last night...

Curious to know if anybody feels like I should refactor the codebase to have a ConfirmationModal and use that instead of DeleteDocumentModal and DeleteClientModal etc?

Think that would be good.

andycwilliams commented 1 year ago

Was thinking about this one last night... Curious to know if anybody feels like I should refactor the codebase to have a ConfirmationModal and use that instead of DeleteDocumentModal and DeleteClientModal etc?

Think that would be good.

I like this idea. I'm just not sure whether that should be tackled in this PR or another one.

timbot1789 commented 1 year ago

@xscottxbrownx now that I think about it more, since this is new code, I'd rather not add in or review code that you intend to delete in the next few days. If you want/need to use Russell's notification system, then you can base it off that branch, and put up a PR to merge into Russell's notification branch.

xscottxbrownx commented 1 year ago

Closing this PR in favor of a new direction (single ConfirmationModal.) Will create a new PR for this issue when I've accomplished solving the issue via this new direction.