search

codeforpdx / PASS

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

Delete old notification system #392

Closed russfraze closed 1 year ago

russfraze commented 1 year ago

name: Delete Old Notification System about: This PR is to delete the useStatusNotification calls, the context, hook, and utility helpers that were a part of the older message notification system. There were also parts of the FormSection component and its props are deleted as well.

Future Steps/PRs Needed to Finish This Work (optional):

I'm seeing that there are now setTimouts that are not calling anything and should be deleted as well.

Issues needing discussion/feedback (optional):

Please take a look.

russfraze commented 1 year ago

I don't know why its saying this branch is out of date, I pulled from Development right before I pushed and its also telling me its up to date when I do a pull now.

xscottxbrownx commented 1 year ago

I don't know why its saying this branch is out of date, I pulled from Development right before I pushed and its also telling me its up to date when I do a pull now.

It's because the PR is set to merge into Master branch... change it to Development branch.

Screenshot 2023-08-21 at 12 44 51 PM

Click "edit" to the right of this

andycwilliams commented 1 year ago

So issue #301 was tied to another PR but was never closed. Since this is related, maybe we could connect that issue, and then finally close it once this is merged.

xscottxbrownx commented 1 year ago

So issue #301 was tied to another PR but was never closed. Since this is related, maybe we could connect that issue, and then finally close it once this is merged.

interesting... I guess it depends on how people feel about the intermediary notifications (something's happening).

I do feel the user should be alerted that something is happening while the async function is running (previous to receiving a success or error snackbar notification.)

Could be done with loading button, an app-wide listening spinner animation component, etc...

We could also delete that part from the issue and make a separate issue for this specific topic.

leekahung commented 1 year ago

It's related to the handleDeleteDocument function in DocumentTableRow.jsx not having the notification hooked up to the new snackbars. There are no notifications triggered whenever a document is deleted.

https://github.com/codeforpdx/PASS/assets/14917816/add091ed-c504-4373-ae6e-38c90150d657

Think we could do a quick inclusion of the new hook for that component.

xscottxbrownx commented 1 year ago

Need to fix conflicts in Profile.jsx and address the error message in SetAclPermsDocContainerForm.jsx still - one of the few unresolved conversations above.

Using the resolve conversation feature of github is great, so we know when everything has been addressed properly.

russfraze commented 1 year ago

With this merge, new snackbar style notifications should now be implemented app wide and the old notification system deleted.