Navigation component + Page/Layout concept #42
Conversation
sample-app/src/Layout.tsx
Outdated
| })) | ||
| ] | ||
|
|
||
| export default function Layout({ page }: { page: JSX.Element }) { |
There was a problem hiding this comment.
can you add some jsdocs to help describe the responsibility of these different components?
There was a problem hiding this comment.
added some and also renamed BaseRouter -> PageRouter
sample-app/src/BaseRouter.tsx
Outdated
| export default function BaseRouter({ Layout, routes }: PageProps) { | ||
| const pages = routes.map(routeData => { | ||
| const { path, page, exact } = routeData; | ||
| if (Layout) { |
There was a problem hiding this comment.
I'm wondering if the Layout props adds much value to the router. I would expect that a router would simply render what you give it at a given route. I briefly looked into the react router and I didn't see any sort of layout concept, but it's possible I missed something there. What are some of the reasons that lead you to adding Layout to the router?
There was a problem hiding this comment.
I wanted to abstract away react-router details for the the kind of use case we would expect theme 2.0 to have, and wanted a way for a given route's page to have shared components. We had some idea of "layouts" for jambo that eventually fizzled out, but I thought made sense here
sample-app/src/BaseRouter.tsx
Outdated
| interface PageProps { | ||
| Layout?: ComponentType<{ page: JSX.Element }> | ||
| routes: RouteData[] | ||
| } |
There was a problem hiding this comment.
Would it make sense to have a Layout interface defined in typescript? Then the Layout.tsc could implement that and we'd maybe want to rename that Layout since it is a concrete implemenation of the interface?
sample-app/src/Layout.tsx
Outdated
| /** | ||
| * A LayoutComponent that provides a SearchBar and Navigation tabs to a given page. | ||
| */ | ||
| const Layout: LayoutComponent = ({ page }) => { |
There was a problem hiding this comment.
super nit: It might be easy to mix up the Layout here and the Layout in PageProps because one is a concrete implementation and the other is an arbitrary instance of the the LayoutComponent interface. What do you think about naming this StandardLayout in order to help differentiate them?
There was a problem hiding this comment.
I think that would be good too. It would follow StandardCard and CardComponent and how we name Section stuff as well
There was a problem hiding this comment.
yeah I like that! I also moved it to the src/pages folder
This PR adds the navigation component, and also reworks the react-router usage into a Page and Layout system.
For the Navigation resizing, I tried to use resize observer, first with https://www.npmjs.com/package/@react-hook/resize-observer and then with ResizeObserver directly. The third party hook did not seem to work well for our use case,
and for using ResizeObserver directly, it turns out the TypeScript types for ResizeObserver are not included by default,
and I felt a little apprehensive to include third party types for it, though it would probably be fine.
microsoft/TypeScript#28502
J=SLAP-1558
TEST=manual
test resizing the page and seeing tabs appear and disappear
test that switching tabs will run searches