Issue 509/splitting messages inbox outbox #551
Conversation
src/AppRoutes.jsx
Outdated
| @@ -43,7 +45,23 @@ const AppRoutes = () => { | |||
| <Route index element={<Contacts />} /> | |||
| <Route path=":webId" element={<Profile />} /> | |||
| </Route> | |||
| <Route path="/messages" element={<Messages />} /> | |||
| <Route path="/messages"> | |||
| {MESSAGE_PAGES_LIST.map((messagePagesProps) => ( | |||
There was a problem hiding this comment.
nit: would this read nicer as messagePage? Then we would get messagePage.path and messagePage.element
There was a problem hiding this comment.
Agreed, definitely a nicer read. I'll have this switched up.
| const path = pathLabel.toLowerCase(); | ||
|
|
||
| setBoxType(path); | ||
| navigate(`/messages/${path}`); |
There was a problem hiding this comment.
To decouple the label and the path (thinking about the future, maybe internationalization, etc.), should we add a path property to the routesArray? e.g.
const routesArray = [
{
label: 'Inbox',
path: 'inbox',
},
{
label: 'Outbox',
path: 'outbox',
}
];
There was a problem hiding this comment.
Then below could send handleNavigate(item) and destructure
There was a problem hiding this comment.
I understand preoptimization is the death of all code, so take this as you will 🙃
There was a problem hiding this comment.
Haha, there's definitely room for improvement. I can switch this up too.
This PR:
Partially addresses #509 similar to PR #514. This PR is for splitting up the message pages into separate sub-routes, while retaining previous functionalities. Have updated unit tests accordingly and created separate tests for Inbox and Outbox.
Screenshots (if applicable):
Screen.Recording.2023-11-29.at.18.40.52.mov
Future Steps/PRs Needed to Finish This Work (optional):
Issues needing discussion/feedback (optional):
1. Issue #509