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.

Sign up for GitHub

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

Jump to bottom

[BugFix] Fixes layout shifting issues #71

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[BugFix] Fixes layout shifting issues #71

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b29bdb4
Fix font flashing bug by pre-loading font
jcalcaben Apr 22, 2023
3180037
Fix layout shift caused by useBreakpointValue() hook
jcalcaben Apr 22, 2023
cf7afc5
Run prettier and remove unused dependency
jcalcaben May 16, 2023
bcaffe2
Run prettier on _app.tsx
jcalcaben May 19, 2023
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
13 changes: 4 additions & 9 deletions components/footer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
Link as ChakraLink,
Text,
useColorModeValue,
useBreakpointValue,
Heading,
} from '@chakra-ui/react'
import { ReactNode } from 'react'
Expand All @@ -19,25 +18,21 @@ export const Footer = () => (
bg={useColorModeValue('gray.50', 'gray.900')}
color={useColorModeValue('gray.700', 'gray.200')}
>
<Container
as={Stack}
maxW={'6xl'}
py={useBreakpointValue({ base: 4, md: 10 })}
>
<Container as={Stack} maxW={'6xl'} py={[4, 10]}>
<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]}>
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

did a yarn install and there are no changes to push up

<Heading as="h2">More Info</Heading>
{FOOTER_ITEMS_MORE_INFO.map((link) => (
<Link href={link.href} key={link.key}>
<Text ml={2}>{link.label}</Text>
</Link>
))}
</Stack>
<Stack order={useBreakpointValue({ base: 2, md: 3 })}>
<Stack order={[2, 3]}>
<Heading as="h2">Support</Heading>
{FOOTER_ITEMS_SUPPORT.map((link) => (
<ChakraLink
Expand All @@ -53,7 +48,7 @@ export const Footer = () => (
<Stack
spacing={6}
gridColumn={{ base: '1 / -1', md: '1' }}
order={useBreakpointValue({ base: 3, md: 1 })}
order={[3, 1]}
>
<Box alignSelf="center">
<Image
Expand Down
19 changes: 11 additions & 8 deletions components/hero.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
Flex,
Text,
VStack,
useBreakpointValue,
Heading,
} from '@chakra-ui/react'

Expand All @@ -30,16 +29,16 @@ const Hero = () => (
>
<Box
boxShadow="2xl"
maxW={useBreakpointValue({ base: 'full', md: 600 })}
p={useBreakpointValue({ base: 4, md: 12 })}
maxW={['full', 600]}
p={[4, 12]}
rounded="md"
bgColor={'blackAlpha.300'}
>
<Heading
as="h1"
color={'orange.300'}
textAlign="center"
fontSize={useBreakpointValue({ base: '6xl', md: '7xl' })}
fontSize={['6xl', '7xl']}
fontWeight={900}
>
Open Austin
Expand All @@ -50,7 +49,7 @@ const Hero = () => (
fontWeight={500}
lineHeight={1.2}
p={4}
fontSize={useBreakpointValue({ base: 'xl', md: 'lg' })}
fontSize={['xl', 'lg']}
>
Open Austin addresses local social and civic challenges through
creative uses of technology. We foster relationships between
Expand All @@ -61,7 +60,7 @@ const Hero = () => (
as="h2"
color={'gray.100'}
textAlign="center"
fontSize={useBreakpointValue({ base: '3xl', md: '5xl' })}
fontSize={['3xl', '5xl']}
fontWeight={700}
pb={4}
>
Expand All @@ -70,10 +69,14 @@ const Hero = () => (

<Stack direction={'row'} align={'flex-end'} justifyContent="center">
<Link href="about" passHref>
<Button variant="primary" as="a">About</Button>
<Button variant="primary" as="a">
About
</Button>
</Link>
<Link href="portfolio" passHref>
<Button variant="primary" as="a">Projects</Button>
<Button variant="primary" as="a">
Projects
</Button>
</Link>
</Stack>
</Box>
Expand Down
15 changes: 14 additions & 1 deletion next.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,17 @@ module.exports = {
reactStrictMode: true,
// exportPathMap: () => {}
swcMinify: true,
}
webpack: (config) => {
config.module = {
...config.module,
rules: [
...config.module.rules,
{
test: /\.(woff|woff2)$/,
type: 'asset/resource'
},
],
};
return config;
},
};
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
"@vercel/analytics": "^0.1.6",
"framer-motion": "^6",
"next": "12.3.1",
"react": "18.2.0",
"react-dom": "18.2.0",
"react-icons": "4.7.1",
"react": "18.2.0"
"react-icons": "4.7.1"
},
"devDependencies": {
"@types/node": "18.7.2",
Expand Down
29 changes: 19 additions & 10 deletions pages/_app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
// Importing fonts (there's probably a better way to do this...)
import '@fontsource/crimson-text'

import "@fontsource/crimson-text";
import crimsonTextLatin from "@fontsource/crimson-text/files/crimson-text-latin-400-normal.woff2";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Next recommends using their @next/Font component for this:

https://nextjs.org/docs/pages/building-your-application/optimizing/fonts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@newswim newswim May 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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 👍


// Setup for Chakra and Next environments
import { ChakraProvider } from '@chakra-ui/react'
import { Analytics } from '@vercel/analytics/react'
import { ChakraProvider } from "@chakra-ui/react";
import { Analytics } from "@vercel/analytics/react";

import { type AppProps } from 'next/app'
import Head from 'next/head'
import { type AppProps } from "next/app";
import Head from "next/head";

import { theme } from '../lib/theme'
import { Header } from '../components/header'
import { Footer } from '../components/footer'
import { theme } from "../lib/theme";
import { Header } from "../components/header";
import { Footer } from "../components/footer";
jcalcaben marked this conversation as resolved.
Show resolved Hide resolved

function MyApp({ Component, pageProps }: AppProps) {
return (
Expand All @@ -22,13 +24,20 @@ function MyApp({ Component, pageProps }: AppProps) {
content="Open Austin is a volunteer citizen brigade advocating for open government, open data, and civic application development since 2009."
/>
<link rel="icon" href="/favicon.ico" />
<link
rel="preload"
href={crimsonTextLatin}
as="font"
type="font/woff2"
crossOrigin="anonymous"
/>
</Head>
<Header />
<Analytics />
<Component {...pageProps} />
<Footer />
</ChakraProvider>
)
);
}

export default MyApp
export default MyApp;