Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement router transitions #358

Merged
merged 6 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 8 additions & 21 deletions dapp/src/DApp.tsx
Original file line number Diff line number Diff line change
@@ -1,36 +1,23 @@
import React from "react"
import { Box, ChakraProvider } from "@chakra-ui/react"
import { ChakraProvider } from "@chakra-ui/react"
import { Provider as ReduxProvider } from "react-redux"
import { RouterProvider } from "react-router-dom"
import { useInitApp } from "./hooks"
import { store } from "./store"
import theme from "./theme"
import { AcreSdkProvider } from "./acre-react/contexts"
import GlobalStyles from "./components/GlobalStyles"
import {
DocsDrawerContextProvider,
LedgerWalletAPIProvider,
SidebarContextProvider,
WalletContextProvider,
} from "./contexts"
import { AcreSdkProvider } from "./acre-react/contexts"
import Header from "./components/Header"
import Sidebar from "./components/Sidebar"
import DocsDrawer from "./components/DocsDrawer"
import GlobalStyles from "./components/GlobalStyles"
import { router } from "./router"
import { useInitApp } from "./hooks"
import { Router } from "./router"
import { store } from "./store"
import theme from "./theme"

function DApp() {
useInitApp()

return (
<>
<Header />
<Box as="main">
<RouterProvider router={router} />
</Box>
<Sidebar />
<DocsDrawer />
</>
)
return <Router />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case when DApp returns only the Router component maybe we should consider moving this logic to the parent and then deleting this file simplifying the structure. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I'd opt for slightly different solution - to leave routing separated as is - routing is kind of hermetic area with it's "natural" namespace and it's good to have it separated in case the application scales and the need of new routes might occur.
My suggestion is to remove the useInitApp hook. Now when DApp component has such a simple structure this hook is only a redundant wrapper to click through when inspecting the code.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather leave useInitApp as it is. I'm considering whether DAppProviders should return GlobalStyles(it's not a provider), so maybe we should move it to DApp component to be semantically correct.

Copy link
Contributor Author

@kpyszkowski kpyszkowski Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ref commit: eb37fa9

}

function DAppProviders() {
Expand Down
45 changes: 45 additions & 0 deletions dapp/src/components/shared/Layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import React, { useState } from "react"
import { AnimatePresence, motion, Variants } from "framer-motion"
import { useLocation, useOutlet } from "react-router-dom"
import Header from "../Header"
import DocsDrawer from "../DocsDrawer"
import Sidebar from "../Sidebar"

const wrapperVariants: Variants = {
in: { opacity: 0, y: 48 },
out: { opacity: 0, y: -48 },
visible: { opacity: 1, y: 0 },
}

// This tricky component makes Outlet persistent so React and Framer Motion can
// distinguish wheather it should be rerendered between routes.
// Ref: https://github.com/remix-run/react-router/discussions/8008#discussioncomment-1280897
function PersistentOutlet() {
const [outlet] = useState(useOutlet())
ioay marked this conversation as resolved.
Show resolved Hide resolved
return outlet
}

function Layout() {
const location = useLocation()
return (
<>
<Header />
<AnimatePresence mode="popLayout">
<motion.main
key={location.pathname}
variants={wrapperVariants}
transition={{ type: "spring", damping: 12, stiffness: 120 }}
initial="in"
animate="visible"
exit="out"
>
<PersistentOutlet />
</motion.main>
</AnimatePresence>
<Sidebar />
<DocsDrawer />
</>
)
}

export default Layout
28 changes: 17 additions & 11 deletions dapp/src/router/index.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
import React from "react"
import { createBrowserRouter } from "react-router-dom"
import { BrowserRouter, Route, Routes } from "react-router-dom"
import OverviewPage from "#/pages/OverviewPage"
import ActivityPage from "#/pages/ActivityPage"
import Layout from "#/components/shared/Layout"
import { routerPath } from "./path"

export const router = createBrowserRouter([
{
path: routerPath.home,
element: <OverviewPage />,
},
{
path: `${routerPath.activity}/:activityId`,
element: <ActivityPage />,
},
])
export function Router() {
return (
<BrowserRouter>
<Routes>
<Route path="/" element={<Layout />}>
<Route index path={routerPath.home} element={<OverviewPage />} />
<Route
path={`${routerPath.activity}/:activityId`}
element={<ActivityPage />}
/>
</Route>
</Routes>
</BrowserRouter>
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it was changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With <Layout /> parent route nested routes in JSX are easier to read and maintain instead of object notation (array of children) elements when using createBrowserRouter factory function.

Loading