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

performance: avoid desktop/mobile menus unnecesary renderings #13042

Merged
merged 7 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
28 changes: 28 additions & 0 deletions src/components/LanguagePicker/MobileCloseBar.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { MouseEventHandler } from "react"
import { useTranslation } from "next-i18next"
import { Flex } from "@chakra-ui/react"

import { Button } from "@/components/Buttons"

type MobileCloseBarProps = {
handleClick: MouseEventHandler<HTMLButtonElement>
}

export const MobileCloseBar = ({ handleClick }: MobileCloseBarProps) => {
const { t } = useTranslation()

return (
<Flex
justifyContent="end"
hideFrom="md"
position="sticky"
zIndex="sticky"
top="0"
bg="background.base"
>
<Button p="4" variant="ghost" alignSelf="end" onClick={handleClick}>
{t("close")}
</Button>
</Flex>
)
}
59 changes: 25 additions & 34 deletions src/components/LanguagePicker/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@ import {
useEventListener,
} from "@chakra-ui/react"

import { Button } from "@/components/Buttons"
import { LocaleDisplayInfo } from "@/lib/types"

import { BaseLink } from "@/components/Link"

import { isMobile } from "@/lib/utils/isMobile"

import MenuItem from "./MenuItem"
import { MobileCloseBar } from "./MobileCloseBar"
import NoResultsCallout from "./NoResultsCallout"
import { useLanguagePicker } from "./useLanguagePicker"

Expand Down Expand Up @@ -59,6 +63,19 @@ const LanguagePicker = ({
inputRef.current?.focus()
})

// onClick handlers
const handleMobileCloseBarClick = () => onClose()
const handleMenuItemClose = (displayInfo: LocaleDisplayInfo) =>
onClose({
eventAction: "Locale chosen",
eventName: displayInfo.localeOption,
})
const handleBaseLinkClose = () =>
onClose({
eventAction: "Translation program link (menu footer)",
eventName: "/contributing/translation-program",
})

return (
<Menu isLazy placement={placement} autoSelect={false} {...disclosure}>
{children}
Expand All @@ -76,23 +93,10 @@ const LanguagePicker = ({
{...props}
>
{/* Mobile Close bar */}
<Flex
justifyContent="end"
hideFrom="md"
position="sticky"
zIndex="sticky"
top="0"
bg="background.base"
>
<Button
p="4"
variant="ghost"
alignSelf="end"
onClick={() => onClose()}
>
{t("common:close")}
</Button>
</Flex>
{/* avoid rendering mobile only feature on desktop */}
{isMobile() && (
<MobileCloseBar handleClick={handleMobileCloseBarClick} />
)}

{/* Main Language selection menu */}
<Box
Expand Down Expand Up @@ -146,10 +150,7 @@ const LanguagePicker = ({
}}
onFocus={handleInputFocus}
/>
<InputRightElement
hideBelow="md"
cursor="text"
>
<InputRightElement hideBelow="md" cursor="text">
<Kbd
fontSize="sm"
lineHeight="none"
Expand Down Expand Up @@ -177,12 +178,7 @@ const LanguagePicker = ({
e.preventDefault()
inputRef.current?.focus()
}}
onClick={() =>
onClose({
eventAction: "Locale chosen",
eventName: displayInfo.localeOption,
})
}
onClick={() => handleMenuItemClose(displayInfo)}
/>
))}

