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

feat: replacing minimap with suiteSelector component #383

Merged
merged 12 commits into from
Sep 3, 2024
Merged
10 changes: 5 additions & 5 deletions .github/workflows/publish-storybook.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: Publish Storybook
on:
push:
branches:
- "main" # change to the branch you wish to deploy from
- 'main' # change to the branch you wish to deploy from

permissions:
contents: read
Expand All @@ -14,7 +14,7 @@ jobs:
deploy:
runs-on: ubuntu-latest
steps:
- id: build-publish
uses: bitovi/[email protected]
with:
path: storybook-static
- id: build-publish
uses: bitovi/[email protected]
with:
path: storybook-static
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { Badge } from 'src/components/data-display/Badge/Badge'
import {
type IGlobalNavigationItem,
type IGlobalNavigationLogo,
type IMiniMapOptions,
} from 'src/components/navigation/GlobalNavigation/GlobalNavigationItems'
import { generateOrgs } from 'src/components/navigation/GlobalNavigation/stories-utils'
import { type INavigationOrg } from 'src/components/navigation/GlobalNavigation/WorkspaceSelector/WorkspaceSelectorItems'
Expand Down Expand Up @@ -165,14 +164,11 @@ export const Primary: Story = {
const mpLogo: IGlobalNavigationLogo = {
label: 'Data Platform',
icon: <Icon name="dataPlatform" />,
onSuiteLogoClick: () => {
alert('Going to mP Home!')
},
}

const mpLogoWithBackground: IGlobalNavigationLogo = {
label: 'Data Platform',
icon: 'catalog',
label: 'Overview',
icon: 'overview',
type: 'background-solid',
onSuiteLogoClick: () => {
alert('Going to mP Home!')
Expand Down Expand Up @@ -865,6 +861,40 @@ export const Indicative: Story = {
},
}

export const IndicativeWithSuiteSwitcher: Story = {
args: {
logo: indLogo,
tools: indTools,
management: indManagement,
createItems: indCreateItems,
orgs: indOrgs,
minimapOptions: {
overviewHref: '/',
onLinkClick: link => {
if (link.linkId !== 'analytics') alert(link.href)
},
links: [
{ linkId: 'oversight', href: '/oversight' },
{ linkId: 'dataPlatform', href: '/data-platform' },
{ linkId: 'customer360', href: '/customer-360' },
{ linkId: 'predictions', href: '/predictions' },
{ linkId: 'analytics', href: '/analytics' },
{ linkId: 'segmentation', href: '/segmentation' },
],
activeLink: 'analytics',
},
navigationButtonItemOptions: {
label: 'Custom Signout Label',
onClick: () => {
alert('Signout!')
},
},
onMpHomeClick: () => {
alert('going to overview map')
},
},
}

const cortexLogo: IGlobalNavigationLogo = {
label: 'Predictions',
icon: 'predictions',
Expand Down Expand Up @@ -1208,26 +1238,6 @@ export const MPWithNavSwitcherTour: Story = {
},
}

const minimapOptions: IMiniMapOptions = {
overviewHref: '/',
onLinkClick: link => {
alert(link.href)
},
onUnauthorizedClick: link => {
alert(`unauthorized ${link?.href} `)
},
unauthorizedLinks: ['dataPlatform'],
activeLink: 'oversight',
links: [
{ linkId: 'oversight', href: '/oversight' },
{ linkId: 'dataPlatform', href: '/data-platform' },
{ linkId: 'customer360', href: '/customer-360' },
{ linkId: 'predictions', href: '/predictions' },
{ linkId: 'analytics', href: '/analytics' },
{ linkId: 'segmentation', href: '/segmentation' },
],
}

return (
<div style={{ width: 800 }}>
<GlobalNavigation
Expand All @@ -1242,9 +1252,47 @@ export const MPWithNavSwitcherTour: Story = {
alert('going to overview map')
}}
navigationButtonItemOptions={navigationButtonItemOptions}
minimapOptions={minimapOptions}
/>
</div>
)
},
}

export const MPWithSuiteSelector: Story = {
args: {
onSearchClick: () => {
alert('Searching!')
},
logo: mpLogoWithBackground,
tools: mpTools,
management: mpManagement,
orgs: mpOrgs,
onMpHomeClick: () => {
alert('going to overview map')
},
avatarOptions: {
// src: "https://static-qa1.qa.corp.mparticle.com/appimg/logo_af_916397d2-9732-8de6-77cc-80e3bba120ca.png",
alt: 'avatar',
},
showSuiteLogo: true,
suiteSelectorOptions: {
overviewHref: '/',
onLinkClick: link => {
alert(link.href)
},
onUnauthorizedClick: link => {
alert(`unauthorized ${link?.href} `)
},
unauthorizedLinks: ['dataPlatform'],
activeLink: 'oversight',
links: [
{ linkId: 'oversight', href: '/oversight' },
{ linkId: 'dataPlatform', href: '/data-platform' },
{ linkId: 'customer360', href: '/customer-360' },
{ linkId: 'predictions', href: '/predictions' },
{ linkId: 'analytics', href: '/analytics' },
{ linkId: 'segmentation', href: '/segmentation' },
],
},
},
}
11 changes: 8 additions & 3 deletions src/components/navigation/GlobalNavigation/GlobalNavigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { NavigationCreate } from 'src/components/navigation/GlobalNavigation/Nav
import { WorkspaceSelector } from 'src/components/navigation/GlobalNavigation/WorkspaceSelector/WorkspaceSelector'
import {
type IGlobalNavigationItem,
type IMiniMapOptions,
type ISuiteSelectorOptions,
} from 'src/components/navigation/GlobalNavigation/GlobalNavigationItems'
import { NavigationItem } from 'src/components/navigation/GlobalNavigation/NavigationItem'
import { useNewExperienceReminder } from 'src/hooks/NewExperienceReminder/useNewExperienceReminder'
Expand All @@ -40,7 +40,12 @@ export interface IGlobalNavigationProps {
onClick: () => void
withoutContainer?: boolean
}
minimapOptions?: IMiniMapOptions
suiteSelectorOptions?: ISuiteSelectorOptions
/**
* @deprecated This variant is a temporary fix for new component.
* This will be removed once all the apps updated.
*/
minimapOptions?: ISuiteSelectorOptions
}

