Move all router access to page-level components #712
Conversation
Deployed to Cloudflare Pages
|
csillag
changed the title
csillag
force-pushed
the
csillag/page-level-cleanup
branch
4 times, most recently
from
ef72418
to
8dad915
Compare
csillag
force-pushed
the
csillag/page-level-cleanup
branch
2 times, most recently
from
25ba0c9
to
b897412
Compare
|
I think we should also move most data loading into the same page-level components in order to reduce redundancy in the code, but I will do that in a separate PR. |
|
A technically interesting detail: we can also use our tabs (wrapping React Router's |
|
|
||
| export const accountTransactionsContainerId = 'transactions' | ||
|
|
||
| export const AccountTransactionsCard: FC = () => { | ||
| const { scope, address } = useOutletContext<AccountDetailsContext>() |
There was a problem hiding this comment.
I don't actually see outlet context as better than getting things from URL. It looks less straightforward :/
There was a problem hiding this comment.
My thinking was that this is part of hierarchical navigation; the outer part (the page) is containing the inner part (the tabs). Therefore if the outer part already go the data from the router, it's only fair that it passes it down to it's children. Doing this, we will be free to move the page wherever we want in the navigation tree, and the only thing we have to update is the top part.
On the other hand, one could also make the argument that since the tabs are references directly inside the routing tree definition, these are routing level components, on equal level as other page-level components, so they can talk to the router directly.
I'm happy to undo the outlet-based changes.
There was a problem hiding this comment.
Lemme try out one more version
There was a problem hiding this comment.
@lukaw3d How about now? This time, all cards (both those that are embedded on the page + those that are used in tab menus) take props the same way, normally. The props are passed down from the menu, and received in the routes definition.
I think the actual components are very easy to read now. They don't have to interact with router, outlet, or anything. All the tricky parts are confined into the router itself, when we are already concentrating on building a menu.
csillag
force-pushed
the
csillag/page-level-cleanup
branch
2 times, most recently
from
667ca20
to
1c9bccc
Compare
| @@ -83,28 +83,23 @@ export const routes: RouteObject[] = [ | |||
| children: [ | |||
| { | |||
| path: '', | |||
| element: <AccountTransactionsCard />, | |||
| loader: addressParamLoader, | |||
| Component: () => <AccountTransactionsCard {...useAccountDetailsProps()} />, | |||
There was a problem hiding this comment.
This is nice. Hopefully react-router doesn't break it (this is v5 remnant)
csillag
force-pushed
the
csillag/page-level-cleanup
branch
from
1c9bccc
to
45fbd64
Compare
We should only be accessing the URL (via the router) from page-level components, not individual components.
This improves the composability of the app.
Cleanup progress: