From 9c76f3958edf757c1bdeeb847ef74347e64a4ecb Mon Sep 17 00:00:00 2001 From: Zach Shilton <4624598+zchsh@users.noreply.github.com> Date: Fri, 4 Aug 2023 17:44:10 -0400 Subject: [PATCH 1/4] chore: begin cleanup of MenuItem interface --- .../sidebar-nav-menu-item/index.tsx | 80 ++++++++++++------- .../components/sidebar-nav-menu-item/types.ts | 9 +-- src/components/sidebar/index.tsx | 9 ++- src/components/sidebar/types.ts | 13 ++- .../docs/add-branded-overview-sidebar-item.ts | 7 +- src/views/onboarding/tutorial-view/server.ts | 2 +- .../tutorial-view/server.ts | 18 +++-- 7 files changed, 91 insertions(+), 47 deletions(-) diff --git a/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx b/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx index 03948803b4..e79aa465c7 100644 --- a/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx +++ b/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx @@ -18,7 +18,7 @@ import { ProductSlug } from 'types/products' import isAbsoluteUrl from 'lib/is-absolute-url' import Badge from 'components/badge' import Link from 'components/link' -import { MenuItem } from 'components/sidebar' +import { MenuItem, MenuItemOptionalProperties } from 'components/sidebar' import ProductIcon from 'components/product-icon' import { SidebarHorizontalRule, @@ -31,7 +31,6 @@ import { SidebarNavLinkItemProps, SidebarNavMenuButtonProps, SidebarNavMenuItemBadgeProps, - SidebarNavMenuItemProps, SupportedIconName, } from './types' import s from './sidebar-nav-menu-item.module.css' @@ -198,18 +197,41 @@ export function SidebarNavMenuButton({ item }: SidebarNavMenuButtonProps) { ) } +/** + * Given a MenuItem, + * Return `true` if the item should be shown "open" by default, + * or `false` otherwise. + * + * TODO: update input `item` type to be the "submenu" item type specifically. + */ +function getDefaultOpen(item: MenuItemOptionalProperties): boolean { + const defaultOpenProps = [ + 'isOpen', + 'hasActiveChild', + 'hasChildrenMatchingFilter', + 'matchesFilter', + ] + const isDefaultOpen = defaultOpenProps.reduce((acc, prop) => { + if (item[prop]) { + return true + } + return acc + }, false) + return isDefaultOpen +} + /** * Handles rendering a collapsible/expandable submenu item and its child menu * items in the Sidebar. */ -const SidebarNavSubmenuItem = ({ item }: SidebarNavMenuItemProps) => { +const SidebarNavSubmenuItem = ({ + item, +}: { + // TODO: update this `item` type to be the "submenu" item type specifically. + item: MenuItemOptionalProperties +}) => { const buttonRef = useRef() - const [isOpen, setIsOpen] = useState( - item.isOpen || - item.hasActiveChild || - item.hasChildrenMatchingFilter || - item.matchesFilter - ) + const [isOpen, setIsOpen] = useState(getDefaultOpen(item)) const hasBadge = !!(item as $TSFixMe).badge /** @@ -218,18 +240,8 @@ const SidebarNavSubmenuItem = ({ item }: SidebarNavMenuItemProps) => { * if we pass the props needed instead of just the item object? */ useEffect(() => { - setIsOpen( - item.isOpen || - item.hasActiveChild || - item.hasChildrenMatchingFilter || - item.matchesFilter - ) - }, [ - item.isOpen, - item.hasActiveChild, - item.hasChildrenMatchingFilter, - item.matchesFilter, - ]) + setIsOpen(getDefaultOpen(item)) + }, [item]) const handleKeyDown: KeyboardEventHandler = (e) => { if (e.key === 'Escape') { @@ -272,7 +284,19 @@ const SidebarNavSubmenuItem = ({ item }: SidebarNavMenuItemProps) => { {isOpen && ( @@ -289,15 +313,15 @@ const SidebarNavSubmenuItem = ({ item }: SidebarNavMenuItemProps) => { * - SidebarNavSubmenu * - SidebarNavLink */ -const SidebarNavMenuItem = ({ item }: SidebarNavMenuItemProps) => { +const SidebarNavMenuItem = ({ item }: { item: MenuItem }) => { let itemContent - if (item.divider) { + if ('divider' in item) { itemContent = - } else if (item.heading) { + } else if ('heading' in item) { itemContent = - } else if (item.routes) { + } else if ('routes' in item) { itemContent = - } else if (item.theme) { + } else if ('theme' in item) { itemContent = ( { itemContent = } - return
  • {itemContent}
    • + return
  • {itemContent}
    • } export { SidebarNavLinkItem, SidebarNavSubmenuItem } diff --git a/src/components/sidebar/components/sidebar-nav-menu-item/types.ts b/src/components/sidebar/components/sidebar-nav-menu-item/types.ts index c977998e38..07457c7dba 100644 --- a/src/components/sidebar/components/sidebar-nav-menu-item/types.ts +++ b/src/components/sidebar/components/sidebar-nav-menu-item/types.ts @@ -6,7 +6,7 @@ import { ReactElement } from 'react' import { IconChevronDown16 } from '@hashicorp/flight-icons/svg-react/chevron-down-16' import Badge, { BadgeProps } from 'components/badge' -import { MenuItem } from 'components/sidebar' +import { MenuItemOptionalProperties } from 'components/sidebar' import { ProductSlug } from 'types/products' interface RightIconsContainerProps { @@ -23,12 +23,8 @@ interface SidebarNavMenuItemBadgeProps { type?: BadgeProps['type'] } -interface SidebarNavMenuItemProps { - item: MenuItem -} - type SupportedIconName = 'home' & 'guide' & ProductSlug -interface SidebarNavLinkItem extends MenuItem { +interface SidebarNavLinkItem extends MenuItemOptionalProperties { leadingIconName?: SupportedIconName } @@ -45,6 +41,5 @@ export type { SidebarNavMenuButtonProps, SidebarNavLinkItemProps, SidebarNavMenuItemBadgeProps, - SidebarNavMenuItemProps, SupportedIconName, } diff --git a/src/components/sidebar/index.tsx b/src/components/sidebar/index.tsx index 8113ed24f8..62a86dbc68 100644 --- a/src/components/sidebar/index.tsx +++ b/src/components/sidebar/index.tsx @@ -22,7 +22,12 @@ import { } from 'components/sidebar/components' // Local imports -import { FilteredNavItem, MenuItem, SidebarProps } from './types' +import { + FilteredNavItem, + MenuItem, + MenuItemOptionalProperties, + SidebarProps, +} from './types' import { addNavItemMetaData, getFilteredNavItems, @@ -146,5 +151,5 @@ const Sidebar = ({ ) } -export type { MenuItem, SidebarProps } +export type { MenuItem, MenuItemOptionalProperties, SidebarProps } export default Sidebar diff --git a/src/components/sidebar/types.ts b/src/components/sidebar/types.ts index ce48a9785c..590022e913 100644 --- a/src/components/sidebar/types.ts +++ b/src/components/sidebar/types.ts @@ -124,7 +124,7 @@ export interface NavHighlightItem { * with optional properties to cover all possible menu item types. * ref: https://app.asana.com/0/1202097197789424/1202405210286689/f */ -interface MenuItem { +interface MenuItemOptionalProperties { divider?: boolean fullPath?: string hasActiveChild?: boolean @@ -136,7 +136,6 @@ interface MenuItem { path?: string routes?: MenuItem[] title?: string - heading?: string isOpen?: boolean /** * Optional icon to display at right of the menu item. @@ -150,6 +149,15 @@ interface MenuItem { theme?: NavHighlightItem['theme'] } +/** + * TODO: build out this revised MenuItem type to capture all of the + * different things `MenuItemOptionalProperties`, but with a union + * of more concrete and distinguishable types instead. + * + * Task: https://app.asana.com/0/1202097197789424/1202405210286689/f + */ +type MenuItem = MenuItemOptionalProperties | HeadingNavItem + interface SidebarBaseProps { /** * Optional props to send to `SidebarBackToLink` which is displayed at the top @@ -219,6 +227,7 @@ export type { EnrichedSubmenuNavItem, FilteredNavItem, LinkNavItemWithMetaData, + MenuItemOptionalProperties, MenuItem, NavItemWithMetaData, SidebarProps, diff --git a/src/lib/docs/add-branded-overview-sidebar-item.ts b/src/lib/docs/add-branded-overview-sidebar-item.ts index df8b15368b..eb9a1054c3 100644 --- a/src/lib/docs/add-branded-overview-sidebar-item.ts +++ b/src/lib/docs/add-branded-overview-sidebar-item.ts @@ -4,12 +4,15 @@ */ import { ProductSlug } from 'types/products' -import { MenuItem, EnrichedNavItem } from 'components/sidebar/types' +import { MenuItem } from 'components/sidebar/types' /** * Determine whether a `menuItem` is an "overview" item. */ function isOverviewItem(item: MenuItem) { + if (!('path' in item)) { + return false + } const isPathMatch = item.path === '' || item.path === '/' || @@ -22,7 +25,7 @@ function isOverviewItem(item: MenuItem) { * Determine whether a `menuItem` is an "heading" item. */ function isHeadingItem(item: MenuItem) { - return typeof item.heading == 'string' + return 'heading' in item && typeof item.heading == 'string' } /** diff --git a/src/views/onboarding/tutorial-view/server.ts b/src/views/onboarding/tutorial-view/server.ts index da3277f38b..cdbb44e9ea 100644 --- a/src/views/onboarding/tutorial-view/server.ts +++ b/src/views/onboarding/tutorial-view/server.ts @@ -102,7 +102,7 @@ export async function getOnboardingTutorialProps( // handle next sidebar collection logic if (isLastTutorial) { const currentIndex = tutorialNavLevelMenuItems.findIndex( - (item: MenuItem) => item.fullPath === `/${currentCollection.slug}` + (item) => item.fullPath === `/${currentCollection.slug}` ) const nextSidebarItem: MenuItem = tutorialNavLevelMenuItems[currentIndex + 1] diff --git a/src/views/well-architected-framework/tutorial-view/server.ts b/src/views/well-architected-framework/tutorial-view/server.ts index 0217564ce7..6558c1440f 100644 --- a/src/views/well-architected-framework/tutorial-view/server.ts +++ b/src/views/well-architected-framework/tutorial-view/server.ts @@ -17,7 +17,7 @@ import { TutorialLite as ClientTutorialLite, } from 'lib/learn-client/types' import { generateTopLevelSidebarNavData } from 'components/sidebar/helpers' -import { MenuItem, SidebarProps } from 'components/sidebar' +import { SidebarProps } from 'components/sidebar' import { EnrichedNavItem } from 'components/sidebar/types' import { generateWafCollectionSidebar } from 'views/well-architected-framework/utils/generate-collection-sidebar' import { getNextPrevious } from 'views/tutorial-view/components' @@ -97,13 +97,21 @@ export async function getWafTutorialViewProps( if (isLastTutorial) { const filteredSidebarItems = categorizedWafCollectionSidebarItems.filter( - (item: MenuItem) => !item.divider && !item.heading + (item) => { + const isDivider = 'divider' in item + const isHeading = 'heading' in item + return !isDivider && !isHeading + } ) const currentIndex = filteredSidebarItems.findIndex( - (item: MenuItem) => item.fullPath === `/${currentCollection.slug}` + (item) => + 'fullPath' in item && item.fullPath === `/${currentCollection.slug}` ) - const nextSidebarItem: MenuItem = filteredSidebarItems[currentIndex + 1] - nextCollection = allWafCollections.find((c) => nextSidebarItem?.id === c.id) + const nextSidebarItem = filteredSidebarItems[currentIndex + 1] + nextCollection = allWafCollections.find((c) => { + const hasId = nextSidebarItem && 'id' in nextSidebarItem + return hasId && nextSidebarItem.id === c.id + }) } /** From 5eddb5d9e6c589baf5d15df42a71fac6dde2d0a8 Mon Sep 17 00:00:00 2001 From: Zach Shilton <4624598+zchsh@users.noreply.github.com> Date: Fri, 4 Aug 2023 17:58:10 -0400 Subject: [PATCH 2/4] docs: add comments --- src/lib/docs/add-branded-overview-sidebar-item.ts | 2 ++ .../well-architected-framework/tutorial-view/server.ts | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/lib/docs/add-branded-overview-sidebar-item.ts b/src/lib/docs/add-branded-overview-sidebar-item.ts index eb9a1054c3..933ec4d726 100644 --- a/src/lib/docs/add-branded-overview-sidebar-item.ts +++ b/src/lib/docs/add-branded-overview-sidebar-item.ts @@ -10,9 +10,11 @@ import { MenuItem } from 'components/sidebar/types' * Determine whether a `menuItem` is an "overview" item. */ function isOverviewItem(item: MenuItem) { + // Item must have a `path`, otherwise definitely not an overview item if (!('path' in item)) { return false } + // Path must match certain criteria for "overview" items const isPathMatch = item.path === '' || item.path === '/' || diff --git a/src/views/well-architected-framework/tutorial-view/server.ts b/src/views/well-architected-framework/tutorial-view/server.ts index 6558c1440f..95b6a900e4 100644 --- a/src/views/well-architected-framework/tutorial-view/server.ts +++ b/src/views/well-architected-framework/tutorial-view/server.ts @@ -98,6 +98,16 @@ export async function getWafTutorialViewProps( if (isLastTutorial) { const filteredSidebarItems = categorizedWafCollectionSidebarItems.filter( (item) => { + /** + * Filter out divider and heading items, these are definitely + * not the "next" item. + * + * Note that tutorials sidebars will never have + * divider or heading items, so this is probably unnecessary. + * But it's here for consistency with other sidebar types. + * Types could potentially be refactored to reflect that + * tutorial sidebars only use a subset of possible `MenuItem` types. + */ const isDivider = 'divider' in item const isHeading = 'heading' in item return !isDivider && !isHeading From 78794e39f3b7f5f9bdd03ecb9a0812fb3a150244 Mon Sep 17 00:00:00 2001 From: Zach Shilton <4624598+zchsh@users.noreply.github.com> Date: Fri, 4 Aug 2023 18:00:32 -0400 Subject: [PATCH 3/4] docs: update comments --- .../sidebar/components/sidebar-nav-menu-item/index.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx b/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx index e79aa465c7..e0780bbef6 100644 --- a/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx +++ b/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx @@ -203,6 +203,8 @@ export function SidebarNavMenuButton({ item }: SidebarNavMenuButtonProps) { * or `false` otherwise. * * TODO: update input `item` type to be the "submenu" item type specifically. + * Can't do this yet cause MenuItem still needs more work before we can. + * Task: https://app.asana.com/0/1202097197789424/1202405210286689/f */ function getDefaultOpen(item: MenuItemOptionalProperties): boolean { const defaultOpenProps = [ @@ -227,7 +229,11 @@ function getDefaultOpen(item: MenuItemOptionalProperties): boolean { const SidebarNavSubmenuItem = ({ item, }: { - // TODO: update this `item` type to be the "submenu" item type specifically. + /** + * TODO: update this `item` type to be the "submenu" item type specifically. + * Can't do this yet cause MenuItem still needs more work before we can. + * Task: https://app.asana.com/0/1202097197789424/1202405210286689/f + */ item: MenuItemOptionalProperties }) => { const buttonRef = useRef() From 00d83a72f6f9da2a9981d9a7735f94971b283034 Mon Sep 17 00:00:00 2001 From: Zach Shilton <4624598+zchsh@users.noreply.github.com> Date: Fri, 4 Aug 2023 18:01:05 -0400 Subject: [PATCH 4/4] chore: fix typo --- .../sidebar/components/sidebar-nav-menu-item/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx b/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx index e0780bbef6..89e20aba94 100644 --- a/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx +++ b/src/components/sidebar/components/sidebar-nav-menu-item/index.tsx @@ -292,7 +292,7 @@ const SidebarNavSubmenuItem = ({ {item.routes.map((route: MenuItem, i) => { /** * Note: these items _aren't_ stable since we filter them - * client-side... perhapse `useId` would be appropriate here? + * client-side... perhaps `useId` would be appropriate here? * Or we could do that server-side before passing props to the * client? `heading` and `divider` items do _not_ have a * meaningful identifier; and some other items could potentially