export const GlobalNavWidth = 90 as const
Expand All @@ -53,7 +58,7 @@ export const GlobalNavigation = ({ showSuiteLogo = true, ...props }: IGlobalNavi
<div>
{showSuiteLogo && (
<>
<SuiteLogo {...props.logo} minimapOptions={props.minimapOptions} />
<SuiteLogo {...props.logo} suiteSelectorOptions={props.minimapOptions ?? props.suiteSelectorOptions} />
<div className="globalNavigation__divider" />
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface IBaseGlobalNavigationItem {
}

export interface IGlobalNavigationLogo extends IBaseGlobalNavigationItem {
onSuiteLogoClick: () => void
onSuiteLogoClick?: () => void
type?: 'default' | 'background-solid' | 'custom-size'
icon?: ReactElement | keyof typeof Icons
navSwitcherTourOptions?: INavSwitcherTourOptions
Expand All @@ -34,19 +34,19 @@ export interface IGlobalNavigationLink extends IBaseGlobalNavigationItem {

export type IGlobalNavigationItem = IGlobalNavigationMenu | IGlobalNavigationLink

export type MiniMapLinks = 'oversight' | 'dataPlatform' | 'customer360' | 'predictions' | 'analytics' | 'segmentation'
export type MiniMapLink = {
linkId: MiniMapLinks
export type SuiteLinks = 'oversight' | 'dataPlatform' | 'customer360' | 'predictions' | 'analytics' | 'segmentation'
export type SuiteLink = {
linkId: SuiteLinks
href: string
}

export interface IMiniMapOptions {
export interface ISuiteSelectorOptions {
overviewHref: string
links: MiniMapLink[]
onLinkClick: (link: MiniMapLink) => void
onUnauthorizedClick?: (link?: MiniMapLink) => void
unauthorizedLinks?: MiniMapLinks[]
activeLink: MiniMapLinks
links: SuiteLink[]
onLinkClick: (link: SuiteLink) => void
onUnauthorizedClick?: (link?: SuiteLink) => void
unauthorizedLinks?: SuiteLinks[]
activeLink: SuiteLinks
}

export interface INavSwitcherTourOptions extends ITourProps {
Expand Down
68 changes: 27 additions & 41 deletions src/components/navigation/GlobalNavigation/SuiteLogo.tsx
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 {
Expand All @@ -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} />
Copy link
Collaborator

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?

Copy link
Collaborator Author

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

}

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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 LogoWithSuiteSelector function every re-render, then react will un-mount and mount a new LogoWithSuiteSelector component every re-render. This can lead to some weird situations and performance issues.
Alternatives are: change it to a render function like renderLogoWithSuiteSelector(), or move this function to outside of render fn (usually even better to another file). I see you are using local variables inside this fn, so I would suggest the first alternative.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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',
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import React, { type ReactNode } from 'react'
import { type IPopoverProps, Popover } from 'src/components'
import {
type ISuiteSelectorOptions,
type SuiteLink,
} from 'src/components/navigation/GlobalNavigation/GlobalNavigationItems'
import { SuiteSelectorContent } from 'src/components/navigation/GlobalNavigation/SuiteSelector/SuiteSelectorContent'

interface ISuiteSelectorProps extends IPopoverProps {
suiteSelectorOptions: ISuiteSelectorOptions
onLinkClick: (link: SuiteLink) => void
children: ReactNode
}

export function SuiteSelector(props: ISuiteSelectorProps) {
return (
<Popover
content={
<SuiteSelectorContent
overviewHref={props.suiteSelectorOptions.overviewHref}
onUnauthorizedClick={props.suiteSelectorOptions.onUnauthorizedClick}
links={props.suiteSelectorOptions.links}
onLinkClick={props.onLinkClick}
unauthorizedLinks={props.suiteSelectorOptions.unauthorizedLinks}
activeLink={props.suiteSelectorOptions.activeLink}
/>
}
placement="bottomLeft"
open={props.open}
trigger="click"
onOpenChange={props.onOpenChange}
arrow={false}>
{props.children}
</Popover>
)
}
Loading
Loading