Expand Down Expand Up @@ -215,12 +211,7 @@ const LanguagePicker = ({
<BaseLink
ref={footerRef}
href="/contributing/translation-program"
onClick={() =>
onClose({
eventAction: "Translation program link (menu footer)",
eventName: "/contributing/translation-program",
})
}
onClick={handleBaseLinkClose}
>
{t("common:learn-more")}
</BaseLink>
Expand Down
3 changes: 3 additions & 0 deletions src/components/Nav/Menu/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ const Menu = ({ sections, ...props }: NavMenuProps) => {
</Text>
</Button>
</NavigationMenu.Trigger>

{/* avoid rendering desktop menu on mobile */}
{/* Desktop Menu content */}
pettinarip marked this conversation as resolved.
Show resolved Hide resolved
<NavigationMenu.Content asChild>
{/**
* This is the CONTAINER for all three menu levels
Expand Down
129 changes: 68 additions & 61 deletions src/components/Nav/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import LanguagePicker from "@/components/LanguagePicker"
import { BaseLink } from "@/components/Link"
import Search from "@/components/Search"

import { isMobile } from "@/lib/utils/isMobile"

import { DESKTOP_LANGUAGE_BUTTON_NAME, NAV_PY } from "@/lib/constants"

import Menu from "./Menu"
Expand All @@ -33,6 +35,7 @@ const Nav = () => {
const { toggleColorMode, linkSections, mobileNavProps } = useNav()
const { locale } = useRouter()
const { t } = useTranslation("common")
const isDesktop = !isMobile()
const searchModalDisclosure = useDisclosure()
const navWrapperRef = useRef(null)
const languagePickerState = useDisclosure()
Expand Down Expand Up @@ -95,76 +98,80 @@ const Nav = () => {
justifyContent={{ base: "flex-end", md: "space-between" }}
ms={{ base: 3, xl: 8 }}
>
<Menu hideBelow="md" sections={linkSections} />
{/* avoid rendering desktop Menu version on mobile */}
{isDesktop && <Menu hideBelow="md" sections={linkSections} />}

<Flex alignItems="center" /* justifyContent="space-between" */>
<Search {...searchModalDisclosure} />

{/* Desktop */}
<HStack hideBelow="md" gap="0">
<IconButton
transition="transform 0.5s, color 0.2s"
icon={ThemeIcon}
aria-label={themeIconAriaLabel}
variant="ghost"
isSecondary
px={{ base: "2", xl: "3" }}
_hover={{
transform: "rotate(10deg)",
color: "primary.hover",
}}
onClick={toggleColorMode}
/>

{/* Locale-picker menu */}
<LanguagePicker
placement="bottom-end"
minH="unset"
maxH="75vh"
w="xs"
inset="unset"
top="unset"
menuState={languagePickerState}
>
<MenuButton
as={Button}
name={DESKTOP_LANGUAGE_BUTTON_NAME}
ref={languagePickerRef}
{/* avoid rendering desktop LanguagePicker version on mobile */}
{isDesktop && (
<HStack hideBelow="md" gap="0">
<IconButton
transition="transform 0.5s, color 0.2s"
icon={ThemeIcon}
aria-label={themeIconAriaLabel}
variant="ghost"
color="body.base"
transition="color 0.2s"
isSecondary
px={{ base: "2", xl: "3" }}
_hover={{
transform: "rotate(10deg)",
color: "primary.hover",
"& svg": {
transform: "rotate(10deg)",
transition: "transform 0.5s",
},
}}
_active={{
color: "primary.hover",
bg: "primary.lowContrast",
}}
sx={{
"& svg": {
transform: "rotate(0deg)",
transition: "transform 0.5s",
},
}}
onClick={toggleColorMode}
/>

{/* Locale-picker menu */}
<LanguagePicker
placement="bottom-end"
minH="unset"
maxH="75vh"
w="xs"
inset="unset"
top="unset"
menuState={languagePickerState}
>
<Icon
as={BsTranslate}
fontSize="2xl"
verticalAlign="middle"
me={2}
/>
<Text hideBelow="lg" as="span">
{t("common:languages")}&nbsp;
</Text>
{locale!.toUpperCase()}
</MenuButton>
</LanguagePicker>
</HStack>
{/* Mobile */}
<MenuButton
as={Button}
name={DESKTOP_LANGUAGE_BUTTON_NAME}
ref={languagePickerRef}
variant="ghost"
color="body.base"
transition="color 0.2s"
px={{ base: "2", xl: "3" }}
_hover={{
color: "primary.hover",
"& svg": {
transform: "rotate(10deg)",
transition: "transform 0.5s",
},
}}
_active={{
color: "primary.hover",
bg: "primary.lowContrast",
}}
sx={{
"& svg": {
transform: "rotate(0deg)",
transition: "transform 0.5s",
},
}}
>
<Icon
as={BsTranslate}
fontSize="2xl"
verticalAlign="middle"
me={2}
/>
<Text hideBelow="lg" as="span">
{t("common:languages")}&nbsp;
</Text>
{locale!.toUpperCase()}
</MenuButton>
</LanguagePicker>
</HStack>
)}

<MobileNavMenu
{...mobileNavProps}
linkSections={linkSections}
Expand Down
Loading