Skip to content

Commit 69d513c

Browse files
authored
fix(dropdowns): ensure focus is returned to menu trigger before calling onChange (#1930)
1 parent 8193d29 commit 69d513c

File tree

6 files changed

+44
-19
lines changed

6 files changed

+44
-19
lines changed

docs/migration.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,12 @@ consider additional positioning prop support on a case-by-case basis.
118118
- Use this package if you were using `@zendeskgarden/react-dropdowns.next` in `v8`
119119
- The `v8` version of `@zendeskgarden/react-dropdowns` is no longer maintained and is
120120
renamed to `@zendeskgarden/react-dropdowns.legacy` in `v9`
121-
- `Menu`: value `auto` is no longer valid for the `fallbackPlacements` prop.
121+
- `Menu`
122+
- value `auto` is no longer valid for the `fallbackPlacements` prop.
123+
- new `restoreFocus` prop (default: `true`) returns focus to trigger
124+
after menu interaction. When menu expansion is controlled to allow
125+
multiple item selection, set `restoreFocus={false}` and manage trigger
126+
focus manually on close.
122127
- Removed `label` prop from `OptGroup`. Use `legend` instead.
123128

124129
#### @zendeskgarden/react-forms

package-lock.json

Lines changed: 13 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/dropdowns/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
"dependencies": {
2424
"@floating-ui/react-dom": "^2.0.0",
2525
"@zendeskgarden/container-combobox": "^2.0.0",
26-
"@zendeskgarden/container-menu": "^0.4.0",
26+
"@zendeskgarden/container-menu": "^0.5.0",
2727
"@zendeskgarden/container-utilities": "^2.0.0",
2828
"@zendeskgarden/react-buttons": "^9.0.0-next.26",
2929
"@zendeskgarden/react-forms": "^9.0.0-next.26",

packages/dropdowns/src/elements/menu/Menu.spec.tsx

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,9 @@ describe('Menu', () => {
132132
expect(item).toBeVisible();
133133
});
134134

135-
it('applies `defaultFocusedValue`', async () => {
136-
const { getByTestId } = render(
137-
<TestMenu defaultExpanded defaultFocusedValue="Cactus">
135+
it('applies `defaultFocusedValue` after a keyboard event opens the menu', async () => {
136+
const { getByTestId, getByRole } = render(
137+
<TestMenu defaultFocusedValue="Cactus">
138138
<Item value="Flower" data-test-id="item-01">
139139
Flower
140140
<Item.Meta>Smooth</Item.Meta>
@@ -148,8 +148,22 @@ describe('Menu', () => {
148148
);
149149

150150
await floating();
151+
const trigger = getByRole('button');
152+
153+
// open menu with onClick
154+
await user.click(trigger);
151155
const item = getByTestId('item-02');
156+
// focus remains on trigger with mouseEvents
157+
expect(trigger).toHaveFocus();
152158

159+
// close menu
160+
await user.click(trigger);
161+
expect(item).not.toBeVisible();
162+
163+
// open menu with keyboard
164+
trigger.focus();
165+
await user.keyboard(' ');
166+
// focus is on the focused item associated with `defaultFocusedValue`
153167
expect(item).toHaveFocus();
154168
});
155169

@@ -603,14 +617,15 @@ describe('Menu', () => {
603617
});
604618

605619
it('calls onChange as expected', async () => {
606-
const { getByTestId } = render(
620+
const { getByRole, getByTestId } = render(
607621
<TestMenu isExpanded focusedValue="Cactus" onChange={handleChange}>
608622
<Item value="Flower" data-test-id="flower" />
609623
<Item value="Cactus" />
610624
</TestMenu>
611625
);
612626

613627
await floating();
628+
const trigger = getByRole('button');
614629
const item1 = getByTestId('flower');
615630

616631
await act(async () => {
@@ -620,6 +635,7 @@ describe('Menu', () => {
620635
const changeTypes = handleChange.mock.calls.map(([change]) => change.type);
621636

622637
expect(changeTypes).toMatchObject(['menuItem:mouseMove', 'menuItem:click']);
638+
expect(trigger).toHaveFocus();
623639
});
624640

625641
it('handles `focusedValue` and `isExpanded` as expected', async () => {

packages/dropdowns/src/elements/menu/Menu.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export const Menu = forwardRef<HTMLUListElement, IMenuProps>(
3232
defaultFocusedValue,
3333
defaultExpanded,
3434
isExpanded: _isExpanded,
35+
restoreFocus,
3536
selectedItems,
3637
onChange,
3738
onMouseLeave,
@@ -61,6 +62,7 @@ export const Menu = forwardRef<HTMLUListElement, IMenuProps>(
6162
focusedValue: _focusedValue,
6263
defaultExpanded,
6364
isExpanded: _isExpanded,
65+
restoreFocus,
6466
selectedItems,
6567
items,
6668
menuRef,

packages/dropdowns/src/types/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,8 @@ export interface IMenuProps extends HTMLAttributes<HTMLUListElement> {
269269
* @param {string | null} [changes.focusedValue] The updated focused value
270270
*/
271271
onChange?: IUseMenuProps['onChange'];
272+
/** Returns keyboard focus to the element that triggered the menu */
273+
restoreFocus?: IUseMenuProps['restoreFocus'];
272274
/** Sets the selected items in a controlled menu */
273275
selectedItems?: IUseMenuProps['selectedItems'];
274276
/** Adjusts the placement of the menu */

0 commit comments

Comments
 (0)