From 809304de1e1cbfca2bccdaba91ebf3fe2bba8473 Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Fri, 28 Mar 2025 15:25:04 -0500 Subject: [PATCH 01/11] Initial clean up of flags. --- .../react/src/UnderlineNav/UnderlineNav.tsx | 15 +- .../UnderlinePanels/UnderlinePanels.tsx | 83 ++---- .../components/UnderlineTabbedInterface.tsx | 254 ++---------------- 3 files changed, 44 insertions(+), 308 deletions(-) diff --git a/packages/react/src/UnderlineNav/UnderlineNav.tsx b/packages/react/src/UnderlineNav/UnderlineNav.tsx index 6808045fd8b..143103ec5d6 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNav.tsx @@ -10,12 +10,7 @@ import {useTheme} from '../ThemeProvider' import type {ChildWidthArray, ResponsiveProps, ChildSize} from './types' import VisuallyHidden from '../_VisuallyHidden' import {moreBtnStyles, getDividerStyle, menuStyles, menuItemStyles, baseMenuStyles, baseMenuMinWidth} from './styles' -import { - StyledUnderlineItemList, - StyledUnderlineWrapper, - LoadingCounter, - GAP, -} from '../internal/components/UnderlineTabbedInterface' +import {UnderlineItemList, UnderlineWrapper, LoadingCounter, GAP} from '../internal/components/UnderlineTabbedInterface' import styled from 'styled-components' import {Button} from '../Button' import {TriangleDownIcon} from '@primer/octicons-react' @@ -311,8 +306,8 @@ export const UnderlineNav = forwardRef( }} > {ariaLabel && {`${ariaLabel} navigation`}} - - + + {listItems} {menuItems.length > 0 && ( @@ -404,8 +399,8 @@ export const UnderlineNav = forwardRef( )} - - + + ) }, diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx index 47ce9c06ea2..377122abb7a 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx @@ -12,25 +12,21 @@ import {TabContainerElement} from '@github/tab-container-element' import type {IconProps} from '@primer/octicons-react' import {createComponent} from '../../utils/create-component' import { - StyledUnderlineItemList, - StyledUnderlineWrapper, + UnderlineItemList, + UnderlineWrapper, UnderlineItem, type UnderlineItemProps, } from '../../internal/components/UnderlineTabbedInterface' import Box, {type BoxProps} from '../../Box' import {useId} from '../../hooks' import {invariant} from '../../utils/invariant' -import {merge, type BetterSystemStyleObject, type SxProp} from '../../sx' +import {type SxProp} from '../../sx' import {defaultSxProp} from '../../utils/defaultSxProp' import {useResizeObserver, type ResizeObserverEntry} from '../../hooks/useResizeObserver' import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' -import {useFeatureFlag} from '../../FeatureFlags' import classes from './UnderlinePanels.module.css' -import {toggleStyledComponent} from '../../internal/utils/toggleStyledComponent' import {clsx} from 'clsx' -const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_ga' - export type UnderlinePanelsProps = { /** * Tabs (UnderlinePanels.Tab) and panels (UnderlinePanels.Panel) to render @@ -82,12 +78,6 @@ export type PanelProps = Omit const TabContainerComponent = createComponent(TabContainerElement, 'tab-container') -const StyledTabContainerComponent = toggleStyledComponent( - CSS_MODULES_FEATURE_FLAG, - 'tab-container', - TabContainerComponent, -) - const UnderlinePanels: FC = ({ 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, @@ -139,8 +129,6 @@ const UnderlinePanels: FC = ({ const tabsHaveIcons = tabs.some(tab => React.isValidElement(tab) && tab.props.icon) - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - // this is a workaround to get the list's width on the first render const [listWidth, setListWidth] = useState(0) useIsomorphicLayoutEffect(() => { @@ -153,19 +141,15 @@ const UnderlinePanels: FC = ({ // when the wrapper resizes, check if the icons should be visible // by comparing the wrapper width to the list width - useResizeObserver( - (resizeObserverEntries: ResizeObserverEntry[]) => { - if (!tabsHaveIcons) { - return - } + useResizeObserver((resizeObserverEntries: ResizeObserverEntry[]) => { + if (!tabsHaveIcons) { + return + } - const wrapperWidth = resizeObserverEntries[0].contentRect.width + const wrapperWidth = resizeObserverEntries[0].contentRect.width - setIconsVisible(wrapperWidth > listWidth) - }, - wrapperRef, - [enabled], - ) + setIconsVisible(wrapperWidth > listWidth) + }, wrapperRef) if (__DEV__) { const selectedTabs = tabs.filter(tab => { @@ -182,53 +166,22 @@ const UnderlinePanels: FC = ({ ) } - if (enabled) { - return ( - - - - {tabs} - - - {tabPanels} - - ) - } - return ( - - + ( - { - width: '100%', - overflowX: 'auto', - overflowY: 'hidden', - '-webkit-overflow-scrolling': 'auto', - '&[data-icons-visible="false"] [data-component="icon"]': { - display: 'none', - }, - }, - sxProp as SxProp, - )} - className={className} + sx={sxProp} + className={clsx(className, classes.StyledUnderlineWrapper)} {...props} > - + {tabs} - - + + {tabPanels} - + ) } diff --git a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx index d4afdaf886d..7263fb1156f 100644 --- a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx +++ b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx @@ -3,238 +3,48 @@ import React, {forwardRef, type FC, type PropsWithChildren} from 'react' import {isElement} from 'react-is' import type {IconProps} from '@primer/octicons-react' -import styled, {keyframes} from 'styled-components' import CounterLabel from '../../CounterLabel' -import sx, {type SxProp} from '../../sx' +import {type SxProp} from '../../sx' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../../utils/polymorphic' import {defaultSxProp} from '../../utils/defaultSxProp' -import {get} from '../../constants' -import {toggleStyledComponent} from '../utils/toggleStyledComponent' +import {toggleSxComponent} from '../utils/toggleSxComponent' import classes from './UnderlineTabbedInterface.module.css' -import {useFeatureFlag} from '../../FeatureFlags' import {clsx} from 'clsx' // The gap between the list items. It is a constant because the gap is used to calculate the possible number of items that can fit in the container. export const GAP = 8 -const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_ga' - -const StyledComponentUnderlineWrapper = toggleStyledComponent( - CSS_MODULES_FEATURE_FLAG, - 'div', - styled.div` - display: flex; - padding-inline: var(--stack-padding-normal, ${get('space.3')}); - justify-content: flex-start; - align-items: center; - /* make space for the underline */ - min-height: var(--control-xlarge-size, 48px); - /* using a box-shadow instead of a border to accomodate 'overflow-y: hidden' on UnderlinePanels */ - box-shadow: inset 0px -1px var(--borderColor-muted, ${get('colors.border.muted')}); - - ${sx}; - `, -) - -type StyledUnderlineWrapperProps = { +type UnderlineWrapperProps = { slot?: string as?: React.ElementType className?: string } & SxProp -export const StyledUnderlineWrapper = forwardRef( - ({children, className, ...rest}: PropsWithChildren, forwardedRef) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - - if (enabled) { - return ( - - {children} - - ) - } +export const UnderlineWrapper = forwardRef( + ({children, className, sx, ...rest}: PropsWithChildren, forwardedRef) => { + const UnderlineWrapperComponent = toggleSxComponent(sx, 'div') as React.ComponentType< + PropsWithChildren + > return ( - + // @ts-ignore ref prop + {children} - + ) }, ) -const StyledComponentUnderlineItemList = toggleStyledComponent( - CSS_MODULES_FEATURE_FLAG, - 'ul', - styled.ul` - display: flex; - list-style: none; - white-space: nowrap; - padding: 0; - margin: 0; - align-items: center; - gap: ${GAP}px; - position: relative; - `, -) - -export const StyledUnderlineItemList = forwardRef(({children, ...rest}: PropsWithChildren, forwardedRef) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - - if (enabled) { - return ( - - {children} - - ) - } - +export const UnderlineItemList = forwardRef(({children, ...rest}: PropsWithChildren, forwardedRef) => { return ( - +
    {children} - +
) }) as PolymorphicForwardRefComponent<'ul'> -export const StyledUnderlineItem = toggleStyledComponent( - CSS_MODULES_FEATURE_FLAG, - 'div', - styled.div` - /* button resets */ - appearance: none; - background-color: transparent; - border: 0; - cursor: pointer; - font: inherit; - - /* underline tab specific styles */ - position: relative; - display: inline-flex; - color: ${get('colors.fg.default')}; - text-align: center; - text-decoration: none; - line-height: var(--text-body-lineHeight-medium, 1.4285); - border-radius: var(--borderRadius-medium, ${get('radii.2')}); - font-size: var(--text-body-size-medium, ${get('fontSizes.1')}); - padding-inline: var(--control-medium-paddingInline-condensed, ${get('space.2')}); - padding-block: var(--control-medium-paddingBlock, 6px); - align-items: center; - - @media (hover: hover) { - &:hover { - background-color: var(--bgColor-neutral-muted, ${get('colors.neutral.subtle')}); - transition: background 0.12s ease-out; - text-decoration: none; - } - } - - &:focus: { - outline: 2px solid transparent; - box-shadow: inset 0 0 0 2px var(--fgColor-accent, ${get('colors.accent.fg')}); - - /* where focus-visible is supported, remove the focus box-shadow */ - &:not(:focus-visible) { - box-shadow: none; - } - } - - &:focus-visible { - outline: 2px solid transparent; - box-shadow: inset 0 0 0 2px var(--fgColor-accent, ${get('colors.accent.fg')}); - } - - /* renders a visibly hidden "copy" of the label in bold, reserving box space for when label becomes bold on selected */ - [data-content]::before { - content: attr(data-content); - display: block; - height: 0; - font-weight: var(--base-text-weight-semibold, ${get('fontWeights.semibold')}); - visibility: hidden; - white-space: nowrap; - } - - [data-component='icon'] { - color: var(--fgColor-muted, ${get('colors.fg.muted')}); - align-items: center; - display: inline-flex; - margin-inline-end: var(--control-medium-gap, ${get('space.2')}); - } - - [data-component='counter'] { - margin-inline-start: var(--control-medium-gap, ${get('space.2')}); - display: flex; - align-items: center; - } - - /* selected state styles */ - &::after { - position: absolute; - right: 50%; - /* TODO: see if we can simplify this positioning */ - /* 48px total height / 2 (24px) + 1px */ - bottom: calc(50% - calc(var(--control-xlarge-size, 48px) / 2 + 1px)); - width: 100%; - height: 2px; - content: ''; - background-color: transparent; - border-radius: 0; - transform: translate(50%, -50%); - } - - &[aria-current]:not([aria-current='false']), - &[aria-selected='true'] { - [data-component='text'] { - font-weight: var(--base-text-weight-semibold, ${get('fontWeights.semibold')}); - } - - &::after { - background-color: var(--underlineNav-borderColor-active, var(--color-primer-border-active, #fd8c73)); - } - } - - @media (forced-colors: active) { - &[aria-current]:not([aria-current='false']), - &[aria-selected='true'] { - ::after { - // Support for Window Force Color Mode https://learn.microsoft.com/en-us/fluent-ui/web-components/design-system/high-contrast - background-color: LinkText; - } - } - } - ${sx}; - `, -) - -const loadingKeyframes = keyframes` - from { opacity: 1; } - to { opacity: 0.2; } -` - -export const StyledComponentLoadingCounter = toggleStyledComponent( - CSS_MODULES_FEATURE_FLAG, - 'span', - styled.span` - animation: ${loadingKeyframes} 1.2s ease-in-out infinite alternate; - background-color: var(--bgColor-neutral-muted, ${get('colors.neutral.subtle')}); - border-color: var(--borderColor-default, ${get('colors.border.default')}); - width: 1.5rem; - height: 1rem; /*16px*/ - display: inline-block; - border-radius: 20px; - `, -) - export const LoadingCounter = () => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - - if (enabled) { - return - } - - return + return } // We can uncomment these when/if we add overflow behavior @@ -292,34 +102,12 @@ export const UnderlineItem = forwardRef( }: PropsWithChildren, forwardedRef, ) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - - if (enabled) { - return ( - - {iconsVisible && Icon && {isElement(Icon) ? Icon : }} - {children && ( - - {children} - - )} - {counter !== undefined ? ( - loadingCounters ? ( - - - - ) : ( - - {counter} - - ) - ) : null} - - ) - } - + const UnderlineComponent = toggleSxComponent(sxProp, as) as React.ComponentType< + PropsWithChildren + > return ( - + // @ts-ignore ref prop + {iconsVisible && Icon && {isElement(Icon) ? Icon : }} {children && ( @@ -337,7 +125,7 @@ export const UnderlineItem = forwardRef( ) ) : null} - + ) }, ) as PolymorphicForwardRefComponent<'a', UnderlineItemProps> From 1e8ed4bdb6d767a814089dfa00eaf5c6dad561f3 Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Tue, 1 Apr 2025 16:19:24 -0500 Subject: [PATCH 02/11] Move toggleSxComponent out of component render cycle. --- .../UnderlinePanels/UnderlinePanels.tsx | 18 ++++++----- .../components/UnderlineTabbedInterface.tsx | 31 ++++++++++++------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx index 377122abb7a..94ec2df6142 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx @@ -141,15 +141,19 @@ const UnderlinePanels: FC = ({ // when the wrapper resizes, check if the icons should be visible // by comparing the wrapper width to the list width - useResizeObserver((resizeObserverEntries: ResizeObserverEntry[]) => { - if (!tabsHaveIcons) { - return - } + useResizeObserver( + (resizeObserverEntries: ResizeObserverEntry[]) => { + if (!tabsHaveIcons) { + return + } - const wrapperWidth = resizeObserverEntries[0].contentRect.width + const wrapperWidth = resizeObserverEntries[0].contentRect.width - setIconsVisible(wrapperWidth > listWidth) - }, wrapperRef) + setIconsVisible(wrapperWidth > listWidth) + }, + wrapperRef, + [], + ) if (__DEV__) { const selectedTabs = tabs.filter(tab => { diff --git a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx index 7263fb1156f..689a661f729 100644 --- a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx +++ b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx @@ -19,16 +19,25 @@ type UnderlineWrapperProps = { slot?: string as?: React.ElementType className?: string + ref?: React.Ref } & SxProp +const UnderlineWrapperComponent = toggleSxComponent({}, 'div') as React.ComponentType< + PropsWithChildren +> + export const UnderlineWrapper = forwardRef( - ({children, className, sx, ...rest}: PropsWithChildren, forwardedRef) => { - const UnderlineWrapperComponent = toggleSxComponent(sx, 'div') as React.ComponentType< - PropsWithChildren - > + ( + {children, className, sx: sxProp = defaultSxProp, ...rest}: PropsWithChildren, + forwardedRef, + ) => { return ( - // @ts-ignore ref prop - + {children} ) @@ -81,13 +90,17 @@ export const LoadingCounter = () => { export type UnderlineItemProps = { as?: React.ElementType | 'a' | 'button' + className?: string iconsVisible?: boolean loadingCounters?: boolean counter?: number | string icon?: FC | React.ReactElement id?: string + ref?: React.Ref } & SxProp +const UnderlineComponent = toggleSxComponent({}, 'a') as React.ComponentType> + export const UnderlineItem = forwardRef( ( { @@ -102,12 +115,8 @@ export const UnderlineItem = forwardRef( }: PropsWithChildren, forwardedRef, ) => { - const UnderlineComponent = toggleSxComponent(sxProp, as) as React.ComponentType< - PropsWithChildren - > return ( - // @ts-ignore ref prop - + {iconsVisible && Icon && {isElement(Icon) ? Icon : }} {children && ( From b982ce6e999e63e69523c65b1e1b471b8629ea0a Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Tue, 1 Apr 2025 16:21:27 -0500 Subject: [PATCH 03/11] changeset. --- .changeset/strong-pens-cross.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/strong-pens-cross.md diff --git a/.changeset/strong-pens-cross.md b/.changeset/strong-pens-cross.md new file mode 100644 index 00000000000..af171607915 --- /dev/null +++ b/.changeset/strong-pens-cross.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Removes css module flag for UnderlinePanels and associated components. From 5db11c87a16bae7d10c1699347939a83d888e726 Mon Sep 17 00:00:00 2001 From: Jamie Shark <5520141+jamieshark@users.noreply.github.com> Date: Tue, 1 Apr 2025 16:23:52 -0500 Subject: [PATCH 04/11] Simplify arguments for toggleSxComponent. --- .../internal/components/UnderlineTabbedInterface.tsx | 4 ++-- .../utils/__tests__/toggleSxComponent.test.tsx | 12 ++++++------ .../react/src/internal/utils/toggleSxComponent.tsx | 4 +--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx index 689a661f729..7ce70043009 100644 --- a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx +++ b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx @@ -22,7 +22,7 @@ type UnderlineWrapperProps = { ref?: React.Ref } & SxProp -const UnderlineWrapperComponent = toggleSxComponent({}, 'div') as React.ComponentType< +const UnderlineWrapperComponent = toggleSxComponent('div') as React.ComponentType< PropsWithChildren > @@ -99,7 +99,7 @@ export type UnderlineItemProps = { ref?: React.Ref } & SxProp -const UnderlineComponent = toggleSxComponent({}, 'a') as React.ComponentType> +const UnderlineComponent = toggleSxComponent('a') as React.ComponentType> export const UnderlineItem = forwardRef( ( diff --git a/packages/react/src/internal/utils/__tests__/toggleSxComponent.test.tsx b/packages/react/src/internal/utils/__tests__/toggleSxComponent.test.tsx index f077b39159d..430070d6f2c 100644 --- a/packages/react/src/internal/utils/__tests__/toggleSxComponent.test.tsx +++ b/packages/react/src/internal/utils/__tests__/toggleSxComponent.test.tsx @@ -6,21 +6,21 @@ const customSx = {color: 'red', p: 2} describe('toggleSxComponent', () => { test('renders the plain component when no sx', () => { - const TestComponent = toggleSxComponent({}, 'span') + const TestComponent = toggleSxComponent('span') const {container} = render() expect(container.firstChild).toBeInstanceOf(HTMLSpanElement) }) test('renders Box with `as` if `sx` is provided', () => { - const TestComponent = toggleSxComponent(customSx, 'div') - const {container} = render() + const TestComponent = toggleSxComponent('div') + const {container} = render() expect(container.firstChild).toBeInstanceOf(HTMLButtonElement) expect(container.firstChild).toHaveStyle('color: red') }) test('swaps out component if `sx` is not the default', () => { - const Label = toggleSxComponent(customSx, 'label') as React.ComponentType<{htmlFor: string}> + const Label = toggleSxComponent('label') as React.ComponentType<{htmlFor: string}> const {container} = render(