Skip to content

Commit cf79941

Browse files
authored
Merge branch 'main' into select-panel-active-descendant
2 parents c94f6f6 + fb2fa6c commit cf79941

File tree

9 files changed

+29
-16
lines changed

9 files changed

+29
-16
lines changed

.changeset/lucky-ducks-camp.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@primer/components": patch
3+
---
4+
5+
Use `Item` `index` as fallback key within `List` groups

.changeset/nine-boxes-change.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@primer/components": patch
3+
---
4+
5+
Allow focus to move to an input outside an overlay on mousedown

src/ActionList/List.tsx

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import React, {useMemo} from 'react'
1+
import React from 'react'
22
import type {AriaRole} from '../utils/types'
33
import {Group, GroupProps} from './Group'
44
import {Item, ItemProps} from './Item'
55
import {Divider} from './Divider'
66
import styled from 'styled-components'
77
import {get} from '../constants'
88
import {SystemCssProperties} from '@styled-system/css'
9-
import {uniqueId} from '../utils/uniqueId'
109

1110
export type ItemInput = ItemProps | (Partial<ItemProps> & {renderItem: typeof Item})
1211

@@ -156,10 +155,10 @@ export function List(props: ListProps): JSX.Element {
156155
* An `Item`-level, `Group`-level, or `List`-level custom `Item` renderer,
157156
* or the default `Item` renderer.
158157
*/
159-
const renderItem = (itemProps: ItemInput, item: ItemInput) => {
158+
const renderItem = (itemProps: ItemInput, item: ItemInput, itemIndex: number) => {
160159
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
161160
const ItemComponent = ('renderItem' in itemProps && itemProps.renderItem) || props.renderItem || Item
162-
const key = itemProps.key ?? itemProps.id?.toString() ?? uniqueId()
161+
const key = itemProps.key ?? itemProps.id?.toString() ?? itemIndex.toString()
163162
return (
164163
<ItemComponent
165164
showDivider={props.showItemDividers}
@@ -178,11 +177,9 @@ export function List(props: ListProps): JSX.Element {
178177
let groups: (GroupProps | (Partial<GroupProps> & {renderItem?: typeof Item; renderGroup?: typeof Group}))[] = []
179178

180179
// Collect rendered `Item`s into `Group`s, avoiding excess iteration over the lists of `items` and `groupMetadata`:
181-
const singleGroupId = useMemo(uniqueId, [])
182-
183180
if (!isGroupedListProps(props)) {
184181
// When no `groupMetadata`s is provided, collect rendered `Item`s into a single anonymous `Group`.
185-
groups = [{items: props.items.map(item => renderItem(item, item)), groupId: singleGroupId}]
182+
groups = [{items: props.items.map((item, index) => renderItem(item, item, index)), groupId: '0'}]
186183
} else {
187184
// When `groupMetadata` is provided, collect rendered `Item`s into their associated `Group`s.
188185

@@ -197,6 +194,7 @@ export function List(props: ListProps): JSX.Element {
197194
for (const itemProps of props.items) {
198195
// Look up the group associated with the current item.
199196
const group = groupMap.get(itemProps.groupId)
197+
const itemIndex = group?.items?.length ?? 0
200198

201199
// Upsert the group to include the current item (rendered).
202200
groupMap.set(itemProps.groupId, {
@@ -209,7 +207,8 @@ export function List(props: ListProps): JSX.Element {
209207
...(group && 'renderItem' in group && {renderItem: group.renderItem}),
210208
...itemProps
211209
},
212-
itemProps
210+
itemProps,
211+
itemIndex
213212
)
214213
]
215214
})

src/__tests__/ActionMenu.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ describe('ActionMenu', () => {
104104
expect(portalRoot).toBeTruthy()
105105
const somethingElse = (await menu.baseElement.querySelector('#something-else')) as HTMLElement
106106
act(() => {
107-
fireEvent.click(somethingElse)
107+
fireEvent.mouseDown(somethingElse)
108108
})
109109
expect(portalRoot?.textContent).toEqual('') // menu items are hidden
110110
})

src/__tests__/AnchoredOverlay.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ describe('AnchoredOverlay', () => {
110110
onCloseCallback={mockCloseCallback}
111111
/>
112112
)
113-
fireEvent.click(anchoredOverlay.baseElement)
113+
fireEvent.mouseDown(anchoredOverlay.baseElement)
114114

115115
expect(mockOpenCallback).toHaveBeenCalledTimes(0)
116116
expect(mockCloseCallback).toHaveBeenCalledTimes(1)

src/__tests__/DropdownMenu.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ describe('DropdownMenu', () => {
124124
expect(portalRoot).toBeTruthy()
125125
const somethingElse = (await menu.baseElement.querySelector('#something-else')) as HTMLElement
126126
act(() => {
127-
fireEvent.click(somethingElse)
127+
fireEvent.mouseDown(somethingElse)
128128
})
129129
// portal is closed after click
130130
expect(portalRoot?.textContent).toEqual('') // menu items are hidden

src/behaviors/focusTrap.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,12 @@ export function focusTrap(
142142

143143
// Prevent focus leaving the trap container
144144
document.addEventListener(
145-
'focusin',
146-
(event: FocusEvent) => {
145+
'focus',
146+
event => {
147147
ensureTrapZoneHasFocus(event.target)
148148
},
149-
{signal: wrappingController.signal}
149+
// use capture to ensure we get all events. focus events do not bubble
150+
{signal: wrappingController.signal, capture: true}
150151
)
151152

152153
// focus the first element

src/hooks/useOnOutsideClick.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@ export const useOnOutsideClick = ({containerRef, ignoreClickRefs, onClickOutside
4949
)
5050

5151
useEffect(() => {
52-
document.addEventListener('click', onOutsideClickInternal)
52+
// use capture to ensure we get all events
53+
document.addEventListener('mousedown', onOutsideClickInternal, {capture: true})
5354
return () => {
54-
document.removeEventListener('click', onOutsideClickInternal)
55+
document.removeEventListener('mousedown', onOutsideClickInternal, {capture: true})
5556
}
5657
}, [onOutsideClickInternal])
5758
}

src/stories/DropdownMenu.stories.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {theme, ThemeProvider} from '..'
44
import {ItemInput} from '../ActionList/List'
55
import BaseStyles from '../BaseStyles'
66
import {DropdownMenu, DropdownButton} from '../DropdownMenu'
7+
import TextInput from '../TextInput'
78

89
const meta: Meta = {
910
title: 'Composite components/DropdownMenu',
@@ -34,6 +35,7 @@ export function FavoriteColorStory(): JSX.Element {
3435
return (
3536
<>
3637
<h1>Favorite Color</h1>
38+
<TextInput placeholder="Input for focus testing" mb={2} />
3739
<div id="favorite-color-label">Please select your favorite color:</div>
3840
<DropdownMenu
3941
renderAnchor={({children, 'aria-labelledby': ariaLabelledBy, ...anchorProps}) => (

0 commit comments

Comments
 (0)