[Enhancement/Refactor]- Issue 509/create new use message list hook

[leekahung]

This PR:

Partially addresses #509. This PR aims to migrate over/refactor the existing workflow for messages loading and rendering from utils and React Context to React Query.

Unit tests that's related to existing changes has been updated. A new test file has been made, mainly for the new custom hook useMessageList.

Screenshots (if applicable):

https://github.com/codeforpdx/PASS/assets/14917816/5e0170ba-d72c-46e9-b1c2-d8c3fc79db0f

https://github.com/codeforpdx/PASS/assets/14917816/3e62a8f1-1085-4523-af1b-42abf4bab5bc

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

Future steps for this will include (in no particular order):

• Updates to other Message functionalities in utils similar to what is done here, but for message building and message sending
• Splitting Inbox/Outbox into separate pages

Issues needing discussion/feedback (optional):

1. Issue #509 will be on-going even after this is merged (see Future Steps above)

[leekahung]

I'm a bit confused by this statement

Had tried using just React Query for isLoading or isFetching, but the UI appears smoother with React Context and does not cause any noticeable re-renders of the message cards, so it remained with React Context.

React Query is built on top of a react context, so this shouldn't make a difference. If there's UI smoothness issues, they probably come from how the UI itself is handling the different loading states.

Was a bit confused myself about this actually, when I first noticed this (see clip when I've used isFetching again), but I think I know what might be happening. Since clicking on unread message previews updates the read status for message Thing on Solid, I that's what's causing the loading screen to pop up. The second press was fine because it's already read and updateMessageReadStatus is no longer going to trigger a state update.

https://github.com/codeforpdx/PASS/assets/14917816/a8c768f4-8992-4642-830e-38f04c2529f0

I notice you're hitting the network multiple times whenever you update a message. This may be why you were having weird behavior, and it shouldn't be necessary.

Think I've managed to fix this behavior by moving the condition for readStatus along with folderType === 'Inbox' in the update I'm coding up now so updateMessageReadStatus won't be triggered at all if the message had already been read.

The react query update mutations can handle all of that for you. You don't need to manually refetch in the components either. The purpose of these hooks is to hide the network from the components. Put all the networking logic in the hooks, and expose simple interfaces to the components.

Oh! That would be nice. I'll see if I could get mutations working for this. I simply wanted to update the readStatus for that one message. Like what's shown above in the clip, but without the loading screen appearing.

You may find it simpler not to handle both the inbox and the outbox in a single hook. Maybe try separating the inbox logic into its own useInbox hook, and the outbox into a useOutbox hook. I think you'll be able to get a clearer design that way.

I was planning on splitting up Inbox and Outbox into their own routes and pages in a later PR.

[leekahung]

Hey @timbot1789, think I've managed to get the React Query mutation working! Also got the loadMessages being handled directly by isFetching now without triggering the loading animation unless it's manually refreshed via the refresh button or when loading into the Messages page for the first time.

[bingeboy]

I'm running this on my local and I'm noticing that when I click on messages from the top nav the Messages section loads with "No Messages Found" while the messages are being retrieved. I also have 20 pages of messages so that might be an edge case.

[leekahung]

I'm running this on my local and I'm noticing that when I click on messages from the top nav the Messages section loads with "No Messages Found" while the messages are being retrieved. I also have 20 pages of messages so that might be an edge case.

Oh that's is strange. I'll take a look.

[leekahung]

I'm running this on my local and I'm noticing that when I click on messages from the top nav the Messages section loads with "No Messages Found" while the messages are being retrieved. I also have 20 pages of messages so that might be an edge case.

Not really seeing it in my case (see clip), so not sure what might be going on.

https://github.com/codeforpdx/PASS/assets/14917816/ab84d8b1-7a45-48af-bef1-785a3cc7eb3f

[timbot1789]

I'm running this on my local and I'm noticing that when I click on messages from the top nav the Messages section loads with "No Messages Found" while the messages are being retrieved. I also have 20 pages of messages so that might be an edge case.

This sounds like a performance bottleneck that we're not set up to handle yet. We haven't implemented any kind of server-side pagination, so the network load when fetching multiple pages of records is quite hefty. I don't think it's an issue we can fix in this PR.

[bingeboy]

I'm running this on my local and I'm noticing that when I click on messages from the top nav the Messages section loads with "No Messages Found" while the messages are being retrieved. I also have 20 pages of messages so that might be an edge case.

Not really seeing it in my case (see clip), so not sure what might be going on. Screen.Recording.2023-11-15.at.12.11.43.mov

I've pulled latest code and issue is resolved. Now showing "Loading Messages ..." 👍

[xscottxbrownx]

I haven't tested this branch, but when creating a message - does the modal still behave like it did in very top video here?

The delay between clearing the form and closing the modal.

If so, is that necessary for some reason?

[bingeboy]

This looks good to me. Cleans up Messages. Separates concerns between UI and Solid data. Nice work 👍