From 83ec51ebbdee0fde5d597f409350c553bf44f2e8 Mon Sep 17 00:00:00 2001 From: Katie Langerman <18661030+langermank@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:04:21 -0800 Subject: [PATCH] revert navlist + break out files --- .../src/ActionList/ActionList.module.css | 73 +---------- .../react/src/ActionList/Group.module.css | 37 ++++++ packages/react/src/ActionList/Group.tsx | 20 ++-- .../react/src/ActionList/Heading.module.css | 13 ++ packages/react/src/ActionList/Heading.tsx | 2 +- packages/react/src/ActionList/Item.tsx | 113 ++++++++++++++++-- packages/react/src/ActionList/LinkItem.tsx | 58 ++++++++- packages/react/src/NavList/NavList.tsx | 98 +++------------ 8 files changed, 239 insertions(+), 175 deletions(-) create mode 100644 packages/react/src/ActionList/Group.module.css create mode 100644 packages/react/src/ActionList/Heading.module.css diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index ecf688a48d7..d5521fd2784 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -2,63 +2,6 @@ /* stylelint-disable selector-max-specificity */ /* stylelint-disable max-nesting-depth */ -/* check this as full width variant */ -.ActionListHeader { - margin-bottom: var(--base-size-8); - - &:where([data-list-variant='full']) { - margin-inline-start: var(--base-size-8); - } - - &:where([data-list-variant='inset']) { - margin-inline-start: calc(var(--control-medium-paddingInline-condensed) + var(--base-size-8)); - } -} - -.Group:not(:first-child) { - margin-block-start: var(--base-size-8); -} - -.GroupHeadingWrap { - display: flex; - font-size: var(--text-body-size-small); - font-weight: var(--base-text-weight-semibold); - line-height: var(--text-body-lineHeight-small); - color: var(--fgColor-muted); - flex-direction: column; - /* stylelint-disable-next-line primer/spacing */ - padding-inline: var(--control-medium-paddingInline-condensed); - padding-block: var(--base-size-8); - - &:where([data-variant='filled']) { - /* stylelint-disable-next-line primer/spacing */ - margin-block-start: calc(var(--base-size-8) - var(--borderWidth-thin)); - margin-block-end: var(--base-size-8); - margin-inline: calc(-1 * var(--base-size-8)); - background: var(--bgColor-muted); - border-top: solid var(--borderWidth-thin) var(--borderColor-muted); - border-bottom: solid var(--borderWidth-thin) var(--borderColor-muted); - - /* if no children, treat as divider */ - &:empty { - height: var(--base-size-8); - box-sizing: border-box; - } - - &:first-child { - margin-block-start: 0; - } - } -} - -.GroupHeading { - margin: 0; - font-size: var(--text-body-size-small); - font-weight: var(--base-text-weight-semibold); - color: var(--fgColor-muted); - align-self: flex-start; -} - .ActionList { padding: 0; margin: 0; @@ -571,6 +514,7 @@ /* button or a tag */ .ActionListContent { + --subitem-depth: 0; --subitem-spacer: calc(var(--subitem-depth) * 8px); position: relative; @@ -742,21 +686,6 @@ border-bottom-left-radius: 0; } -/* show trailing action button on hover */ - -.ActionListItem--trailingActionHover { - & .TrailingActionButton { - visibility: hidden; - } - - &:hover, - &:focus-within { - & .TrailingActionButton { - visibility: visible; - } - } -} - .Divider { display: block; height: var(--borderWidth-thin); diff --git a/packages/react/src/ActionList/Group.module.css b/packages/react/src/ActionList/Group.module.css new file mode 100644 index 00000000000..3fccf06b9c6 --- /dev/null +++ b/packages/react/src/ActionList/Group.module.css @@ -0,0 +1,37 @@ +.Group:not(:first-child) { + margin-block-start: var(--base-size-8); +} + +.GroupHeadingWrap { + display: flex; + font-size: var(--text-body-size-small); + font-weight: var(--base-text-weight-semibold); + line-height: var(--text-body-lineHeight-small); + color: var(--fgColor-muted); + flex-direction: column; + /* stylelint-disable-next-line primer/spacing */ + padding-inline: var(--control-medium-paddingInline-condensed); + padding-block: var(--base-size-8); + + &:where([data-variant='filled']) { + /* stylelint-disable-next-line primer/spacing */ + margin-block-start: calc(var(--base-size-8) - var(--borderWidth-thin)); + margin-block-end: var(--base-size-8); + margin-inline: calc(-1 * var(--base-size-8)); + background: var(--bgColor-muted); + border-top: solid var(--borderWidth-thin) var(--borderColor-muted); + border-bottom: solid var(--borderWidth-thin) var(--borderColor-muted); + + &:first-child { + margin-block-start: 0; + } + } +} + +.GroupHeading { + margin: 0; + font-size: var(--text-body-size-small); + font-weight: var(--base-text-weight-semibold); + color: var(--fgColor-muted); + align-self: flex-start; +} diff --git a/packages/react/src/ActionList/Group.tsx b/packages/react/src/ActionList/Group.tsx index 4466a207552..56a20436929 100644 --- a/packages/react/src/ActionList/Group.tsx +++ b/packages/react/src/ActionList/Group.tsx @@ -11,6 +11,7 @@ import {invariant} from '../utils/invariant' import {clsx} from 'clsx' import {useFeatureFlag} from '../FeatureFlags' import classes from './ActionList.module.css' +import groupClasses from './Group.module.css' type HeadingProps = { as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' @@ -92,7 +93,7 @@ export const Group: React.FC> = ({ if (enabled) { if (sx !== defaultSxProp) { return ( - + {title && !slots.groupHeading ? ( // Escape hatch: supports old API in a non breaking way @@ -115,7 +116,7 @@ export const Group: React.FC> = ({ ) } return ( -
  • +
  • {title && !slots.groupHeading ? ( // Escape hatch: supports old API in a non breaking way @@ -224,22 +225,22 @@ export const GroupHeading: React.FC
  • ) diff --git a/packages/react/src/ActionList/LinkItem.tsx b/packages/react/src/ActionList/LinkItem.tsx index 414c632a5a3..2123734bdec 100644 --- a/packages/react/src/ActionList/LinkItem.tsx +++ b/packages/react/src/ActionList/LinkItem.tsx @@ -44,11 +44,63 @@ export const LinkItem = React.forwardRef( const enabled = useFeatureFlag('primer_react_css_modules_team') + if (enabled) { + if (sx !== defaultSxProp) { + return ( + { + const clickHandler = (event: React.MouseEvent) => { + onClick && onClick(event) + props.onClick && props.onClick(event as React.MouseEvent) + } + return inactiveText ? ( + {children} + ) : ( + + {children} + + ) + }} + > + {props.children} + + ) + } + + // return ( + // { + // const clickHandler = (event: React.MouseEvent) => { + // onClick && onClick(event) + // props.onClick && props.onClick(event as React.MouseEvent) + // } + // return inactiveText ? ( + // {children} + // ) : ( + // + // {children} + // + // ) + // }} + // > + // {props.children} + // + // ) + } + return ( { @@ -57,13 +109,13 @@ export const LinkItem = React.forwardRef( props.onClick && props.onClick(event as React.MouseEvent) } return inactiveText ? ( - + {children} ) : ( { return { @@ -39,9 +35,7 @@ export type NavListProps = { } & SxProp & React.ComponentProps<'nav'> -// const NavBox = styled.nav(sx) - -const NavBox = toggleStyledComponent('primer_react_css_modules_team', 'nav', styled.nav(sx)) +const NavBox = styled.nav(sx) const Root = React.forwardRef(({children, ...props}, ref) => { return ( @@ -72,7 +66,6 @@ export type NavListItemProps = { const Item = React.forwardRef( ({'aria-current': ariaCurrent, children, defaultOpen, sx: sxProp = defaultSxProp, ...props}, ref) => { - const enabled = useFeatureFlag('primer_react_css_modules_team') const {depth} = React.useContext(SubNavContext) // Get SubNav from children @@ -90,16 +83,8 @@ const Item = React.forwardRef( // Render ItemWithSubNav if SubNav is present if (subNav && isValidElement(subNav)) { return ( - + {childrenWithoutSubNavOrTrailingAction} - depth:{depth} ) } @@ -109,14 +94,10 @@ const Item = React.forwardRef( ref={ref} aria-current={ariaCurrent} active={Boolean(ariaCurrent) && ariaCurrent !== 'false'} - sx={enabled ? undefined : merge(getSubnavStyles(depth), sxProp)} - className={classes.SubItem} - data-depth={depth} - style={{'--subitem-depth': depth} as React.CSSProperties} + sx={merge(getSubnavStyles(depth), sxProp)} {...props} > {children} - depth:{depth} ) }, @@ -142,14 +123,7 @@ const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: s // TODO: ref prop // TODO: Animate open/close transition -function ItemWithSubNav({ - children, - subNav, - depth, - defaultOpen, - style, - sx: sxProp = defaultSxProp, -}: ItemWithSubNavProps) { +function ItemWithSubNav({children, subNav, depth, defaultOpen, sx: sxProp = defaultSxProp}: ItemWithSubNavProps) { const buttonId = useId() const subNavId = useId() const [isOpen, setIsOpen] = React.useState((defaultOpen || null) ?? false) @@ -169,55 +143,26 @@ function ItemWithSubNav({ } }, [subNav, buttonId]) - const enabled = useFeatureFlag('primer_react_css_modules_team') - if (enabled) { - if (sxProp !== defaultSxProp) { - return

    sxprop

    - } - return ( - - {/*
  • */} - setIsOpen(open => !open)} - // wrapper="button" - style={style} - > - {children} - - {/* What happens if the user provides a TrailingVisual? */} - - - - {React.cloneElement(subNav as React.ReactElement, {ref: subNavRef})} - - - {/*
  • */} -
    - ) - } return ( setIsOpen(open => !open)} - sx={merge( - { - ...getSubnavStyles(depth), - fontWeight: containsCurrentItem ? 'bold' : null, // Parent item is bold if any of it's sub-items are current - }, - sxProp, - )} + data-hi + sx={{color: 'red'}} + // sx={merge( + // { + // ...getSubnavStyles(depth), + // fontWeight: containsCurrentItem ? 'bold' : null, // Parent item is bold if any of it's sub-items are current + // }, + // sxProp, + // )} > {children} {/* What happens if the user provides a TrailingVisual? */} @@ -251,7 +196,7 @@ const SubNavContext = React.createContext<{depth: number}>({depth: 0}) const SubNav = ({children, sx: sxProp = defaultSxProp}: NavListSubNavProps) => { const {buttonId, subNavId, isOpen} = React.useContext(ItemWithSubNavContext) const {depth} = React.useContext(SubNavContext) - const enabled = useFeatureFlag('primer_react_css_modules_team') + if (!buttonId || !subNavId) { // eslint-disable-next-line no-console console.error('NavList.SubNav must be a child of a NavList.Item') @@ -264,19 +209,6 @@ const SubNav = ({children, sx: sxProp = defaultSxProp}: NavListSubNavProps) => { return null } - if (enabled) { - if (sxProp !== defaultSxProp) { - return

    sxprop

    - } - return ( - -
      - {children} -
    -
    - ) - } - return (