-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: replacing minimap with suiteSelector component #383
Changes from 9 commits
9d20a9a
4f5cc65
4e4a2aa
492e499
9787677
b0164f3
164fcd6
dfe5276
edd929e
8934db7
88ca328
6c78a32
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,21 @@ | ||
import React, { type ReactNode, useRef, useState } from 'react' | ||
import { Center, Icon, type ITourProps, Popover, Tour } from 'src/components' | ||
import { Center, Icon, type ITourProps, Tour } from 'src/components' | ||
import { NavigationIcon } from 'src/components/navigation/GlobalNavigation/NavigationIcon' | ||
import MiniMap from 'src/components/navigation/MiniMap/MiniMap' | ||
import { SuiteSelector } from 'src/components/navigation/GlobalNavigation/SuiteSelector/SuiteSelector' | ||
import { type Icons } from 'src/constants/Icons' | ||
import { | ||
type IGlobalNavigationLogo, | ||
type IMiniMapOptions, | ||
type INavSwitcherTourOptions, | ||
type MiniMapLink, | ||
type ISuiteSelectorOptions, | ||
type SuiteLink, | ||
} from 'src/components/navigation/GlobalNavigation/GlobalNavigationItems' | ||
import { type IconColor } from 'src/components/general/Icon/Icon' | ||
|
||
// custom-size is the default size to prevent breaking changes. | ||
type IconColorOptions = 'default' | 'background-solid' | 'custom-size' | ||
|
||
interface SuiteLogoProps extends IGlobalNavigationLogo { | ||
minimapOptions?: IMiniMapOptions | ||
suiteSelectorOptions?: ISuiteSelectorOptions | ||
} | ||
|
||
function isStringIcon(icon: ReactNode | string): icon is keyof typeof Icons { | ||
|
@@ -28,69 +28,55 @@ export function SuiteLogo({ | |
type = 'custom-size', | ||
onSuiteLogoClick, | ||
navSwitcherTourOptions, | ||
minimapOptions, | ||
suiteSelectorOptions, | ||
}: SuiteLogoProps) { | ||
const logoRef = useRef(null) | ||
|
||
if (!minimapOptions || navSwitcherTourOptions?.open) { | ||
return <SuiteLogoContent /> | ||
if (!suiteSelectorOptions || navSwitcherTourOptions?.open) { | ||
return <SuiteLogoContent onLogoClick={onSuiteLogoClick} /> | ||
} | ||
|
||
return ( | ||
<MinimapWithPopover | ||
onUnauthorizedClick={minimapOptions.onUnauthorizedClick} | ||
overviewHref={minimapOptions.overviewHref} | ||
links={minimapOptions.links} | ||
onLinkClick={minimapOptions.onLinkClick} | ||
unauthorizedLinks={minimapOptions.unauthorizedLinks} | ||
activeLink={minimapOptions.activeLink} | ||
/> | ||
) | ||
|
||
function SuiteLogoContent() { | ||
return <LogoWithSuiteSelector {...suiteSelectorOptions} /> | ||
|
||
function SuiteLogoContent({ onLogoClick }: { onLogoClick?: () => void }) { | ||
return ( | ||
<> | ||
<div ref={logoRef}> | ||
{renderNavLogo()} | ||
<NavLogo onLogoClick={onLogoClick} /> | ||
{navSwitcherTourOptions && renderNavTour(navSwitcherTourOptions)} | ||
</div> | ||
</> | ||
) | ||
} | ||
|
||
function MinimapWithPopover(props: IMiniMapOptions) { | ||
function LogoWithSuiteSelector(props: ISuiteSelectorOptions) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not a good practice to have component functions inside the render function, because it will create a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
const [isPopoverOpen, setIsPopoverOpen] = useState(false) | ||
const handleLinkClick = (link: MiniMapLink) => { | ||
|
||
const handleLinkClick = (link: SuiteLink) => { | ||
setIsPopoverOpen(false) | ||
props.onLinkClick(link) | ||
} | ||
|
||
const handlePopoverOpenChange = (newPopoverState: boolean) => { | ||
setIsPopoverOpen(newPopoverState) | ||
} | ||
|
||
const handleLogoClick = () => { | ||
setIsPopoverOpen(prevPopoverState => !prevPopoverState) | ||
} | ||
|
||
return ( | ||
<Popover | ||
content={ | ||
<MiniMap | ||
overviewHref={props.overviewHref} | ||
onUnauthorizedClick={props.onUnauthorizedClick} | ||
links={props.links} | ||
onLinkClick={handleLinkClick} | ||
unauthorizedLinks={props.unauthorizedLinks} | ||
activeLink={props.activeLink} | ||
/> | ||
} | ||
placement="bottomLeft" | ||
<SuiteSelector | ||
open={isPopoverOpen} | ||
trigger="hover" | ||
onOpenChange={handlePopoverOpenChange} | ||
arrow={false}> | ||
<SuiteLogoContent /> | ||
</Popover> | ||
onLinkClick={handleLinkClick} | ||
suiteSelectorOptions={props}> | ||
<NavLogo onLogoClick={handleLogoClick} /> | ||
</SuiteSelector> | ||
) | ||
} | ||
|
||
function renderNavLogo() { | ||
function NavLogo({ onLogoClick }: { onLogoClick?: () => void }) { | ||
const classMap = { | ||
default: '', | ||
'custom-size': 'globalNavigation__icon--suiteLogo', | ||
|
@@ -111,7 +97,7 @@ export function SuiteLogo({ | |
} | ||
|
||
return ( | ||
<Center vertical className="globalNavigation__suiteLogo" onClick={onSuiteLogoClick}> | ||
<Center vertical className="globalNavigation__suiteLogo" onClick={onLogoClick}> | ||
<NavigationIcon icon={getIcon()} label="" hideLabel className={classMap[type]} /> | ||
{label} | ||
</Center> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you add
onLogoClick={onSuiteLogoClick}
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so the idea is: if there is no suite selector, I want to pass the onSuiteLogoClick to the component (like in analytics it's used to navigate to home page). but if there is a suite selector, the click should not be the prop but the functionality to show/hide the suite selector