Skip to content

Commit 4402b7a

Browse files
pksjcepksjce
authored
Merge branch 'main' into jellobagel-action-bar
2 parents c24d35a + c395547 commit 4402b7a

File tree

4 files changed

+319
-105
lines changed

4 files changed

+319
-105
lines changed

‎.changeset/fair-bars-smile.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': major
3+
---
4+
5+
Support nested children in ActionBar.

‎packages/react/src/ActionBar/ActionBar.stories.tsx‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,33 @@ export const Default = () => (
6363
<ActionBar.IconButton icon={TasklistIcon} aria-label="Task List"></ActionBar.IconButton>
6464
</ActionBar>
6565
)
66+
67+
const BoldButton = () => <ActionBar.IconButton icon={BoldIcon} aria-label="Bold"></ActionBar.IconButton>
68+
69+
const FormattingButtons = () => (
70+
<>
71+
<BoldButton />
72+
<ActionBar.IconButton icon={ItalicIcon} aria-label="Italic"></ActionBar.IconButton>
73+
<ActionBar.IconButton icon={CodeIcon} aria-label="Code"></ActionBar.IconButton>
74+
<ActionBar.IconButton icon={LinkIcon} aria-label="Link"></ActionBar.IconButton>
75+
</>
76+
)
77+
78+
const AdvancedFormattingButtons = () => (
79+
<>
80+
<ActionBar.IconButton icon={FileAddedIcon} aria-label="File Added"></ActionBar.IconButton>
81+
<ActionBar.IconButton icon={SearchIcon} aria-label="Search"></ActionBar.IconButton>
82+
<ActionBar.IconButton icon={QuoteIcon} aria-label="Insert Quote"></ActionBar.IconButton>
83+
<ActionBar.IconButton icon={ListUnorderedIcon} aria-label="Unordered List"></ActionBar.IconButton>
84+
<ActionBar.IconButton icon={ListOrderedIcon} aria-label="Ordered List"></ActionBar.IconButton>
85+
<ActionBar.IconButton icon={TasklistIcon} aria-label="Task List"></ActionBar.IconButton>
86+
</>
87+
)
88+
89+
export const DeepChildTree = () => (
90+
<ActionBar aria-label="Toolbar">
91+
<FormattingButtons />
92+
<ActionBar.Divider />
93+
<AdvancedFormattingButtons />
94+
</ActionBar>
95+
)

‎packages/react/src/ActionBar/ActionBar.test.tsx‎

Lines changed: 158 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import {describe, expect, it, afterEach, vi} from 'vitest'
22
import {render, screen, act} from '@testing-library/react'
33
import userEvent from '@testing-library/user-event'
4-
54
import ActionBar from './'
6-
import {BoldIcon} from '@primer/octicons-react'
5+
import {BoldIcon, ItalicIcon, CodeIcon} from '@primer/octicons-react'
6+
import {useState} from 'react'
77

