diff --git a/.changeset/brave-penguins-clap.md b/.changeset/brave-penguins-clap.md new file mode 100644 index 00000000000..5b111e9c7e8 --- /dev/null +++ b/.changeset/brave-penguins-clap.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +Corrects horizontal padding of ` - - Item 1 - Item 2 - - - ) - } - - const {getByRole} = HTMLRender() - const triggerBtn = getByRole('button', {name: 'Prompt'}) - const focusTarget = getByRole('listitem', {name: 'Item 1'}) - - fireEvent.click(triggerBtn) - - expect(document.activeElement).toBe(focusTarget) - }) - - it('should render ActionList.Item as li when feature flag is enabled and has proper aria role', async () => { - const {container} = HTMLRender( - - - Item 1 - Item 2 - - , - ) - - const listitem = container.querySelector('li') - const button = container.querySelector('button') - - expect(listitem).toHaveTextContent('Item 1') - expect(listitem).toHaveAttribute('tabindex', '0') - expect(button).toBeNull() - - const listItems = container.querySelectorAll('li') - expect(listItems.length).toBe(2) - }) - - it('should render the trailing action as a button (default)', async () => { - const {container} = HTMLRender( - - - Item 1 - - - , - ) - - const action = container.querySelector('button[aria-labelledby]') - expect(action).toHaveAccessibleName('Action') - }) - - it('should render the trailing action as a link', async () => { - const {container} = HTMLRender( - - - Item 1 - - - , - ) - - const action = container.querySelector('a[href="#"][aria-labelledby]') - expect(action).toHaveAccessibleName('Action') - }) - - it('should do action when trailing action is clicked', async () => { - const onClick = jest.fn() - const component = HTMLRender( - - - Item 1 - - - , - ) - - const trailingAction = await waitFor(() => component.getByRole('button', {name: 'Action'})) - fireEvent.click(trailingAction) - expect(onClick).toHaveBeenCalled() - }) - - it('should focus the trailing action', async () => { - HTMLRender( - - - Item 1 - - - , - ) - - await userEvent.tab() - expect(document.activeElement).toHaveTextContent('Item 1') - await userEvent.tab() - expect(document.activeElement).toHaveAccessibleName('Action') - }) - - it('should only trigger a key event once when feature flag is enabled', async () => { - const mockOnSelect = jest.fn() - const user = userEvent.setup() - const {getByRole} = HTMLRender( - - - Item 1 - - , - ) - const item = getByRole('button') - - item.focus() - - expect(document.activeElement).toBe(item) - await user.keyboard('{Enter}') - - expect(mockOnSelect).toHaveBeenCalledTimes(1) - }) - - it('should not render buttons when feature flag is enabled and is specified role', async () => { - const {getByRole} = HTMLRender( - - - Item 1 - Item 2 - Item 3 - Item 4 - Item 5 - - , - ) - - const option = getByRole('option') - expect(option.tagName).toBe('LI') - expect(option.textContent).toBe('Item 1') - - const menuItem = getByRole('menuitem') - expect(menuItem.tagName).toBe('LI') - - const menuItemCheckbox = getByRole('menuitemcheckbox') - expect(menuItemCheckbox.tagName).toBe('LI') - - const menuItemRadio = getByRole('menuitemradio') - expect(menuItemRadio.tagName).toBe('LI') - - const button = getByRole('button') - expect(button.parentElement?.tagName).toBe('LI') - expect(button.textContent).toBe('Item 5') - }) - it('should be navigatable with arrow keys for certain roles', async () => { HTMLRender( @@ -603,54 +94,6 @@ describe('ActionList', () => { expect(document.activeElement).toHaveTextContent('Option 4') }) - describe('ActionList.Description', () => { - it('should render the description as inline without truncation by default', () => { - const {getByText} = HTMLRender( - - - Item 1Item 1 description - - , - ) - - const description = getByText('Item 1 description') - expect(description.tagName).toBe('SPAN') - expect(description).toHaveStyleRule('flex-basis', 'auto') - expect(description).not.toHaveStyleRule('overflow', 'ellipsis') - expect(description).not.toHaveStyleRule('white-space', 'nowrap') - }) - it('should render the description as `Truncate` when truncate is true', () => { - const {getByText} = HTMLRender( - - - Item 1Item 1 description - - , - ) - - const description = getByText('Item 1 description') - expect(description.tagName).toBe('DIV') - expect(description).toHaveAttribute('title', 'Item 1 description') - expect(description).toHaveStyleRule('flex-basis', '0') - expect(description).toHaveStyleRule('text-overflow', 'ellipsis') - expect(description).toHaveStyleRule('overflow', 'hidden') - expect(description).toHaveStyleRule('white-space', 'nowrap') - }) - it('should render the description in a new line when variant is block', () => { - const {getByText} = HTMLRender( - - - Item 1Item 1 description - - , - ) - - const description = getByText('Item 1 description') - expect(description.tagName).toBe('SPAN') - expect(description.parentElement).toHaveAttribute('data-component', 'ActionList.Item--DividerContainer') - }) - }) - it('should support a custom `className` on the outermost element', () => { const Element = () => { return ( diff --git a/packages/react/src/ActionList/Description.test.tsx b/packages/react/src/ActionList/Description.test.tsx new file mode 100644 index 00000000000..858ede5e70e --- /dev/null +++ b/packages/react/src/ActionList/Description.test.tsx @@ -0,0 +1,82 @@ +import {render as HTMLRender} from '@testing-library/react' +import React from 'react' +import {ActionList} from '.' +import {FeatureFlags} from '../FeatureFlags' + +describe('ActionList.Description', () => { + it('should render the description as inline without truncation by default', () => { + const {getByText} = HTMLRender( + + + Item 1Item 1 description + + , + ) + + const description = getByText('Item 1 description') + expect(description.tagName).toBe('SPAN') + expect(description).toHaveStyleRule('flex-basis', 'auto') + expect(description).not.toHaveStyleRule('overflow', 'ellipsis') + expect(description).not.toHaveStyleRule('white-space', 'nowrap') + }) + it('should render the description as `Truncate` when truncate is true', () => { + const {getByText} = HTMLRender( + + + Item 1Item 1 description + + , + ) + + const description = getByText('Item 1 description') + expect(description.tagName).toBe('DIV') + expect(description).toHaveAttribute('title', 'Item 1 description') + expect(description).toHaveStyleRule('flex-basis', '0') + expect(description).toHaveStyleRule('text-overflow', 'ellipsis') + expect(description).toHaveStyleRule('overflow', 'hidden') + expect(description).toHaveStyleRule('white-space', 'nowrap') + }) + it('should render the description in a new line when variant is block', () => { + const {getByText} = HTMLRender( + + + Item 1Item 1 description + + , + ) + + const description = getByText('Item 1 description') + expect(description.tagName).toBe('SPAN') + expect(description.parentElement).toHaveAttribute('data-component', 'ActionList.Item--DividerContainer') + }) + it('should support a custom `className`', () => { + const Element = () => { + return ( + + + Item 1Item 1 description + + + ) + } + const FeatureFlagElement = () => { + return ( + + + + ) + } + expect( + HTMLRender().container.querySelector('span[data-component="ActionList.Description"]'), + ).toHaveClass('test-class-name') + expect( + HTMLRender().container.querySelector('span[data-component="ActionList.Description"]'), + ).toHaveClass('test-class-name') + }) +}) diff --git a/packages/react/src/ActionList/Description.tsx b/packages/react/src/ActionList/Description.tsx index 68bc4f661f9..15b2e19c3f4 100644 --- a/packages/react/src/ActionList/Description.tsx +++ b/packages/react/src/ActionList/Description.tsx @@ -4,6 +4,11 @@ import Truncate from '../Truncate' import type {SxProp} from '../sx' import {merge} from '../sx' import {ItemContext} from './shared' +import {useFeatureFlag} from '../FeatureFlags' +import classes from './ActionList.module.css' +import {clsx} from 'clsx' +import {defaultSxProp} from '../utils/defaultSxProp' +import {actionListCssModulesFlag} from './featureflag' export type ActionListDescriptionProps = { /** @@ -22,7 +27,7 @@ export type ActionListDescriptionProps = { export const Description: React.FC> = ({ variant = 'inline', - sx = {}, + sx = defaultSxProp, className, truncate, ...props @@ -42,6 +47,65 @@ export const Description: React.FC + {props.children} + + ) + } else { + return ( + + {props.children} + + ) + } + } + if (variant === 'block' || !truncate) { + return ( + + {props.children} + + ) + } else { + return ( + + {props.children} + + ) + } + } + return variant === 'block' || !truncate ? ( > = ({sx = defaultSxProp, className}) => { - const enabled = useFeatureFlag('primer_react_css_modules_team') + const enabled = useFeatureFlag(actionListCssModulesFlag) if (enabled) { if (sx !== defaultSxProp) { return ( @@ -43,8 +44,6 @@ export const Divider: React.FC> marginTop: (theme: Theme) => `calc(${get('space.2')(theme)} - 1px)`, marginBottom: 2, listStyle: 'none', // hide the ::marker inserted by browser's stylesheet - marginRight: 'calc(-1 * var(--base-size-8))', - marginLeft: 'calc(-1 * var(--base-size-8))', }, sx as SxProp, )} diff --git a/packages/react/src/ActionList/Group.module.css b/packages/react/src/ActionList/Group.module.css new file mode 100644 index 00000000000..1a517c52463 --- /dev/null +++ b/packages/react/src/ActionList/Group.module.css @@ -0,0 +1,52 @@ +.Group:not(:first-child) { + margin-block-start: var(--base-size-8); + + /* If somebody tries to pass the `title` prop AND a `NavList.GroupHeading` as a child, hide the `ActionList.GroupHeading */ + /* stylelint-disable-next-line selector-max-specificity */ + &:has(.GroupHeadingWrap + ul > .GroupHeadingWrap) { + /* stylelint-disable-next-line selector-max-specificity */ + & > .GroupHeadingWrap { + display: none; + } + } +} + +.GroupHeadingWrap { + display: flex; + font-size: var(--text-body-size-small); + font-weight: var(--base-text-weight-semibold); + + /* line-height: var(--text-body-lineHeight-small); use when FF rolls out */ + /* stylelint-disable-next-line primer/typography */ + line-height: 18px; + color: var(--fgColor-muted); + flex-direction: column; + padding-inline: var(--base-size-16); + padding-block: var(--base-size-6); + + &: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); + background: var(--bgColor-muted); + border-top: solid var(--borderWidth-thin) var(--borderColor-muted); + border-bottom: solid var(--borderWidth-thin) var(--borderColor-muted); + padding-inline: var(--base-size-16); + + &:first-child { + margin-block-start: 0; + } + } + + /* & + ul:has(.GroupHeadingWrap) { + outline: solid 1px red; + } */ +} + +.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.test.tsx b/packages/react/src/ActionList/Group.test.tsx new file mode 100644 index 00000000000..742cd50c105 --- /dev/null +++ b/packages/react/src/ActionList/Group.test.tsx @@ -0,0 +1,150 @@ +import {render as HTMLRender} from '@testing-library/react' +import React from 'react' +import theme from '../theme' +import {ActionList} from '.' +import {BaseStyles, ThemeProvider, ActionMenu} from '..' +import {FeatureFlags} from '../FeatureFlags' + +describe('ActionList.Group', () => { + it('should throw an error when ActionList.GroupHeading has an `as` prop when it is used within ActionMenu context', async () => { + const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn()) + expect(() => + HTMLRender( + + + + Trigger + + + + Group Heading + + + + + + , + ), + ).toThrow( + "Looks like you are trying to set a heading level to a menu role. Group headings for menu type action lists are for representational purposes, and rendered as divs. Therefore they don't need a heading level.", + ) + expect(spy).toHaveBeenCalled() + spy.mockRestore() + }) + + it('should render the ActionList.GroupHeading component as a heading with the given heading level', async () => { + const container = HTMLRender( + + Heading + + Group Heading + + , + ) + const heading = container.getByRole('heading', {level: 2}) + expect(heading).toBeInTheDocument() + expect(heading).toHaveTextContent('Group Heading') + }) + it('should throw an error if ActionList.GroupHeading is used without an `as` prop when no role is specified (for list role)', async () => { + const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn()) + expect(() => + HTMLRender( + + Heading + + Group Heading + Item + + , + ), + ).toThrow( + "You are setting a heading for a list, that requires a heading level. Please use 'as' prop to set a proper heading level.", + ) + expect(spy).toHaveBeenCalled() + spy.mockRestore() + }) + it('should render the ActionList.GroupHeading component as a span (not a heading tag) when role is specified as listbox', async () => { + const container = HTMLRender( + + Heading + + Group Heading + + , + ) + const label = container.getByText('Group Heading') + expect(label).toBeInTheDocument() + expect(label.tagName).toEqual('SPAN') + }) + it('should render the ActionList.GroupHeading component as a span with role="presentation" and aria-hidden="true" when role is specified as listbox', async () => { + const container = HTMLRender( + + Heading + + Group Heading + + , + ) + const label = container.getByText('Group Heading') + const wrapper = label.parentElement + expect(wrapper).toHaveAttribute('role', 'presentation') + expect(wrapper).toHaveAttribute('aria-hidden', 'true') + }) + it('should label the list with the group heading id', async () => { + const {container, getByText} = HTMLRender( + + Heading + + Group Heading + Item + + , + ) + const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`) + const heading = getByText('Group Heading') + expect(list).toHaveAttribute('aria-labelledby', heading.id) + }) + it('should NOT label the list with the group heading id when role is specified', async () => { + const {container, getByText} = HTMLRender( + + Heading + + Group Heading + Item + + , + ) + const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`) + const heading = getByText('Group Heading') + expect(list).not.toHaveAttribute('aria-labelledby', heading.id) + }) + + it('should support a custom `className` on the outermost element', () => { + const Element = () => { + return ( + + + + Test + + + + ) + } + const FeatureFlagElement = () => { + return ( + + + + ) + } + expect(HTMLRender().container.querySelector('h2')).toHaveClass('test-class-name') + expect(HTMLRender().container.querySelector('h2')).toHaveClass('test-class-name') + }) +}) diff --git a/packages/react/src/ActionList/Group.tsx b/packages/react/src/ActionList/Group.tsx index 957f37a4863..44be2fa68d4 100644 --- a/packages/react/src/ActionList/Group.tsx +++ b/packages/react/src/ActionList/Group.tsx @@ -4,12 +4,48 @@ import Box from '../Box' import type {SxProp} from '../sx' import {ListContext, type ActionListProps} from './shared' import type {AriaRole} from '../utils/types' -import {default as Heading} from '../Heading' import type {ActionListHeadingProps} from './Heading' import {useSlots} from '../hooks/useSlots' import {defaultSxProp} from '../utils/defaultSxProp' 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' +import {actionListCssModulesFlag} from './featureflag' + +type HeadingProps = { + as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' + className?: string + children: React.ReactNode + id?: string +} & SxProp + +const Heading: React.FC> = ({ + as: Component = 'h3', + className, + children, + sx = defaultSxProp, + id, + ...rest +}) => { + return ( + // Box is temporary to support lingering sx usage + + {children} + + ) +} + +type HeadingWrapProps = { + as?: 'div' | 'li' + className?: string + children: React.ReactNode +} + +const HeadingWrap: React.FC = ({as = 'div', children, className, ...rest}) => { + return React.createElement(as, {...rest, className}, children) +} export type ActionListGroupProps = { /** @@ -18,7 +54,7 @@ export type ActionListGroupProps = { * - `"filled"` - Superimposed on a background, offset from nearby content * - `"subtle"` - Relatively less offset from nearby content */ - variant?: 'subtle' | 'filled' + variant?: 'filled' | 'subtle' /** * @deprecated (Use `ActionList.GroupHeading` instead. i.e. → Group title) */ @@ -50,9 +86,10 @@ export const Group: React.FC> = ({ auxiliaryText, selectionVariant, role, - sx = {}, + sx = defaultSxProp, ...props }) => { + const enabled = useFeatureFlag(actionListCssModulesFlag) const id = useId() const {role: listRole} = React.useContext(ListContext) @@ -72,6 +109,54 @@ export const Group: React.FC> = ({ groupHeadingId = id } + if (enabled) { + if (sx !== defaultSxProp) { + return ( + + + {title && !slots.groupHeading ? ( + // Escape hatch: supports old API in a non breaking way + + ) : null} + {/* Supports new API ActionList.GroupHeading */} + {!title && slots.groupHeading ? React.cloneElement(slots.groupHeading) : null} + + + + ) + } + return ( +
  • + + {title && !slots.groupHeading ? ( + // Escape hatch: supports old API in a non breaking way + + ) : null} + {/* Supports new API ActionList.GroupHeading */} + {!title && slots.groupHeading ? React.cloneElement(slots.groupHeading) : null} + + +
    • + ) + } return ( > = ({ as, - variant, + variant = 'subtle', // We are not recommending this prop to be used, it should only be used internally for incremental rollout. _internalBackwardCompatibleTitle, auxiliaryText, @@ -136,7 +222,7 @@ export const GroupHeading: React.FC { - const {variant: listVariant, role: listRole} = React.useContext(ListContext) + const {role: listRole} = React.useContext(ListContext) const {groupHeadingId} = React.useContext(GroupContext) // for list role, the headings are proper heading tags, for menu and listbox, they are just representational and divs const missingAsForList = (listRole === undefined || listRole === 'list') && children !== undefined && as === undefined @@ -154,59 +240,54 @@ export const GroupHeading: React.FC {/* for listbox (SelectPanel) and menu (ActionMenu) roles, group titles are presentational. */} {listRole && listRole !== 'list' ? ( - + ) : ( // for explicit (role="list" is passed as prop) and implicit list roles (ActionList ins rendered as list by default), group titles are proper heading tags. - - - {_internalBackwardCompatibleTitle ?? children} - - {auxiliaryText &&
    {auxiliaryText}
    } -     + + {sx !== defaultSxProp ? ( + + {_internalBackwardCompatibleTitle ?? children} + + ) : ( + + {_internalBackwardCompatibleTitle ?? children} + + )} + {auxiliaryText &&
    {auxiliaryText}
    } +     )} ) diff --git a/packages/react/src/ActionList/Heading.tsx b/packages/react/src/ActionList/Heading.tsx index d024be646c7..a100a3fef74 100644 --- a/packages/react/src/ActionList/Heading.tsx +++ b/packages/react/src/ActionList/Heading.tsx @@ -12,6 +12,7 @@ import {invariant} from '../utils/invariant' import {clsx} from 'clsx' import {useFeatureFlag} from '../FeatureFlags' import classes from './Heading.module.css' +import {actionListCssModulesFlag} from './featureflag' type HeadingLevels = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' type HeadingVariants = 'large' | 'medium' | 'small' @@ -27,7 +28,7 @@ export const Heading = forwardRef( const innerRef = React.useRef(null) useRefObjectAsForwardedRef(forwardedRef, innerRef) - const enabled = useFeatureFlag('primer_react_css_modules_team') + const enabled = useFeatureFlag(actionListCssModulesFlag) const {headingId: headingId, variant: listVariant} = React.useContext(ListContext) const {container} = React.useContext(ActionListContainerContext) diff --git a/packages/react/src/ActionList/Item.test.tsx b/packages/react/src/ActionList/Item.test.tsx new file mode 100644 index 00000000000..a242a02e907 --- /dev/null +++ b/packages/react/src/ActionList/Item.test.tsx @@ -0,0 +1,356 @@ +import {render as HTMLRender, waitFor, fireEvent} from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import React from 'react' +import {ActionList} from '.' +import {BookIcon} from '@primer/octicons-react' +import {FeatureFlags} from '../FeatureFlags' + +function SimpleActionList(): JSX.Element { + return ( + + New file + + Copy link + Edit file + Delete file + + Link Item + + + ) +} + +const projects = [ + {name: 'Primer Backlog', scope: 'GitHub'}, + {name: 'Primer React', scope: 'github/primer'}, + {name: 'Disabled Project', scope: 'github/primer', disabled: true}, + {name: 'Inactive Project', scope: 'github/primer', inactiveText: 'Unavailable due to an outage'}, + {name: 'Loading Project', scope: 'github/primer', loading: true}, + { + name: 'Inactive and Loading Project', + scope: 'github/primer', + loading: true, + inactiveText: 'Unavailable due to an outage, but loading still passed', + }, +] + +function SingleSelectListStory(): JSX.Element { + const [selectedIndex, setSelectedIndex] = React.useState(0) + + return ( + + {projects.map((project, index) => ( + setSelectedIndex(index)} + disabled={project.disabled} + inactiveText={project.inactiveText} + loading={project.loading} + > + {project.name} + + ))} + + ) +} + +describe('ActionList.Item', () => { + it('should have aria-keyshortcuts applied to the correct element', async () => { + const {container} = HTMLRender() + const linkOptions = await waitFor(() => container.querySelectorAll('a')) + expect(linkOptions[0]).toHaveAttribute('aria-keyshortcuts', 'd') + expect(linkOptions[0].parentElement).not.toHaveAttribute('aria-keyshortcuts', 'd') + }) + it('should fire onSelect on click and keypress', async () => { + const component = HTMLRender() + const options = await waitFor(() => component.getAllByRole('option')) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[1]).toHaveAttribute('aria-selected', 'false') + fireEvent.click(options[1]) + expect(options[0]).toHaveAttribute('aria-selected', 'false') + expect(options[1]).toHaveAttribute('aria-selected', 'true') + // We pass keycode here to navigate a implementation detail in react-testing-library + // https://github.com/testing-library/react-testing-library/issues/269#issuecomment-455854112 + fireEvent.keyPress(options[0], {key: 'Enter', charCode: 13}) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[1]).toHaveAttribute('aria-selected', 'false') + fireEvent.keyPress(options[1], {key: ' ', charCode: 32}) + expect(options[0]).toHaveAttribute('aria-selected', 'false') + expect(options[1]).toHaveAttribute('aria-selected', 'true') + }) + it('should skip onSelect on disabled items', async () => { + const component = HTMLRender() + const options = await waitFor(() => component.getAllByRole('option')) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[2]).toHaveAttribute('aria-selected', 'false') + fireEvent.click(options[2]) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[2]).toHaveAttribute('aria-selected', 'false') + fireEvent.keyPress(options[2], {key: 'Enter', charCode: 13}) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[2]).toHaveAttribute('aria-selected', 'false') + }) + it('should skip onSelect on inactive items', async () => { + const component = HTMLRender() + const options = await waitFor(() => component.getAllByRole('option')) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[3]).toHaveAttribute('aria-selected', 'false') + fireEvent.click(options[3]) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[3]).toHaveAttribute('aria-selected', 'false') + fireEvent.keyPress(options[3], {key: 'Enter', charCode: 13}) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[3]).toHaveAttribute('aria-selected', 'false') + }) + it('should skip onSelect on loading items', async () => { + const component = HTMLRender() + const options = await waitFor(() => component.getAllByRole('option')) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[4]).toHaveAttribute('aria-selected', 'false') + fireEvent.click(options[4]) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[4]).toHaveAttribute('aria-selected', 'false') + fireEvent.keyPress(options[3], {key: 'Enter', charCode: 13}) + expect(options[0]).toHaveAttribute('aria-selected', 'true') + expect(options[4]).toHaveAttribute('aria-selected', 'false') + }) + it('should not crash when clicking an item without an onSelect', async () => { + const component = HTMLRender( + + Primer React + , + ) + const option = await waitFor(() => component.getByRole('option')) + expect(option).toBeInTheDocument() + fireEvent.click(option) + fireEvent.keyPress(option, {key: 'Enter', charCode: 13}) + expect(option).toBeInTheDocument() + }) + it('should focus the button around the leading visual when tabbing to an inactive item', async () => { + const component = HTMLRender() + const inactiveOptionButton = await waitFor(() => component.getByRole('button', {name: projects[3].inactiveText})) + await userEvent.tab() // get focus on first element + await userEvent.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') + expect(inactiveOptionButton).toHaveFocus() + }) + it('should behave as inactive if both inactiveText and loading props are passed', async () => { + const component = HTMLRender() + const inactiveOptionButton = await waitFor(() => component.getByRole('button', {name: projects[5].inactiveText})) + await userEvent.tab() // get focus on first element + await userEvent.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') + await userEvent.keyboard('{ArrowDown}') + expect(inactiveOptionButton).toHaveFocus() + }) + it('should call onClick for a link item', async () => { + const onClick = jest.fn() + const component = HTMLRender( + + + Primer React + + , + ) + const link = await waitFor(() => component.getByRole('link')) + fireEvent.click(link) + expect(onClick).toHaveBeenCalled() + }) + it('should render ActionList.Item as button when feature flag is enabled', async () => { + const featureFlag = { + primer_react_css_modules_team: true, + primer_react_css_modules_staff: true, + primer_react_css_modules_ga: true, + } + const {container} = HTMLRender( + + + Item 1 + Item 2 + + , + ) + const button = container.querySelector('button') + expect(button).toHaveTextContent('Item 1') + // Ensure passed prop "disabled" is applied to the button + expect(button).toHaveAttribute('aria-disabled', 'true') + const listItems = container.querySelectorAll('li') + expect(listItems.length).toBe(2) + }) + it('should render ActionList.Item as li when feature flag is disabled', async () => { + const {container} = HTMLRender( + + + Item 1 + Item 2 + + , + ) + const listitem = container.querySelector('li') + const button = container.querySelector('button') + expect(listitem).toHaveTextContent('Item 1') + expect(listitem).toHaveAttribute('tabindex', '0') + expect(button).toBeNull() + const listItems = container.querySelectorAll('li') + expect(listItems.length).toBe(2) + }) + it('should apply ref to ActionList.Item when feature flag is disabled', async () => { + const MockComponent = () => { + const ref = React.useRef(null) + const focusRef = () => { + if (ref.current) ref.current.focus() + } + return ( + + + + Item 1 + Item 2 + + + ) + } + const {getByRole} = HTMLRender() + const triggerBtn = getByRole('button', {name: 'Prompt'}) + const focusTarget = getByRole('listitem', {name: 'Item 1'}) + fireEvent.click(triggerBtn) + expect(document.activeElement).toBe(focusTarget) + }) + it('should render ActionList.Item as li when item has proper aria role', async () => { + const {container} = HTMLRender( + + Item 1 + Item 2 + , + ) + const listitem = container.querySelector('li') + const button = container.querySelector('button') + expect(listitem).toHaveTextContent('Item 1') + expect(listitem).toHaveAttribute('tabindex', '0') + expect(button).toBeNull() + const listItems = container.querySelectorAll('li') + expect(listItems.length).toBe(2) + }) + it('should render the trailing action as a button (default)', async () => { + const {container} = HTMLRender( + + + Item 1 + + + , + ) + const action = container.querySelector('button[aria-labelledby]') + expect(action).toHaveAccessibleName('Action') + }) + it('should render the trailing action as a link', async () => { + const {container} = HTMLRender( + + + Item 1 + + + , + ) + const action = container.querySelector('a[href="#"][aria-labelledby]') + expect(action).toHaveAccessibleName('Action') + }) + it('should do action when trailing action is clicked', async () => { + const onClick = jest.fn() + const component = HTMLRender( + + + Item 1 + + + , + ) + const trailingAction = await waitFor(() => component.getByRole('button', {name: 'Action'})) + fireEvent.click(trailingAction) + expect(onClick).toHaveBeenCalled() + }) + it('should focus the trailing action', async () => { + HTMLRender( + + + Item 1 + + + , + ) + await userEvent.tab() + expect(document.activeElement).toHaveTextContent('Item 1') + await userEvent.tab() + expect(document.activeElement).toHaveAccessibleName('Action') + }) + it('should only trigger a key event once when feature flag is enabled', async () => { + const mockOnSelect = jest.fn() + const user = userEvent.setup() + const {getByRole} = HTMLRender( + + + Item 1 + + , + ) + const item = getByRole('button') + item.focus() + expect(document.activeElement).toBe(item) + await user.keyboard('{Enter}') + expect(mockOnSelect).toHaveBeenCalledTimes(1) + }) + it('should not render buttons when feature flag is enabled and is specified role', async () => { + const {getByRole} = HTMLRender( + + + Item 1 + Item 2 + Item 3 + Item 4 + Item 5 + + , + ) + const option = getByRole('option') + expect(option.tagName).toBe('LI') + expect(option.textContent).toBe('Item 1') + const menuItem = getByRole('menuitem') + expect(menuItem.tagName).toBe('LI') + const menuItemCheckbox = getByRole('menuitemcheckbox') + expect(menuItemCheckbox.tagName).toBe('LI') + const menuItemRadio = getByRole('menuitemradio') + expect(menuItemRadio.tagName).toBe('LI') + const button = getByRole('button') + expect(button.parentElement?.tagName).toBe('LI') + expect(button.textContent).toBe('Item 5') + }) +}) diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx index fe614b91e43..d90b40a1ce8 100644 --- a/packages/react/src/ActionList/Item.tsx +++ b/packages/react/src/ActionList/Item.tsx @@ -15,20 +15,42 @@ import {GroupContext} from './Group' import type {ActionListItemProps, ActionListProps} from './shared' import {Selection} from './Selection' import {LeadingVisual, TrailingVisual, VisualOrIndicator} from './Visuals' -import {getVariantStyles, ItemContext, TEXT_ROW_HEIGHT, ListContext} from './shared' +import {getVariantStyles, ItemContext, ListContext} from './shared' import {TrailingAction} from './TrailingAction' import {ConditionalWrapper} from '../internal/components/ConditionalWrapper' import {invariant} from '../utils/invariant' import {useFeatureFlag} from '../FeatureFlags' import VisuallyHidden from '../_VisuallyHidden' +import classes from './ActionList.module.css' +import {clsx} from 'clsx' + +import {actionListCssModulesFlag} from './featureflag' const LiBox = styled.li(sx) -const ButtonItemContainer = React.forwardRef(({as: Component = 'button', children, styles, ...props}, forwardedRef) => { +type ActionListSubItemProps = { + children?: React.ReactNode +} + +export const SubItem: React.FC = ({children}) => { + return <>{children} +} + +SubItem.displayName = 'ActionList.SubItem' + +const ButtonItemContainerNoBox = React.forwardRef(({children, style, ...props}, forwardedRef) => { + return ( + + ) +}) as PolymorphicForwardRefComponent + +const DivItemContainerNoBox = React.forwardRef(({children, ...props}, forwardedRef) => { return ( - +
    } {...props}> {children} - +
    ) }) as PolymorphicForwardRefComponent @@ -46,23 +68,36 @@ export const Item = React.forwardRef( role, loading, _PrivateItemWrapper, + className, ...props }, forwardedRef, ): JSX.Element => { - const [slots, childrenWithoutSlots] = useSlots(props.children, { + const enabled = useFeatureFlag(actionListCssModulesFlag) + + const baseSlots = { leadingVisual: LeadingVisual, trailingVisual: TrailingVisual, trailingAction: TrailingAction, - blockDescription: [Description, props => props.variant === 'block'], - inlineDescription: [Description, props => props.variant !== 'block'], - }) + subItem: SubItem, + } + + const [partialSlots, childrenWithoutSlots] = useSlots( + props.children, + enabled + ? {...baseSlots, description: Description} + : { + ...baseSlots, + blockDescription: [Description, props => props.variant === 'block'], + inlineDescription: [Description, props => props.variant !== 'block'], + }, + ) + + const slots = {blockDescription: undefined, inlineDescription: undefined, description: undefined, ...partialSlots} const {container, afterSelect, selectionAttribute, defaultTrailingVisual} = React.useContext(ActionListContainerContext) - const buttonSemanticsFeatureFlag = useFeatureFlag('primer_react_action_list_item_as_button') - // Be sure to avoid rendering the container unless there is a default const wrappedDefaultTrailingVisual = defaultTrailingVisual ? ( {defaultTrailingVisual} @@ -124,9 +159,8 @@ export const Item = React.forwardRef( role === 'option' || role === 'menuitem' || role === 'menuitemradio' || role === 'menuitemcheckbox' const listRoleTypes = ['listbox', 'menu', 'list'] - const listSemantics = - (listRole && listRoleTypes.includes(listRole)) || inactive || container === 'NavList' || listItemSemantics - const buttonSemantics = !listSemantics && !_PrivateItemWrapper && buttonSemanticsFeatureFlag + const listSemantics = (listRole && listRoleTypes.includes(listRole)) || inactive || listItemSemantics || !enabled + const buttonSemantics = !listSemantics && !_PrivateItemWrapper const {theme} = useTheme() @@ -140,49 +174,20 @@ export const Item = React.forwardRef( width: '4px', height: '24px', content: '""', - bg: 'accent.fg', + bg: 'var(--borderColor-accent-emphasis)', borderRadius: 2, }, } - const hoverStyles = { - '@media (hover: hover) and (pointer: fine)': { - '&:hover:not([aria-disabled]):not([data-inactive])': { - backgroundColor: `actionListItem.${variant}.hoverBg`, - color: getVariantStyles(variant, disabled, inactive).hoverColor, - boxShadow: `inset 0 0 0 max(1px, 0.0625rem) ${theme?.colors.actionListItem.default.activeBorder}`, - }, - '&:focus-visible, > a.focus-visible, &:focus.focus-visible': { - outline: 'none', - border: `2 solid`, - boxShadow: `0 0 0 2px var(--focus-outlineColor)`, - }, - '&:active:not([aria-disabled]):not([data-inactive])': { - backgroundColor: `actionListItem.${variant}.activeBg`, - color: getVariantStyles(variant, disabled, inactive).hoverColor, - }, - }, - } - - const listItemStyles = { - display: 'flex', - // show between 2 items - ':not(:first-of-type)': {'--divider-color': theme?.colors.actionListItem.inlineDivider}, - width: buttonSemantics && listVariant !== 'full' ? 'calc(100% - 16px)' : '100%', - marginX: buttonSemantics && listVariant !== 'full' ? '2' : '0', - borderRadius: 2, - ...(buttonSemantics ? hoverStyles : {}), - } - const styles = { position: 'relative', display: 'flex', paddingX: 2, fontSize: 1, paddingY: '6px', // custom value off the scale - lineHeight: TEXT_ROW_HEIGHT, + lineHeight: '16px', minHeight: 5, - marginX: listVariant === 'inset' && !buttonSemantics ? 2 : 0, + marginX: listVariant === 'inset' ? 2 : 0, borderRadius: 2, transition: 'background 33.333ms linear', color: getVariantStyles(variant, disabled, inactive || loading).color, @@ -206,7 +211,7 @@ export const Item = React.forwardRef( appearance: 'none', background: 'unset', border: 'unset', - width: listVariant === 'inset' && !buttonSemantics ? 'calc(100% - 16px)' : '100%', + width: listVariant === 'inset' ? 'calc(100% - 16px)' : '100%', fontFamily: 'unset', textAlign: 'unset', marginY: 'unset', @@ -218,6 +223,23 @@ export const Item = React.forwardRef( }, }, + '@media (hover: hover) and (pointer: fine)': { + '&:hover:not([aria-disabled]):not([data-inactive])': { + backgroundColor: `actionListItem.${variant}.hoverBg`, + color: getVariantStyles(variant, disabled, inactive).hoverColor, + boxShadow: `inset 0 0 0 max(1px, 0.0625rem) ${theme?.colors.actionListItem.default.activeBorder}`, + }, + '&:focus-visible, > a.focus-visible, &:focus.focus-visible': { + outline: 'none', + border: `2 solid`, + boxShadow: `0 0 0 2px var(--focus-outlineColor)`, + }, + '&:active:not([aria-disabled]):not([data-inactive])': { + backgroundColor: `actionListItem.${variant}.activeBg`, + color: getVariantStyles(variant, disabled, inactive).hoverColor, + }, + }, + /** Divider styles */ '[data-component="ActionList.Item--DividerContainer"]': { position: 'relative', @@ -249,8 +271,6 @@ export const Item = React.forwardRef( /** Active styles */ ...(active ? activeStyles : {}), // NavList '&[data-is-active-descendant]': {...activeStyles, fontWeight: 'normal'}, // SelectPanel - - ...(!buttonSemantics ? hoverStyles : {}), } const clickHandler = React.useCallback( @@ -285,8 +305,10 @@ export const Item = React.forwardRef( const inactiveWarningId = inactive && !showInactiveIndicator ? `${itemId}--warning-message` : undefined let DefaultItemWrapper = React.Fragment - if (buttonSemanticsFeatureFlag) { - DefaultItemWrapper = listSemantics ? React.Fragment : ButtonItemContainer + if (enabled) { + DefaultItemWrapper = listSemantics ? DivItemContainerNoBox : ButtonItemContainerNoBox + } else { + DefaultItemWrapper = React.Fragment } const ItemWrapper = _PrivateItemWrapper || DefaultItemWrapper @@ -313,28 +335,172 @@ export const Item = React.forwardRef( ...(includeSelectionAttribute && {[itemSelectionAttribute]: selected}), role: itemRole, id: itemId, + className, } - let containerProps - let wrapperProps - - if (buttonSemanticsFeatureFlag) { - containerProps = _PrivateItemWrapper - ? {role: itemRole ? 'none' : undefined, ...props} - : // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - (listSemantics && {...menuItemProps, ...props, ref: forwardedRef}) || {} - - wrapperProps = _PrivateItemWrapper - ? menuItemProps - : !listSemantics && { - ...menuItemProps, - ...props, - styles: merge(styles, sxProp), - ref: forwardedRef, - } - } else { - containerProps = _PrivateItemWrapper ? {role: itemRole ? 'none' : undefined} : {...menuItemProps, ...props} - wrapperProps = _PrivateItemWrapper ? menuItemProps : {} + const containerProps = _PrivateItemWrapper + ? {role: itemRole ? 'none' : undefined, ...props} + : // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + (listSemantics && {...menuItemProps, ...props, ref: forwardedRef}) || {} + + const wrapperProps = _PrivateItemWrapper + ? menuItemProps + : !listSemantics && { + ...menuItemProps, + ...props, + ref: forwardedRef, + } + + // Extract the variant prop value from the description slot component + + const descriptionVariant = slots.description?.props.variant ?? 'inline' + + if (enabled) { + if (sxProp !== defaultSxProp) { + return ( + + (styles, sxProp)} + ref={listSemantics ? forwardedRef : null} + data-variant={variant === 'danger' ? variant : undefined} + data-active={active ? true : undefined} + data-inactive={inactiveText ? true : undefined} + data-has-subitem={slots.subItem ? true : undefined} + className={clsx(classes.ActionListItem, className)} + > + + + + + {slots.leadingVisual} + + + + + {childrenWithoutSlots} + {/* Loading message needs to be in here so it is read with the label */} + {loading === true && Loading} + + {slots.description} + + + {trailingVisual} + + + { + // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel), + // render the inactive warning message directly in the item. + inactive && container ? ( + + {inactiveText} + + ) : null + } + + + {!inactive && !loading && !menuContext && Boolean(slots.trailingAction) && slots.trailingAction} + {slots.subItem} + + + ) + } + return ( + +
  • + + + + + {slots.leadingVisual} + + + + + {childrenWithoutSlots} + {/* Loading message needs to be in here so it is read with the label */} + {loading === true && Loading} + + {slots.description} + + + {trailingVisual} + + + { + // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel), + // render the inactive warning message directly in the item. + inactive && container ? ( + + {inactiveText} + + ) : null + } + + + {!inactive && !loading && !menuContext && Boolean(slots.trailingAction) && slots.trailingAction} + {slots.subItem} +
    • +     + ) } return ( @@ -349,15 +515,9 @@ export const Item = React.forwardRef( }} > ( - listSemantics || _PrivateItemWrapper ? styles : listItemStyles, - listSemantics || _PrivateItemWrapper ? sxProp : {}, - ) - : merge(styles, sxProp) - } + ref={listSemantics ? forwardedRef : null} + className={className} + sx={merge(styles, sxProp)} data-variant={variant === 'danger' ? variant : undefined} {...containerProps} > diff --git a/packages/react/src/ActionList/LinkItem.tsx b/packages/react/src/ActionList/LinkItem.tsx index c878a821d1b..fd9d908f292 100644 --- a/packages/react/src/ActionList/LinkItem.tsx +++ b/packages/react/src/ActionList/LinkItem.tsx @@ -6,6 +6,9 @@ import {merge} from '../sx' import {Item} from './Item' import type {ActionListItemProps} from './shared' import Box from '../Box' +import {defaultSxProp} from '../utils/defaultSxProp' +import {useFeatureFlag} from '../FeatureFlags' +import {actionListCssModulesFlag} from './featureflag' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -18,56 +21,119 @@ type LinkProps = { target?: string type?: string referrerPolicy?: React.AnchorHTMLAttributes['referrerPolicy'] + className?: string } // LinkItem does not support selected, loading, variants, etc. -export type ActionListLinkItemProps = Pick & +export type ActionListLinkItemProps = Pick< + ActionListItemProps, + 'active' | 'children' | 'sx' | 'inactiveText' | 'variant' +> & LinkProps -export const LinkItem = React.forwardRef(({sx = {}, active, inactiveText, as: Component, ...props}, forwardedRef) => { - const styles = { - // occupy full size of Item - paddingX: 2, - paddingY: '6px', // custom value off the scale - display: 'flex', - flexGrow: 1, // full width - borderRadius: 2, +export const LinkItem = React.forwardRef( + ({sx = defaultSxProp, active, inactiveText, variant, as: Component, className, ...props}, forwardedRef) => { + const styles = { + // occupy full size of Item + paddingX: 2, + paddingY: '6px', // custom value off the scale + display: 'flex', + flexGrow: 1, // full width + borderRadius: 2, - // inherit Item styles - color: 'inherit', - '&:hover': {color: 'inherit', textDecoration: 'none'}, - } + // inherit Item styles + color: 'inherit', + '&:hover': {color: 'inherit', textDecoration: 'none'}, + } - return ( - { - const clickHandler = (event: React.MouseEvent) => { - onClick && onClick(event) - props.onClick && props.onClick(event as React.MouseEvent) - } - return inactiveText ? ( - - {children} - - ) : ( - { + const clickHandler = (event: React.MouseEvent) => { + onClick && onClick(event) + props.onClick && props.onClick(event as React.MouseEvent) + } + return inactiveText ? ( + {children} + ) : ( + + {children} + + ) + }} > - {children} - + {props.children} + ) - }} - > - {props.children} - - ) -}) as PolymorphicForwardRefComponent<'a', ActionListLinkItemProps> + } + + 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} + + ) + }, +) as PolymorphicForwardRefComponent<'a', ActionListLinkItemProps> diff --git a/packages/react/src/ActionList/List.tsx b/packages/react/src/ActionList/List.tsx index c957b40060a..9b84e3d1424 100644 --- a/packages/react/src/ActionList/List.tsx +++ b/packages/react/src/ActionList/List.tsx @@ -14,6 +14,7 @@ import {FocusKeys, useFocusZone} from '../hooks/useFocusZone' import {clsx} from 'clsx' import {useFeatureFlag} from '../FeatureFlags' import classes from './ActionList.module.css' +import {actionListCssModulesFlag} from './featureflag' const ListBox = styled.ul(sx) @@ -57,7 +58,7 @@ export const List = React.forwardRef( focusOutBehavior: listRole === 'menu' ? 'wrap' : undefined, }) - const enabled = useFeatureFlag('primer_react_css_modules_team') + const enabled = useFeatureFlag(actionListCssModulesFlag) return ( -export const Selection: React.FC> = ({selected}) => { +type SelectionProps = Pick +export const Selection: React.FC> = ({selected, className}) => { const {selectionVariant: listSelectionVariant, role: listRole} = React.useContext(ListContext) const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext) + const enabled = useFeatureFlag(actionListCssModulesFlag) + /** selectionVariant in Group can override the selectionVariant in List root */ /** fallback to selectionVariant from container menu if any (ActionMenu, SelectPanel ) */ let selectionVariant: ActionListProps['selectionVariant'] | ActionListGroupProps['selectionVariant'] @@ -30,6 +35,13 @@ export const Selection: React.FC> = ({se } if (selectionVariant === 'single' || listRole === 'menu') { + if (enabled) { + return ( + + + + ) + } return ( {selected && } @@ -62,6 +74,13 @@ export const Selection: React.FC> = ({se }, } + if (enabled) { + return ( + +
    + + ) + } return ( { - if (!icon) { - return ( - - {/* @ts-expect-error TODO: Fix this */} - - - ) - } else { +export const TrailingAction = forwardRef( + ({as = 'button', icon, label, href = null, className, ...props}, forwardedRef) => { + const enabled = useFeatureFlag(actionListCssModulesFlag) + + if (enabled) { + return ( + + {icon ? ( + + ) : ( + // @ts-expect-error shhh + + )} + + ) + } + return ( - + {icon ? ( + + ) : ( + // @ts-expect-error shhh + + )} ) - } -}) as PolymorphicForwardRefComponent<'button' | 'a', ActionListTrailingActionProps> + }, +) as PolymorphicForwardRefComponent<'button' | 'a', ActionListTrailingActionProps> TrailingAction.displayName = 'ActionList.TrailingAction' diff --git a/packages/react/src/ActionList/Visuals.tsx b/packages/react/src/ActionList/Visuals.tsx index 4e75348f67a..3cad24a0eb2 100644 --- a/packages/react/src/ActionList/Visuals.tsx +++ b/packages/react/src/ActionList/Visuals.tsx @@ -4,19 +4,39 @@ import Box from '../Box' import Spinner from '../Spinner' import type {SxProp} from '../sx' import {merge} from '../sx' -import {ItemContext, TEXT_ROW_HEIGHT, getVariantStyles} from './shared' +import {ItemContext, getVariantStyles} from './shared' import {Tooltip, type TooltipProps} from '../TooltipV2' +import {clsx} from 'clsx' +import {useFeatureFlag} from '../FeatureFlags' +import classes from './ActionList.module.css' +import {defaultSxProp} from '../utils/defaultSxProp' +import {actionListCssModulesFlag} from './featureflag' export type VisualProps = SxProp & React.HTMLAttributes -export const LeadingVisualContainer: React.FC> = ({sx = {}, ...props}) => { +export const VisualContainer: React.FC> = ({ + sx = defaultSxProp, + className, + ...props +}) => { + if (sx !== defaultSxProp) { + return + } + return +} + +// remove when primer_react_css_modules_X is shipped +export const LeadingVisualContainer: React.FC> = ({ + sx = defaultSxProp, + ...props +}) => { return ( > = ({sx = {}, ...props}) => { +export const LeadingVisual: React.FC> = ({ + sx = defaultSxProp, + className, + ...props +}) => { const {variant, disabled, inactive} = React.useContext(ItemContext) + + const enabled = useFeatureFlag(actionListCssModulesFlag) + + if (enabled) { + return ( + + {props.children} + + ) + } return ( > = ({s } export type ActionListTrailingVisualProps = VisualProps -export const TrailingVisual: React.FC> = ({sx = {}, ...props}) => { +export const TrailingVisual: React.FC> = ({ + sx = defaultSxProp, + className, + ...props +}) => { const {variant, disabled, inactive, trailingVisualId} = React.useContext(ItemContext) + const enabled = useFeatureFlag(actionListCssModulesFlag) + if (enabled) { + if (sx !== defaultSxProp) { + return ( + + {props.children} + + ) + } + return ( + + {props.children} + + ) + } return ( -> = ({children, labelId, loading, inactiveText, itemHasLeadingVisual, position}) => { +> = ({children, labelId, loading, inactiveText, itemHasLeadingVisual, position, className}) => { const VisualComponent = position === 'leading' ? LeadingVisual : TrailingVisual if (!loading && !inactiveText) return children @@ -111,26 +167,17 @@ export const VisualOrIndicator: React.FC< } return inactiveText ? ( - - - - - - - + + + + + ) : ( - + ) diff --git a/packages/react/src/ActionList/featureflag.ts b/packages/react/src/ActionList/featureflag.ts new file mode 100644 index 00000000000..f4e5922f54d --- /dev/null +++ b/packages/react/src/ActionList/featureflag.ts @@ -0,0 +1 @@ +export const actionListCssModulesFlag = 'primer_react_css_modules_team' diff --git a/packages/react/src/ActionList/shared.ts b/packages/react/src/ActionList/shared.ts index a8f62cfc86d..bae0b66c2a9 100644 --- a/packages/react/src/ActionList/shared.ts +++ b/packages/react/src/ActionList/shared.ts @@ -52,6 +52,7 @@ export type ActionListItemProps = { * Private API for use internally only. Used by LinkItem to wrap contents in an anchor */ _PrivateItemWrapper?: React.FC> + className?: string } & SxProp type MenuItemProps = { @@ -62,6 +63,7 @@ type MenuItemProps = { 'aria-labelledby'?: string 'aria-describedby'?: string role?: string + className?: string } export type ItemContext = Pick & { diff --git a/packages/react/src/Autocomplete/AutocompleteMenu.module.css b/packages/react/src/Autocomplete/AutocompleteMenu.module.css new file mode 100644 index 00000000000..5806e54c560 --- /dev/null +++ b/packages/react/src/Autocomplete/AutocompleteMenu.module.css @@ -0,0 +1,9 @@ +.SpinnerWrapper { + display: flex; + justify-content: center; + padding: var(--base-size-16); +} + +.EmptyStateWrapper { + padding: var(--base-size-16); +} diff --git a/packages/react/src/Autocomplete/AutocompleteMenu.tsx b/packages/react/src/Autocomplete/AutocompleteMenu.tsx index cb09cdacc2c..dc348b14345 100644 --- a/packages/react/src/Autocomplete/AutocompleteMenu.tsx +++ b/packages/react/src/Autocomplete/AutocompleteMenu.tsx @@ -13,6 +13,9 @@ import type {IconProps} from '@primer/octicons-react' import {PlusIcon} from '@primer/octicons-react' import VisuallyHidden from '../_VisuallyHidden' import {isElement} from 'react-is' +import {useFeatureFlag} from '../FeatureFlags' + +import classes from './AutocompleteMenu.module.css' type OnSelectedChange = (item: T | T[]) => void export type AutocompleteMenuItem = MandateProps & { @@ -117,6 +120,8 @@ export type AutocompleteMenuInternalProps = { ['aria-labelledby']: string } +const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_team' + function AutocompleteMenu(props: AutocompleteMenuInternalProps) { const autocompleteContext = useContext(AutocompleteContext) if (autocompleteContext === null) { @@ -324,12 +329,20 @@ function AutocompleteMenu(props: AutocompleteMe throw new Error('Autocomplete: selectionVariant "single" cannot be used with multiple selected items') } + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + return ( {loading ? ( - - - + enabled ? ( + + + + ) : ( + + + + ) ) : (
    {allItemsToRender.length ? ( @@ -367,7 +380,11 @@ function AutocompleteMenu(props: AutocompleteMe })} ) : emptyStateText !== false && emptyStateText !== null ? ( - {emptyStateText} + enabled ? ( + {emptyStateText} + ) : ( + {emptyStateText} + ) ) : null}
    )} diff --git a/packages/react/src/Banner/Banner.figma.tsx b/packages/react/src/Banner/Banner.figma.tsx index cb37565ab9e..b711599eee6 100644 --- a/packages/react/src/Banner/Banner.figma.tsx +++ b/packages/react/src/Banner/Banner.figma.tsx @@ -1,7 +1,7 @@ // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-nocheck import React from 'react' -import {Banner} from '../../src/drafts' +import {Banner} from '../../src/experimental/' import figma from '@figma/code-connect' const componentProps = { diff --git a/packages/react/src/Box/Box.tsx b/packages/react/src/Box/Box.tsx index b93aeac4e4e..816a9fe2873 100644 --- a/packages/react/src/Box/Box.tsx +++ b/packages/react/src/Box/Box.tsx @@ -12,11 +12,13 @@ import type { TypographyProps, } from 'styled-system' import {background, border, color, flexbox, grid, layout, position, shadow, space, typography} from 'styled-system' -import type {SxProp} from '../sx' +import type {BetterSystemStyleObject} from '../sx' import sx from '../sx' import type {ComponentProps} from '../utils/types' -type StyledBoxProps = SpaceProps & +type StyledBoxProps = { + sx?: BetterSystemStyleObject +} & SpaceProps & ColorProps & TypographyProps & LayoutProps & @@ -25,8 +27,7 @@ type StyledBoxProps = SpaceProps & BackgroundProps & BorderProps & PositionProps & - ShadowProps & - SxProp + ShadowProps const Box = styled.div( space, diff --git a/packages/react/src/Button/Button.features.stories.tsx b/packages/react/src/Button/Button.features.stories.tsx index 61bceaafd6f..e71cfdf3667 100644 --- a/packages/react/src/Button/Button.features.stories.tsx +++ b/packages/react/src/Button/Button.features.stories.tsx @@ -61,9 +61,7 @@ export const TrailingCounter = () => { ) } -export const TrailingCounterWithNoText = () => { - return ) @@ -359,8 +418,15 @@ function TableSortHeader({align, children, direction, onToggleSort, ...rest}: Ta export type TableRowProps = React.ComponentPropsWithoutRef<'tr'> function TableRow({children, ...rest}: TableRowProps) { + const enabled = useFeatureFlag(cssModulesFlag) return ( - + {children} ) @@ -384,11 +450,20 @@ export type TableCellProps = Omit, 'align'> } function TableCell({align, className, children, scope, ...rest}: TableCellProps) { + const enabled = useFeatureFlag(cssModulesFlag) const BaseComponent = scope ? 'th' : 'td' const role = scope ? 'rowheader' : 'cell' return ( - + {children} ) @@ -403,68 +478,82 @@ function TableCellPlaceholder({children}: TableCellPlaceholderProps) { // ---------------------------------------------------------------------------- // TableContainer // ---------------------------------------------------------------------------- -const StyledTableContainer = styled.div` - display: grid; - grid-template-columns: 1fr 1fr; - grid-template-areas: - 'title actions' - 'divider divider' - 'subtitle subtitle' - 'filter filter' - 'table table' - 'footer footer'; - column-gap: ${get('space.2')}; - - ${sx} - - /* TableTitle */ - .TableTitle { - grid-area: title; - align-self: center; - } - - /* TableSubtitle */ - .TableSubtitle { - grid-area: subtitle; - } - - /* TableActions */ - .TableActions { - display: flex; +const StyledTableContainer = toggleStyledComponent( + cssModulesFlag, + 'div', + styled.div` + display: grid; + grid-template-columns: 1fr 1fr; + grid-template-areas: + 'title actions' + 'divider divider' + 'subtitle subtitle' + 'filter filter' + 'table table' + 'footer footer'; column-gap: ${get('space.2')}; - align-items: center; - grid-area: actions; - justify-self: end; - } - - /* TableDivider */ - .TableDivider { - grid-area: divider; - margin-top: ${get('space.3')}; - margin-bottom: ${get('space.2')}; - } - - /* Table */ - .Table { - grid-area: table; - } - - /* Spacing before the table */ - .TableTitle + .TableOverflowWrapper, - .TableSubtitle + .TableOverflowWrapper, - .TableActions + .TableOverflowWrapper { - margin-top: ${get('space.2')}; - } - - .TableOverflowWrapper { - grid-area: table; - } -` + + ${sx} + + /* TableTitle */ + .TableTitle { + grid-area: title; + align-self: center; + } + + /* TableSubtitle */ + .TableSubtitle { + grid-area: subtitle; + } + + /* TableActions */ + .TableActions { + display: flex; + column-gap: ${get('space.2')}; + align-items: center; + grid-area: actions; + justify-self: end; + } + + /* TableDivider */ + .TableDivider { + grid-area: divider; + margin-top: ${get('space.3')}; + margin-bottom: ${get('space.2')}; + } + + /* Table */ + .Table { + grid-area: table; + } + + /* Spacing before the table */ + .TableTitle + .TableOverflowWrapper, + .TableSubtitle + .TableOverflowWrapper, + .TableActions + .TableOverflowWrapper { + margin-top: ${get('space.2')}; + } + + .TableOverflowWrapper { + grid-area: table; + } + `, +) export type TableContainerProps = React.PropsWithChildren function TableContainer({children, sx}: TableContainerProps) { - return {children} + const enabled = useFeatureFlag(cssModulesFlag) + return ( + + {children} + + ) } export type TableTitleProps = React.PropsWithChildren<{ @@ -483,25 +572,33 @@ export type TableTitleProps = React.PropsWithChildren<{ }> const TableTitle = React.forwardRef(function TableTitle({as = 'h2', children, id}, ref) { + const enabled = useFeatureFlag(cssModulesFlag) return ( - {children} - + ) }) +const StyledTableTitle = toggleStyledComponent( + cssModulesFlag, + 'h2', + styled.h2` + color: var(--fgColor-default); + font-size: var(--text-body-size-medium); + font-weight: var(--base-text-weight-semibold); + line-height: calc(20 / 14); + margin: 0; + `, +) + export type TableSubtitleProps = React.PropsWithChildren<{ /** * Provide an alternate element or component to use as the container for @@ -518,42 +615,67 @@ export type TableSubtitleProps = React.PropsWithChildren<{ }> function TableSubtitle({as, children, id}: TableSubtitleProps) { + const enabled = useFeatureFlag(cssModulesFlag) return ( - {children} - + ) } +const StyledTableSubtitle = toggleStyledComponent( + cssModulesFlag, + 'div', + styled.div` + color: var(--fgColor-default); + font-weight: var(--base-text-weight-normal); + font-size: var(--text-body-size-small); + line-height: var(--text-title-lineHeight-small); + margin: 0; + `, +) + function TableDivider() { + const enabled = useFeatureFlag(cssModulesFlag) return ( - ) } +const StyledTableDivider = toggleStyledComponent( + cssModulesFlag, + 'div', + styled.div` + background-color: var(--borderColor-default); + width: 100%; + height: 1px; + `, +) + export type TableActionsProps = React.PropsWithChildren function TableActions({children}: TableActionsProps) { - return
    {children}
    + const enabled = useFeatureFlag(cssModulesFlag) + return ( +
    + {children} +
    + ) } // ---------------------------------------------------------------------------- @@ -580,6 +702,7 @@ export type TableSkeletonProps = React.ComponentPropsWit } function TableSkeleton({cellPadding, columns, rows = 10, ...rest}: TableSkeletonProps) { + const enabled = useFeatureFlag(cssModulesFlag) const {gridTemplateColumns} = useTableLayout(columns) return ( @@ -600,12 +723,26 @@ function TableSkeleton({cellPadding, columns, rows = 10, {Array.from({length: columns.length}).map((_, i) => { return ( - + Loading -
    +
    {Array.from({length: rows}).map((_, i) => { return ( -
    +
    ) @@ -620,32 +757,6 @@ function TableSkeleton({cellPadding, columns, rows = 10, ) } -// ---------------------------------------------------------------------------- -// Utilities -// ---------------------------------------------------------------------------- - -// Button "reset" component that provides an unstyled