open-saas
open-saas copied to clipboard
Replacing a tags with link tags in AppNavBar
Description
Describe your PR! If it fixes specific issue, mention it with "Fixes # (issue)".
Fixes #254. Replaces anchor tags with link tags to prevent full page reloads when navigating the app navbar on both desktop and mobile.
I had to restructure the navigation array as outbound links can't use the link tags, and must use anchor tags. They are rendered after mapping through all the prior navigation items. Happy to discuss/implement a better solution.
Contributor Checklist
Make sure to do the following steps if they are applicable to your PR:
- [ ] Update e2e tests: If you changed the /template/app, then make sure to do any neccessary updates to /template/e2e-tests also.
- [x] Update demo app: If you changed the /template/app, then make sure to do any neccessary updates to /opensaas-sh/app_diff also. Check /opensaas-sh/README.md for details.
- [ ] Update docs: If needed, update the /opensaas-sh/blog/src/content/docs.
Thanks for the contribution @npenza!
Two things to consider:
- I'm wondering why we can't use the
Linkcomponent that Wasp exports since most (all?) of these links are routes defined in themain.waspfile and could benefit from its typesafety feature. Did you try it out and run into any issues? - We also need to update the landing page, as it has its own, separate "nav bar" in
src/landing-page/components/Header.tsx.
Edit:
Maybe this might be a nice solution, or am I forcing the use of Wasp's Link component here? WDYT @infomiho ?
import { Link, routes, type Routes } from 'wasp/client/router';
//...
interface NavigationItem {
name: string
href: Routes['to']
}
const navigation: NavigationItem[] = [
{ name: 'AI Scheduler (Demo App)', href: '/demo-app'}, // because we typed it, we get autocompleted routes here based on our main.wasp file
{ name: 'File Upload (AWS S3)', href: '/file-upload' },
{ name: 'Pricing', href: '/pricing' },
];
Thanks @vincanger. We definitely can use Wasp's Link component.
Before I make the changes, can you please clarify the differences between .build() and .to for the route objects? E.g this works:
const navigation = [
{ name: 'AI Scheduler (Demo App)', href: routes.DemoAppRoute.to },
{ name: 'File Upload (AWS S3)', href: routes.FileUploadRoute.to },
{ name: 'Pricing', href: routes.PricingPageRoute.to },
];
But this causes a ts error 'Type 'string' is not assignable to type' when mapping through navigation.
const navigation = [
{ name: 'AI Scheduler (Demo App)', href: routes.DemoAppRoute.build() },
{ name: 'File Upload (AWS S3)', href: routes.FileUploadRoute.build() },
{ name: 'Pricing', href: routes.PricingPageRoute.build() },
];
@npenza check out the proposed solution I gave above. The Wasp Link component expects a type which is a union of the route strings, so it functions more like an enum. That's why it won’t accept a general string, which is the error you referenced. The reason the build method throws this error is because it’s typed to return a string, which doesn’t match the union of route strings.