88
describe('ActionBar', () => {
99
afterEach(() => {
@@ -78,3 +78,159 @@ describe('ActionBar', () => {
7878
expect(onClick).toHaveBeenCalled()
7979
})
8080
})
81+
82+
describe('ActionBar Registry System', () => {
83+
it('should preserve order with deep nesting', () => {
84+
render(
85+
<ActionBar aria-label="Deep test">
86+
<div>
87+
<ActionBar.IconButton icon={BoldIcon} aria-label="First" />
88+
</div>
89+
<ActionBar.IconButton icon={ItalicIcon} aria-label="Second" />
90+
<div>
91+
<ActionBar.IconButton icon={CodeIcon} aria-label="Third" />
92+
</div>
93+
</ActionBar>,
94+
)
95+
96+
const buttons = screen.getAllByRole('button')
97+
expect(buttons).toHaveLength(3)
98+
expect(buttons[0]).toHaveAccessibleName('First')
99+
expect(buttons[1]).toHaveAccessibleName('Second')
100+
expect(buttons[2]).toHaveAccessibleName('Third')
101+
})
102+
103+
it('should handle conditional rendering without breaking order', async () => {
104+
const ConditionalTest = () => {
105+
const [show, setShow] = useState([true, true, true])
106+
107+
return (
108+
<div>
109+
<ActionBar aria-label="Conditional">
110+
{show[0] && <ActionBar.IconButton icon={BoldIcon} aria-label="First" />}
111+
{show[1] && <ActionBar.IconButton icon={ItalicIcon} aria-label="Second" />}
112+
{show[2] && <ActionBar.IconButton icon={CodeIcon} aria-label="Third" />}
113+
</ActionBar>
114+
<button type="button" onClick={() => setShow([false, true, true])}>
115+
Hide first
116+
</button>
117+
<button type="button" onClick={() => setShow([true, true, true])}>
118+
Show all
119+
</button>
120+
</div>
121+
)
122+
}
123+
124+
const user = userEvent.setup()
125+
render(<ConditionalTest />)
126+
127+
// Initially should have 3 buttons
128+
expect(screen.getAllByRole('button', {name: /First|Second|Third/})).toHaveLength(3)
129+
130+
// Hide first button
131+
await user.click(screen.getByText('Hide first'))
132+
133+
const buttonsAfterHide = screen.getAllByRole('button', {name: /Second|Third/})
134+
expect(buttonsAfterHide).toHaveLength(2)
135+
expect(buttonsAfterHide[0]).toHaveAccessibleName('Second')
136+
expect(buttonsAfterHide[1]).toHaveAccessibleName('Third')
137+
138+
// Show first button again
139+
await user.click(screen.getByText('Show all'))
140+
141+
const buttonsAfterShow = screen.getAllByRole('button', {name: /First|Second|Third/})
142+
expect(buttonsAfterShow).toHaveLength(3)
143+
expect(buttonsAfterShow[0]).toHaveAccessibleName('First')
144+
expect(buttonsAfterShow[1]).toHaveAccessibleName('Second')
145+
expect(buttonsAfterShow[2]).toHaveAccessibleName('Third')
146+
})
147+
148+
it('should handle fragments and array mapping', () => {
149+
render(
150+
<ActionBar aria-label="Fragment test">
151+
<>
152+
<ActionBar.IconButton icon={BoldIcon} aria-label="In fragment" />
153+
{[1, 2].map(i => (
154+
<ActionBar.IconButton key={i} icon={ItalicIcon} aria-label={`Mapped ${i}`} />
155+
))}
156+
</>
157+
<ActionBar.IconButton icon={CodeIcon} aria-label="After fragment" />
158+
</ActionBar>,
159+
)
160+
161+
const buttons = screen.getAllByRole('button')
162+
expect(buttons).toHaveLength(4)
163+
expect(buttons[0]).toHaveAccessibleName('In fragment')
164+
expect(buttons[1]).toHaveAccessibleName('Mapped 1')
165+
expect(buttons[2]).toHaveAccessibleName('Mapped 2')
166+
expect(buttons[3]).toHaveAccessibleName('After fragment')
167+
})
168+
169+
it('should handle rapid re-renders without losing registry data', async () => {
170+
const RapidRerenderTest = () => {
171+
const [count, setCount] = useState(0)
172+
173+
return (
174+
<div>
175+
<ActionBar aria-label="Rapid rerender">
176+
<ActionBar.IconButton icon={BoldIcon} aria-label={`Button ${count}`} />
177+
</ActionBar>
178+
<button type="button" onClick={() => setCount(c => c + 1)}>
179+
Increment
180+
</button>
181+
</div>
182+
)
183+
}
184+
185+
const user = userEvent.setup()
186+
render(<RapidRerenderTest />)
187+
188+
// Rapidly trigger re-renders
189+
for (let i = 0; i < 10; i++) {
190+
await user.click(screen.getByText('Increment'))
191+
}
192+
193+
expect(screen.getByRole('button', {name: 'Button 10'})).toBeInTheDocument()
194+
})
195+
196+
it('should handle zero-width scenarios gracefully', () => {
197+
render(
198+
<div style={{width: 0, overflow: 'hidden'}}>
199+
<ActionBar aria-label="Zero width">
200+
<ActionBar.IconButton icon={BoldIcon} aria-label="Zero width button" />
201+
</ActionBar>
202+
</div>,
203+
)
204+
205+
// Component should still render even with zero width
206+
expect(screen.getByRole('button', {name: 'Zero width button'})).toBeInTheDocument()
207+
})
208+
209+
it('should clean up registry on unmount', async () => {
210+
const UnmountTest = () => {
211+
const [mounted, setMounted] = useState(true)
212+
213+
return (
214+
<div>
215+
{mounted && (
216+
<ActionBar aria-label="Unmount test">
217+
<ActionBar.IconButton icon={BoldIcon} aria-label="Will unmount" />
218+
</ActionBar>
219+
)}
220+
<button type="button" onClick={() => setMounted(false)}>
221+
Unmount
222+
</button>
223+
</div>
224+
)
225+
}
226+
227+
const user = userEvent.setup()
228+
render(<UnmountTest />)
229+
230+
expect(screen.getByRole('button', {name: 'Will unmount'})).toBeInTheDocument()
231+
232+
await user.click(screen.getByText('Unmount'))
233+
234+
expect(screen.queryByRole('button', {name: 'Will unmount'})).not.toBeInTheDocument()
235+
})
236+
})

0 commit comments

Comments
 (0)