Skip to content

Commit

Permalink
Refactor the fix for two active pills
Browse files Browse the repository at this point in the history
The previous fix used hooks inside loops which should not be done (see
https://legacy.reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level).

As a workaround I decided to refactor the fix. Now we gather all resolved paths
for all the pills that are in the menu using `resolvePath` function and we pass
those paths to each pill component. Inside the component we check if `useMatch`
hook returns anything for the given pill and if yes then we check if is is
because it has asterisk (`*`) in it's path. If yes, then we first have to check
if the url match any of the other pills using our array of resolved paths. If
not then we can safely assume that this pill can be active.
  • Loading branch information
michalsmiarowski committed Jul 10, 2023
1 parent 4cefd91 commit e288ce4
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 50 deletions.
81 changes: 33 additions & 48 deletions src/components/SubNavigationPills/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import {
Stack,
useColorModeValue,
} from "@threshold-network/components"
import { useMatch, useResolvedPath } from "react-router-dom"
import {
matchPath,
resolvePath,
useLocation,
useMatch,
useResolvedPath,
} from "react-router-dom"
import { RouteProps } from "../../types"
import Link from "../Link"

Expand All @@ -16,12 +22,12 @@ interface SubNavigationPillsProps {
}

interface NavPill extends RouteProps {
isActive: boolean
resolvedPaths: string[]
}

const SubNavigationPills: FC<SubNavigationPillsProps> = ({ links }) => {
const linksWithTitle = links.filter((link) => !!link.title)
const navPills = addActiveStatusToPills(linksWithTitle)
const resolvedPaths = getResolvedPathsFromPills(linksWithTitle)
const wrapperBorderColor = useColorModeValue("gray.100", "gray.700")

return (
Expand All @@ -43,14 +49,28 @@ const SubNavigationPills: FC<SubNavigationPillsProps> = ({ links }) => {
height="28px"
as="ul"
>
{navPills.map(renderPill)}
{linksWithTitle.map((linkWithTitle) =>
renderPill(linkWithTitle, resolvedPaths)
)}
</HStack>
</Box>
</>
)
}

const NavPill: FC<NavPill> = ({ path, title, isActive }) => {
const NavPill: FC<NavPill> = ({ path, pathOverride, title, resolvedPaths }) => {
let isActive = false
const resolved = useResolvedPath(pathOverride || path)
const { pathname } = useLocation()
const match = matchPath(resolved.pathname, pathname)
if (match) {
if (match.params["*"]) {
const lastPieceOfPath = match.pathname.replace(match.pathnameBase, "")
if (!resolvedPaths.includes(lastPieceOfPath)) isActive = true
} else {
isActive = true
}
}
const activeColor = useColorModeValue("brand.500", "gray.100")
const underlineColor = useColorModeValue("brand.500", "white")

Expand Down Expand Up @@ -84,53 +104,18 @@ const NavPill: FC<NavPill> = ({ path, title, isActive }) => {
)
}

const renderPill = (pill: NavPill) => <NavPill key={pill.path} {...pill} />
const renderPill = (pill: RouteProps, resolvedPaths: string[]) => (
<NavPill key={pill.path} resolvedPaths={resolvedPaths} {...pill} />
)

/**
* Adds `isActive` property to each of the link. If there are two or more links
* that are active (based on `useMatch` hook) then we only keep the active
* status for the last one.
* @param {RouteProps[]} pills Array of links (RouteProps)
* @return {RouteProps[]} Array of links with active status added to each of them. Only one
* link can have an active status.
*/
const addActiveStatusToPills = (pills: RouteProps[]) => {
const pillsWithActiveStatus: NavPill[] = []
const lastActivePill: {
index: number | undefined
pathnameBase: string
} = {
index: undefined,
pathnameBase: "",
}
const getResolvedPathsFromPills = (pills: RouteProps[]) => {
const resolvedPaths: string[] = []
for (let i = 0; i < pills.length; i++) {
const { path, pathOverride } = pills[i]
const resolved = useResolvedPath(pathOverride || path)
const match = useMatch({ path: resolved.pathname, end: true })
// The second condition here checks if the current match pathnameBase
// includes the pathnameBase of the last active pill. If it does, then this
// pill will be active and we will remove the active status from the
// previous pill. This means that if we have multiple paths that start with
// the same page, such as "staking/how-it-works" and "staking", the active
// one will be the one that useMatch returns true for and has the longest
// pathnameBase (so in this case it will be `staking/how-it-works`).
const isActive =
!!match && match.pathnameBase.includes(lastActivePill.pathnameBase)
if (isActive) {
// Remove the active status of the previous pill
if (lastActivePill.index !== undefined) {
pillsWithActiveStatus[lastActivePill.index].isActive = false
}
lastActivePill.index = i
lastActivePill.pathnameBase = match.pathnameBase
}
const pillWithActiveStatus = {
...pills[i],
isActive,
}
pillsWithActiveStatus.push(pillWithActiveStatus)
const resolved = resolvePath(pathOverride || path)
resolvedPaths.push(resolved.pathname)
}
return pillsWithActiveStatus
return resolvedPaths
}

export default SubNavigationPills
4 changes: 2 additions & 2 deletions src/pages/tBTC/Bridge/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useEffect } from "react"
import { Grid, Box, Skeleton, Stack } from "@threshold-network/components"
import { Grid, Box } from "@threshold-network/components"
import { PageComponent } from "../../../types"
import { TbtcBalanceCard } from "./TbtcBalanceCard"
import { MintUnmintNav } from "./MintUnmintNav"
Expand Down Expand Up @@ -70,7 +70,7 @@ const TBTCBridge: PageComponent = (props) => {
TBTCBridge.route = {
path: "",
index: false,
pathOverride: "/tBTC/*",
pathOverride: "*",
pages: [MintPage, UnmintPage],
title: "Bridge",
isPageEnabled: true,
Expand Down

0 comments on commit e288ce4

Please sign in to comment.