[BugFix] Fixes layout shifting issues #71
Conversation
|
@jcalcaben is attempting to deploy a commit to the open-austin-vercel Team on Vercel. A member of the Team first needs to authorize it. |
|
Hey James! Thanks a lot for taking a look at this. Before I review this, would you mind re-running Project setting is located here: https://github.com/open-austin/website-v2-vercel/blob/main/.prettierrc#L3 |
pages/_app.tsx
Outdated
| import '@fontsource/crimson-text' | ||
|
|
||
| import "@fontsource/crimson-text"; | ||
| import crimsonTextLatin from "@fontsource/crimson-text/files/crimson-text-latin-400-normal.woff2"; |
There was a problem hiding this comment.
I think Next recommends using their @next/Font component for this:
https://nextjs.org/docs/pages/building-your-application/optimizing/fonts
There was a problem hiding this comment.
Error: `@next/font` is only available in Next.js 13 and newer.
Project is currently using 12.3.1
Since it's a major version bump, I hesitate to include it in this PR since it would require regression testing.
There was a problem hiding this comment.
Oh right! I think this is what prompted the creation of #47
In that case, I think what you have here is a good approach 👍
| <SimpleGrid | ||
| templateColumns={{ base: '1fr 1fr', md: '2fr 1fr 1fr' }} | ||
| templateRows={{ base: '1fr 1fr', md: '1fr' }} | ||
| spacing={8} | ||
| > | ||
| <Stack order={useBreakpointValue({ base: 1, md: 2 })}> | ||
| <Stack order={[1, 2]}> |
There was a problem hiding this comment.
I'm not super familiar with this hook, could you give a little bit of context for why you opted to not use useBreakpointValue? I'm sure it's totally valid, I'm just not sure what the trade-offs are.
There was a problem hiding this comment.
From their docs:
All style props accept arrays as values for mobile-first responsive styles. This is the recommended method.
https://chakra-ui.com/docs/styled-system/responsive-styles#the-array-syntax
If I understand the docs for the useBreakpointValue(), this hook is more useful when the calculated values are dynamic instead of static.
Also, how common is it to use custom react hooks inline instead of inside the function body? I've personally never seen it in other projects and code samples, but I've also never worked with the chakra library.
There was a problem hiding this comment.
I agree, it's a little strange. This seems quite a bit more manageable.
I just approved the deployment but it looks like the build is still failing. I'll try to figure out why. I was thinking that once it builds, we may want to check how the site is looking on mobile now.
There was a problem hiding this comment.
Oh, I think you might've bumped the version of React but didn't re-run yarn install, so the yarn.lock is out of sync.
There was a problem hiding this comment.
did a yarn install and there are no changes to push up
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Fixes #35 by pre-loading the font.
Fixes the layout shifting on the homepage and footer by replacing the
useBreakpointValue()hook with the array syntax.