Skip to content

Commit 8babf1a

Browse files
cee-chencee-chen
committed
Fix focus being lost while transitioning between panels
- updateFocus() was being called and focus being blurred between panel transitions. If users were pressing the left/right arrow keys very quickly on the EUI documentation site, this would lead to accidental page navigation which can be quite annoying - It looks like the `hasFocus` prop simply isn't necessary and is already adequately handled by the transition props, hence the complete removal - the added `.focus()` line on 227 keeps focus on the primary panel between transitions instead of stranding focus on removed panels - preventScroll prevents scroll jumping between panels
1 parent aa7e893 commit 8babf1a

5 files changed

Lines changed: 65 additions & 32 deletions

File tree

‎src/components/basic_table/__snapshots__/collapsed_item_actions.test.tsx.snap‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ exports[`CollapsedItemActions render with href and _target provided 1`] = `
5050
popoverRef={[Function]}
5151
>
5252
<EuiContextMenuPanel
53-
hasFocus={true}
5453
items={
5554
Array [
5655
<EuiContextMenuItem

‎src/components/context_menu/__snapshots__/context_menu_panel.test.tsx.snap‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ exports[`EuiContextMenuPanel props transitionDirection previous with transitionT
559559
`;
560560

561561
exports[`EuiContextMenuPanel updating items and content updates to items should not re-render if any items's watchedItemProps did not change 1`] = `
562-
"<EuiContextMenuPanel items={{...}} hasFocus={true}>
562+
"<EuiContextMenuPanel items={{...}}>
563563
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex={-1} onAnimationEnd={[Function]}>
564564
<div>
565565
<EuiResizeObserver onResize={[Function: onResize]}>
@@ -590,7 +590,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
590590
`;
591591
592592
exports[`EuiContextMenuPanel updating items and content updates to items should not re-render if any items's watchedItemProps did not change 2`] = `
593-
"<EuiContextMenuPanel items={{...}} hasFocus={true}>
593+
"<EuiContextMenuPanel items={{...}}>
594594
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex={-1} onAnimationEnd={[Function]}>
595595
<div>
596596
<EuiResizeObserver onResize={[Function: onResize]}>
@@ -621,7 +621,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
621621
`;
622622
623623
exports[`EuiContextMenuPanel updating items and content updates to items should re-render at all times when children exists 1`] = `
624-
"<EuiContextMenuPanel hasFocus={true} items={{...}}>
624+
"<EuiContextMenuPanel items={{...}}>
625625
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex={-1} onAnimationEnd={[Function]}>
626626
<div>
627627
<EuiResizeObserver onResize={[Function: onResize]}>
@@ -635,7 +635,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
635635
`;
636636
637637
exports[`EuiContextMenuPanel updating items and content updates to items should re-render at all times when children exists 2`] = `
638-
"<EuiContextMenuPanel hasFocus={true} items={{...}}>
638+
"<EuiContextMenuPanel items={{...}}>
639639
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex={-1} onAnimationEnd={[Function]}>
640640
<div>
641641
<EuiResizeObserver onResize={[Function: onResize]}>
@@ -649,7 +649,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
649649
`;
650650
651651
exports[`EuiContextMenuPanel updating items and content updates to items should re-render if any items's watchedItemProps did change 1`] = `
652-
"<EuiContextMenuPanel watchedItemProps={{...}} items={{...}} hasFocus={true}>
652+
"<EuiContextMenuPanel watchedItemProps={{...}} items={{...}}>
653653
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex={-1} onAnimationEnd={[Function]}>
654654
<div>
655655
<EuiResizeObserver onResize={[Function: onResize]}>
@@ -680,7 +680,7 @@ exports[`EuiContextMenuPanel updating items and content updates to items should
680680
`;
681681
682682
exports[`EuiContextMenuPanel updating items and content updates to items should re-render if any items's watchedItemProps did change 2`] = `
683-
"<EuiContextMenuPanel watchedItemProps={{...}} items={{...}} hasFocus={true}>
683+
"<EuiContextMenuPanel watchedItemProps={{...}} items={{...}}>
684684
<div className=\\"euiContextMenuPanel\\" onKeyDown={[Function]} tabIndex={-1} onAnimationEnd={[Function]}>
685685
<div>
686686
<EuiResizeObserver onResize={[Function: onResize]}>

‎src/components/context_menu/context_menu.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,6 @@ export class EuiContextMenu extends Component<EuiContextMenuProps, State> {
392392
? this.state.transitionDirection
393393
: undefined
394394
}
395-
hasFocus={transitionType === 'in'}
396395
items={this.state.idToRenderedItemsMap[panelId]}
397396
initialFocusedItemIndex={
398397
this.state.isUsingKeyboardToNavigate

‎src/components/context_menu/context_menu_panel.spec.tsx‎

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import React from 'react';
1212

13+
import { EuiContextMenu } from './context_menu';
1314
import { EuiContextMenuItem } from './context_menu_item';
1415
import { EuiContextMenuPanel } from './context_menu_panel';
1516

@@ -35,15 +36,6 @@ describe('EuiContextMenuPanel', () => {
3536
);
3637
cy.focused().should('have.attr', 'data-test-subj', 'button');
3738
});
38-
39-
it('is not set on anything if hasFocus is false', () => {
40-
cy.mount(
41-
<EuiContextMenuPanel hasFocus={false}>
42-
<button data-test-subj="button">Hello world</button>
43-
</EuiContextMenuPanel>
44-
);
45-
cy.focused().should('not.exist');
46-
});
4739
});
4840

4941
describe('Keyboard navigation of items', () => {
@@ -115,6 +107,60 @@ describe('EuiContextMenuPanel', () => {
115107
expect(showPreviousPanelHandler).to.be.called;
116108
});
117109
});
110+
111+
it('does not lose focus while using left/right arrow navigation between panels', () => {
112+
const panels = [
113+
{
114+
id: 0,
115+
title: 'First panel',
116+
items: [
117+
{
118+
name: 'Go to second panel',
119+
panel: 1,
120+
'data-test-subj': 'itemA',
121+
},
122+
],
123+
},
124+
{
125+
id: 1,
126+
title: 'Second panel',
127+
items: [
128+
{
129+
name: 'Go to third panel',
130+
panel: 2,
131+
'data-test-subj': 'itemB',
132+
},
133+
],
134+
initialFocusedItemIndex: 0,
135+
},
136+
{
137+
id: 2,
138+
title: 'Third panel',
139+
items: [
140+
{
141+
name: 'End',
142+
'data-test-subj': 'itemC',
143+
},
144+
],
145+
initialFocusedItemIndex: 0,
146+
},
147+
];
148+
cy.mount(<EuiContextMenu panels={panels} initialPanelId={0} />);
149+
cy.realPress('{downarrow}');
150+
cy.focused().should('have.attr', 'data-test-subj', 'itemA');
151+
cy.realPress('{rightarrow}');
152+
cy.focused().should('have.attr', 'data-test-subj', 'itemB');
153+
cy.realPress('{rightarrow}');
154+
cy.focused().should('have.attr', 'data-test-subj', 'itemC');
155+
156+
// Test extremely rapid left/right arrow usage
157+
cy.repeatRealPress('{leftarrow}');
158+
cy.focused().should('have.attr', 'data-test-subj', 'itemA');
159+
cy.repeatRealPress('{rightarrow}');
160+
cy.focused().should('have.attr', 'data-test-subj', 'itemC');
161+
cy.repeatRealPress('{leftarrow}');
162+
cy.focused().should('have.attr', 'data-test-subj', 'itemA');
163+
});
118164
});
119165

120166
describe('tab key', () => {

‎src/components/context_menu/context_menu_panel.tsx‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ const titleSizeToClassNameMap = {
4040
export const SIZES = keysOf(titleSizeToClassNameMap);
4141

4242
export interface EuiContextMenuPanelProps {
43-
hasFocus?: boolean;
4443
initialFocusedItemIndex?: number;
4544
items?: ReactElement[];
4645
onClose?: NoArgCallback<void>;
@@ -89,7 +88,6 @@ interface State {
8988

9089
export class EuiContextMenuPanel extends Component<Props, State> {
9190
static defaultProps: Partial<Props> = {
92-
hasFocus: true,
9391
items: [],
9492
};
9593

@@ -220,17 +218,13 @@ export class EuiContextMenuPanel extends Component<Props, State> {
220218
return;
221219
}
222220

223-
// If this panel has lost focus, then none of its content should be focused.
224-
if (!this.props.hasFocus) {
225-
if (this.panel && this.panel.contains(document.activeElement)) {
226-
(document.activeElement as HTMLElement).blur();
227-
}
228-
return;
229-
}
230-
231221
// Setting focus while transitioning causes the animation to glitch, so we have to wait
232222
// until it's finished before we focus anything.
233223
if (this.props.transitionType) {
224+
// If the panel is transitioning, set focus to the panel so that users using
225+
// arrow keys that are fast clickers don't accidentally get stranded focus
226+
// or trigger keystrokes when it shouldn't
227+
this.panel?.focus({ preventScroll: true });
234228
return;
235229
}
236230

@@ -348,10 +342,6 @@ export class EuiContextMenuPanel extends Component<Props, State> {
348342

349343
shouldComponentUpdate(nextProps: Props, nextState: State) {
350344
// Prevent calling `this.updateFocus()` below if we don't have to.
351-
if (nextProps.hasFocus !== this.props.hasFocus) {
352-
return true;
353-
}
354-
355345
if (nextProps.transitionType !== this.props.transitionType) {
356346
return true;
357347
}
@@ -429,7 +419,6 @@ export class EuiContextMenuPanel extends Component<Props, State> {
429419
transitionDirection,
430420
onTransitionComplete,
431421
onUseKeyboardToNavigate,
432-
hasFocus,
433422
items,
434423
watchedItemProps,
435424
initialFocusedItemIndex,

0 commit comments

Comments
 (0)