Skip to content
This repository was archived by the owner on Apr 17, 2025. It is now read-only.

Sidebar improvement #294

Merged
Damola18 merged 4 commits into from
Jan 16, 2025
Merged

Sidebar improvement #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f3270d1
fix sidebar nav hover state
Damola18 Jan 15, 2025
4290337
add spacing
Damola18 Jan 15, 2025
dcb86f1
fix lint
Damola18 Jan 15, 2025
68cb639
Refactor SidebarItem to handle external links properly
Damola18 Jan 15, 2025
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
2 changes: 1 addition & 1 deletion src/components/client/TreasuryPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const TreasuryPage: NextPage = () => {
href="https://etherscan.io/address/0x56398b89d53e8731bca8c1b06886cfb14bd6b654"
isExternal
>
IQ.eth
IQ.eth{' '}
</Link>
</Box>
<Box as="span">{t('treasury')}</Box>
Expand Down
84 changes: 48 additions & 36 deletions src/components/dashboard/sidebar-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Flex, FlexProps, Icon, LinkBox, Image } from '@chakra-ui/react'
import { dataAttr } from '@chakra-ui/utils'
import { usePathname } from 'next/navigation'
import React from 'react'
import Link from 'next/link'
import LinkOverlay from '../elements/LinkElements/LinkOverlay'
import { useTranslations } from 'next-intl'

Expand All @@ -20,46 +21,57 @@ export const SidebarItem = (props: SidebarItemProps) => {

const pathname = usePathname()
const isActiveRoute = pathname?.endsWith(item.route)
const isExternalLink = item.route.startsWith('http')
Copy link
Contributor

@Yomzy2223 Yomzy2223 Jan 16, 2025

Choose a reason for hiding this comment

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

Since internal links can also start with 'https', I think this will be a better way to check for external links:

  const isExternalLink = (url: string) => {
    try {
      let currentHost = window.location.hostname
      currentHost = currentHost.replace('localhost', 'iq.iqai.com')
      const urlHost = new URL(url).hostname
      return currentHost !== urlHost
    } catch {
      return false
    }
  }
  const isExternal = isExternalLink(item.route)


const renderLinkContent = () => (
<Flex
h="40px"
align="center"
pl={{ base: 5, lg: '15' }}
cursor="pointer"
data-active={dataAttr(isActiveRoute)}
color="grayText"
fontWeight="medium"
_hover={{
bg: 'divider',
color: 'dimmedText',
}}
_active={{
bg: 'hoverBg',
color: 'brandText',
}}
transition="all .2s ease"
position="relative"
gap="2"
>
{typeof item.icon === 'string' ? (
<Image src={item.icon} boxSize="6" alt="icon" />
) : (
<Icon as={item.icon} boxSize="6" />
)}
{t(item.label)}
{isActiveRoute && (
<NavIndicator position="absolute" right="0" bg="transparent" />
)}
</Flex>
)

return (
<LinkBox {...rest}>
<LinkOverlay href={item.route} target={item.target} onClick={onClose}>
<Flex
h="40px"
align="center"
pl={{ base: 5, lg: '15' }}
cursor="pointer"
data-active={dataAttr(isActiveRoute)}
color="grayText"
fontWeight="medium"
_hover={{
bg: 'divider',
color: 'dimmedText',
}}
_active={{
bg: 'hoverBg',
color: 'brandText',
}}
transition="all .2s ease"
role="group"
display="flex"
gap="2"
{isExternalLink ? (
Copy link
Contributor

@Yomzy2223 Yomzy2223 Jan 16, 2025

Choose a reason for hiding this comment

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

Then instead of the condition here, we can do this:

 const linkProps = {
   href: item.route,
   onClick: onClose,
   ...(isExternal && {
     target: '_blank',
     rel: 'noopener noreferrer',
   }),
 }

 <LinkBox {...rest}>
     <Link {...linkProps}>
       <Flex
         h="40px"
         align="center"
         pl={{ base: 5, lg: '15' }}
         cursor="pointer"
         data-active={dataAttr(isActiveRoute)}
         color="grayText"
         fontWeight="medium"
         _hover={{
           bg: 'divider',
           color: 'dimmedText',
         }}
         _active={{
           bg: 'hoverBg',
           color: 'brandText',
         }}
         transition="all .2s ease"
         position="relative"
         gap="2"
       >
         {typeof item.icon === 'string' ? (
           <Image src={item.icon} boxSize="6" alt="icon" />
         ) : (
           <Icon as={item.icon} boxSize="6" />
         )}
         {t(item.label)}
         {isActiveRoute && (
           <NavIndicator position="absolute" right="0" bg="transparent" />
         )}
       </Flex>
     </Link>
   </LinkBox>

What do you think?

<Link
href={item.route}
target="_blank"
rel="noopener noreferrer"
onClick={onClose}
>
{typeof item.icon === 'string' ? (
<Image src={item.icon} boxSize="6" alt="icon" />
) : (
<Icon as={item.icon} boxSize="6" />
)}
{t(item.label)}
<NavIndicator
display="none"
_groupActive={{
display: 'inherit',
}}
ml="auto"
/>
</Flex>
</LinkOverlay>
{renderLinkContent()}
</Link>
) : (
<LinkOverlay href={item.route} target={item.target} onClick={onClose}>
{renderLinkContent()}
</LinkOverlay>
)}
</LinkBox>
)
}
Loading