Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/busy-islands-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

ActionBar: Adds `ActionBar.Group` sub component
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 16 additions & 0 deletions e2e/components/drafts/ActionBar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,22 @@ test.describe('ActionBar', () => {
}
})

test.describe('Groups', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'experimental-components-actionbar-examples--with-groups',
globals: {
colorScheme: theme,
},
})
expect(await page.screenshot()).toMatchSnapshot(`drafts.ActionBar.WithGroups.${theme}.png`)
})
})
}
})

test.describe('ActionBar Interactions', () => {
for (const theme of themes) {
test.describe(theme, () => {
Expand Down
10 changes: 10 additions & 0 deletions packages/react/src/ActionBar/ActionBar.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@
{
"name": "ActionBar.Divider",
"props": []
},
{
"name": "ActionBar.Group",
"props": [
{
"name": "children",
"type": "React.ReactNode",
"defaultValue": ""
}
]
}
]
}
24 changes: 24 additions & 0 deletions packages/react/src/ActionBar/ActionBar.examples.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,30 @@ export default {
title: 'Experimental/Components/ActionBar/Examples',
} as Meta<typeof ActionBar>

export const WithGroups = () => (
<ActionBar aria-label="Toolbar">
<ActionBar.Group>
<>
<ActionBar.IconButton icon={BoldIcon} aria-label="Bold"></ActionBar.IconButton>
<ActionBar.IconButton icon={ItalicIcon} aria-label="Italic"></ActionBar.IconButton>
<ActionBar.IconButton icon={CodeIcon} aria-label="Code"></ActionBar.IconButton>
<ActionBar.IconButton icon={LinkIcon} aria-label="Link"></ActionBar.IconButton>
</>
</ActionBar.Group>
<ActionBar.Divider />
<ActionBar.Group>
<ActionBar.IconButton icon={FileAddedIcon} aria-label="File Added"></ActionBar.IconButton>
<ActionBar.IconButton icon={SearchIcon} aria-label="Search"></ActionBar.IconButton>
</ActionBar.Group>
<ActionBar.Group>
<ActionBar.IconButton icon={ListUnorderedIcon} aria-label="Unordered List"></ActionBar.IconButton>
<ActionBar.IconButton icon={ListOrderedIcon} aria-label="Ordered List"></ActionBar.IconButton>
</ActionBar.Group>
<ActionBar.IconButton icon={TasklistIcon} aria-label="Task List"></ActionBar.IconButton>
<ActionBar.IconButton icon={ReplyIcon} aria-label="Saved Replies"></ActionBar.IconButton>
</ActionBar>
)

export const TextLabels = () => (
<ActionBar aria-label="Toolbar">
<Button>Edit</Button>
Expand Down
5 changes: 5 additions & 0 deletions packages/react/src/ActionBar/ActionBar.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@
background: var(--borderColor-muted);
}
}

.Group {
display: flex;
gap: inherit;
}
30 changes: 29 additions & 1 deletion packages/react/src/ActionBar/ActionBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,32 @@ describe('ActionBar Registry System', () => {
expect(buttons[2]).toHaveAccessibleName('Third')
})

it('should preserve group order with deep nesting', () => {
render(
<ActionBar aria-label="Deep test">
<div>
<ActionBar.Group>
<ActionBar.IconButton icon={BoldIcon} aria-label="First" />
</ActionBar.Group>
</div>
<ActionBar.Group>
<ActionBar.IconButton icon={ItalicIcon} aria-label="Second" />
</ActionBar.Group>
<div>
<ActionBar.Group>
<ActionBar.IconButton icon={CodeIcon} aria-label="Third" />
</ActionBar.Group>
</div>
</ActionBar>,
)

const buttons = screen.getAllByRole('button')
expect(buttons).toHaveLength(3)
expect(buttons[0]).toHaveAccessibleName('First')
expect(buttons[1]).toHaveAccessibleName('Second')
expect(buttons[2]).toHaveAccessibleName('Third')
})

it('should handle conditional rendering without breaking order', async () => {
const ConditionalTest = () => {
const [show, setShow] = useState([true, true, true])
Expand All @@ -108,7 +134,9 @@ describe('ActionBar Registry System', () => {
<div>
<ActionBar aria-label="Conditional">
{show[0] && <ActionBar.IconButton icon={BoldIcon} aria-label="First" />}
{show[1] && <ActionBar.IconButton icon={ItalicIcon} aria-label="Second" />}
<ActionBar.Group>
{show[1] && <ActionBar.IconButton icon={ItalicIcon} aria-label="Second" />}
</ActionBar.Group>
{show[2] && <ActionBar.IconButton icon={CodeIcon} aria-label="Third" />}
</ActionBar>
<button type="button" onClick={() => setShow([false, true, true])}>
Expand Down
111 changes: 105 additions & 6 deletions packages/react/src/ActionBar/ActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ type ChildProps =
icon: ActionBarIconButtonProps['icon']
onClick: MouseEventHandler
width: number
groupId?: string
}
| {type: 'divider'; width: number}
| {type: 'group'; width: number}

/**
* Registry of descendants to render in the list or menu. To preserve insertion order across updates, children are
Expand All @@ -38,9 +40,16 @@ type ChildRegistry = ReadonlyMap<string, ChildProps | null>
const ActionBarContext = React.createContext<{
size: Size
registerChild: (id: string, props: ChildProps) => void
unregisterChild: (id: string) => void
unregisterChild: (id: string, groupId?: string) => void
isVisibleChild: (id: string) => boolean
}>({size: 'medium', registerChild: () => {}, unregisterChild: () => {}, isVisibleChild: () => true})
groupId?: string
}>({
size: 'medium',
registerChild: () => {},
unregisterChild: () => {},
isVisibleChild: () => true,
groupId: undefined,
})

/*
small (28px), medium (32px), large (40px)
Expand Down Expand Up @@ -107,7 +116,10 @@ const getMenuItems = (
childRegistry: ChildRegistry,
hasActiveMenu: boolean,
): Set<string> | void => {
const registryEntries = Array.from(childRegistry).filter((entry): entry is [string, ChildProps] => entry[1] !== null)
const registryEntries = Array.from(childRegistry).filter(
(entry): entry is [string, ChildProps] =>
entry[1] !== null && (entry[1].type !== 'action' || entry[1].groupId === undefined),
)

if (registryEntries.length === 0) return new Set()
const numberOfItemsPossible = calculatePossibleItems(registryEntries, navWidth)
Expand Down Expand Up @@ -223,6 +235,19 @@ export const ActionBar: React.FC<React.PropsWithChildren<ActionBarProps>> = prop
focusOutBehavior: 'wrap',
})

const groupedItems = React.useMemo(() => {
const groupedItemsMap = new Map<string, Array<[string, ChildProps]>>()

for (const [key, childProps] of childRegistry) {
if (childProps?.type === 'action' && childProps.groupId) {
const existingGroup = groupedItemsMap.get(childProps.groupId) || []
existingGroup.push([key, childProps])
groupedItemsMap.set(childProps.groupId, existingGroup)
}
}
return groupedItemsMap
}, [childRegistry])

return (
<ActionBarContext.Provider value={{size, registerChild, unregisterChild, isVisibleChild}}>
<div ref={navRef} className={clsx(className, styles.Nav)} data-flush={flush}>
Expand All @@ -248,7 +273,7 @@ export const ActionBar: React.FC<React.PropsWithChildren<ActionBarProps>> = prop

if (menuItem.type === 'divider') {
return <ActionList.Divider key={id} />
} else {
} else if (menuItem.type === 'action') {
const {onClick, icon: Icon, label, disabled} = menuItem
return (
<ActionList.Item
Expand All @@ -268,6 +293,40 @@ export const ActionBar: React.FC<React.PropsWithChildren<ActionBarProps>> = prop
</ActionList.Item>
)
}

// Use the memoized map instead of filtering each time
const groupedMenuItems = groupedItems.get(id) || []

// If we ever add additional types, this condition will be necessary
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (menuItem.type === 'group') {
return (
<React.Fragment key={id}>
{groupedMenuItems.map(([key, childProps]) => {
if (childProps.type === 'action') {
const {onClick, icon: Icon, label, disabled} = childProps
return (
<ActionList.Item
key={key}
onSelect={event => {
closeOverlay()
focusOnMoreMenuBtn()
typeof onClick === 'function' && onClick(event as React.MouseEvent<HTMLElement>)
}}
disabled={disabled}
>
<ActionList.LeadingVisual>
<Icon />
</ActionList.LeadingVisual>
{label}
</ActionList.Item>
)
}
return null
})}
</React.Fragment>
)
}
})}
</ActionList>
</ActionMenu.Overlay>
Expand All @@ -286,6 +345,7 @@ export const ActionBarIconButton = forwardRef(
const id = useId()

const {size, registerChild, unregisterChild, isVisibleChild} = React.useContext(ActionBarContext)
const {groupId} = React.useContext(ActionBarGroupContext)

// Storing the width in a ref ensures we don't forget about it when not visible
const widthRef = useRef<number>()
Expand All @@ -302,9 +362,12 @@ export const ActionBarIconButton = forwardRef(
disabled: !!disabled,
onClick: onClick as MouseEventHandler,
width: widthRef.current,
groupId: groupId ?? undefined,
})

return () => unregisterChild(id)
return () => {
unregisterChild(id)
}
Comment on lines +368 to +370
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The cleanup function can be simplified to return () => unregisterChild(id) since it's just a single function call.

Suggested change
return () => {
unregisterChild(id)
}
return () => unregisterChild(id)

Copilot uses AI. Check for mistakes.
}, [registerChild, unregisterChild, props['aria-label'], props.icon, disabled, onClick])

const clickHandler = useCallback(
Expand All @@ -315,7 +378,7 @@ export const ActionBarIconButton = forwardRef(
[disabled, onClick],
)

if (!isVisibleChild(id)) return null
if (!isVisibleChild(id) || (groupId && !isVisibleChild(groupId))) return null

return (
<IconButton
Expand All @@ -325,11 +388,47 @@ export const ActionBarIconButton = forwardRef(
onClick={clickHandler}
{...props}
variant="invisible"
data-testid={id}
/>
)
},
)

const ActionBarGroupContext = React.createContext<{
groupId: string | null
}>({groupId: null})

export const ActionBarGroup = forwardRef(({children}: React.PropsWithChildren, forwardedRef) => {
const backupRef = useRef<HTMLDivElement>(null)
const ref = (forwardedRef ?? backupRef) as RefObject<HTMLDivElement>
const id = useId()
const {registerChild, unregisterChild} = React.useContext(ActionBarContext)

// Like IconButton, we store the width in a ref ensures we don't forget about it when not visible
// If a child has a groupId, it won't be visible if the group isn't visible, so we don't need to check isVisibleChild here
const widthRef = useRef<number>()

useIsomorphicLayoutEffect(() => {
const width = ref.current?.getBoundingClientRect().width
if (width) widthRef.current = width
if (!widthRef.current) return

registerChild(id, {type: 'group', width: widthRef.current})

return () => {
unregisterChild(id)
}
}, [registerChild, unregisterChild])

return (
<ActionBarGroupContext.Provider value={{groupId: id}}>
<div className={styles.Group} ref={ref}>
{children}
</div>
</ActionBarGroupContext.Provider>
)
})

export const VerticalDivider = () => {
const ref = useRef<HTMLDivElement>(null)
const id = useId()
Expand Down
3 changes: 2 additions & 1 deletion packages/react/src/ActionBar/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import {ActionBar as Bar, ActionBarIconButton, VerticalDivider} from './ActionBar'
import {ActionBar as Bar, ActionBarIconButton, VerticalDivider, ActionBarGroup} from './ActionBar'
export type {ActionBarProps} from './ActionBar'

const ActionBar = Object.assign(Bar, {
IconButton: ActionBarIconButton,
Divider: VerticalDivider,
Group: ActionBarGroup,
})

export default ActionBar
Expand Down
Loading