Skip to content

Commit 512c975

Browse files
ahorowitz123ahorowitz123
authored
fix(makeselectable): disable arrow navigation when in overlay (#3866)
* fix(makeselectable): disable arrow navigation when in overlay * fix(makeselectable): disable arrow navigation when in overlay
1 parent 5de13ca commit 512c975

File tree

2 files changed

+94
-6
lines changed

2 files changed

+94
-6
lines changed

‎src/components/table/__tests__/makeSelectable.test.js‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,22 @@ describe('components/table/makeSelectable', () => {
2222
const getWrapper = (props = {}) =>
2323
shallow(<SelectableTable onSelect={sandbox.stub()} data={data} selectedItems={[]} enableHotkeys {...props} />);
2424

25+
const testClassNamePreventsArrowNavigation = (className, hotKey, isGridView = false) => {
26+
const wrapper = getWrapper({
27+
gridColumnCount: 3,
28+
isGridView,
29+
selectedItems: ['a'],
30+
});
31+
32+
jest.spyOn(document, 'querySelector').mockImplementation(selector => className === selector);
33+
34+
wrapper.setState({ focusedIndex: undefined });
35+
const instance = wrapper.instance();
36+
const shortcut = instance.getHotkeyConfigs().find(h => h.get('key') === hotKey);
37+
shortcut.handler({ preventDefault: sandbox.stub() });
38+
expect(wrapper.state('focusedIndex')).toEqual(undefined);
39+
};
40+
2541
afterEach(() => {
2642
jest.clearAllTimers();
2743
jest.clearAllMocks();
@@ -677,6 +693,7 @@ describe('components/table/makeSelectable', () => {
677693
describe('ListView specific', () => {
678694
describe('down', () => {
679695
const hotKey = 'down';
696+
680697
test('should set focus to first row when no currently focused item', () => {
681698
const wrapper = getWrapper({
682699
selectedItems: ['a'],
@@ -687,6 +704,7 @@ describe('components/table/makeSelectable', () => {
687704
shortcut.handler({ preventDefault: sandbox.stub() });
688705
expect(wrapper.state('focusedIndex')).toEqual(0);
689706
});
707+
690708
test('should call event.preventDefault() and set focus to next item', () => {
691709
const wrapper = getWrapper({
692710
selectedItems: ['a'],
@@ -697,6 +715,7 @@ describe('components/table/makeSelectable', () => {
697715
shortcut.handler({ preventDefault: sandbox.mock() });
698716
expect(wrapper.state('focusedIndex')).toEqual(1);
699717
});
718+
700719
test('should not focus on an index higher than the highest index in the table', () => {
701720
const wrapper = getWrapper({
702721
selectedItems: ['a'],
@@ -707,9 +726,18 @@ describe('components/table/makeSelectable', () => {
707726
shortcut.handler({ preventDefault: sandbox.stub() });
708727
expect(wrapper.state('focusedIndex')).toEqual(4);
709728
});
729+
730+
test.each([['flyout-overlay'], ['dropdown-menu-element']])(
731+
'should not set focus if element with class %s is rendered',
732+
className => {
733+
testClassNamePreventsArrowNavigation(className, hotKey);
734+
},
735+
);
710736
});
737+
711738
describe('up', () => {
712739
const hotKey = 'up';
740+
713741
test('should call event.preventDefault() and call onSelect with new focused item', () => {
714742
const wrapper = getWrapper({
715743
selectedItems: ['a'],
@@ -720,6 +748,7 @@ describe('components/table/makeSelectable', () => {
720748
shortcut.handler({ preventDefault: sandbox.mock() });
721749
expect(wrapper.state('focusedIndex')).toEqual(0);
722750
});
751+
723752
test('should not focus on an index lower than 0', () => {
724753
const wrapper = getWrapper({
725754
selectedItems: ['a'],
@@ -730,6 +759,13 @@ describe('components/table/makeSelectable', () => {
730759
shortcut.handler({ preventDefault: sandbox.stub() });
731760
expect(wrapper.state('focusedIndex')).toEqual(0);
732761
});
762+
763+
test.each([['flyout-overlay'], ['dropdown-menu-element']])(
764+
'should not set focus if element with class %s is rendered',
765+
className => {
766+
testClassNamePreventsArrowNavigation(className, hotKey);
767+
},
768+
);
733769
});
734770

735771
describe('shift+down', () => {
@@ -858,6 +894,7 @@ describe('components/table/makeSelectable', () => {
858894

859895
describe('right', () => {
860896
const hotKey = 'right';
897+
861898
test('should set focus to first row when no currently focused item', () => {
862899
const wrapper = getWrapper({
863900
gridColumnCount,
@@ -870,6 +907,7 @@ describe('components/table/makeSelectable', () => {
870907
shortcut.handler({ preventDefault: sandbox.stub() });
871908
expect(wrapper.state('focusedIndex')).toEqual(0);
872909
});
910+
873911
test('should not set focus to first row if target has role of slider', () => {
874912
const wrapper = getWrapper({
875913
gridColumnCount,
@@ -895,6 +933,7 @@ describe('components/table/makeSelectable', () => {
895933
shortcut.handler({ preventDefault: sandbox.mock() });
896934
expect(wrapper.state('focusedIndex')).toEqual(1);
897935
});
936+
898937
test('should not focus on an index higher than the highest index in the table', () => {
899938
const wrapper = getWrapper({
900939
gridColumnCount,
@@ -907,9 +946,18 @@ describe('components/table/makeSelectable', () => {
907946
shortcut.handler({ preventDefault: sandbox.stub() });
908947
expect(wrapper.state('focusedIndex')).toEqual(4);
909948
});
949+
950+
test.each([['flyout-overlay'], ['dropdown-menu-element']])(
951+
'should not set focus if element with class %s is rendered',
952+
className => {
953+
testClassNamePreventsArrowNavigation(className, hotKey, true);
954+
},
955+
);
910956
});
957+
911958
describe('left', () => {
912959
const hotKey = 'left';
960+
913961
test('should not set focus to first row if target has role of slider', () => {
914962
const wrapper = getWrapper({
915963
gridColumnCount,
@@ -922,6 +970,7 @@ describe('components/table/makeSelectable', () => {
922970
shortcut.handler({ target: { role: 'slider' } });
923971
expect(wrapper.state('focusedIndex')).toEqual(undefined);
924972
});
973+
925974
test('should call event.preventDefault() and call onSelect with new focused item', () => {
926975
const wrapper = getWrapper({
927976
gridColumnCount,
@@ -934,6 +983,7 @@ describe('components/table/makeSelectable', () => {
934983
shortcut.handler({ preventDefault: sandbox.mock() });
935984
expect(wrapper.state('focusedIndex')).toEqual(0);
936985
});
986+
937987
test('should call event.preventDefault() and set focus to previous item', () => {
938988
const wrapper = getWrapper({
939989
gridColumnCount,
@@ -946,6 +996,7 @@ describe('components/table/makeSelectable', () => {
946996
shortcut.handler({ preventDefault: sandbox.mock() });
947997
expect(wrapper.state('focusedIndex')).toEqual(0);
948998
});
999+
9491000
test('should not focus on an index lower than 0', () => {
9501001
const wrapper = getWrapper({
9511002
gridColumnCount,
@@ -958,9 +1009,17 @@ describe('components/table/makeSelectable', () => {
9581009
shortcut.handler({ preventDefault: sandbox.stub() });
9591010
expect(wrapper.state('focusedIndex')).toEqual(0);
9601011
});
1012+
1013+
test.each([['flyout-overlay'], ['dropdown-menu-element']])(
1014+
'should not set focus if element with class %s is rendered',
1015+
className => {
1016+
testClassNamePreventsArrowNavigation(className, hotKey, true);
1017+
},
1018+
);
9611019
});
9621020
describe('down', () => {
9631021
const hotKey = 'down';
1022+
9641023
test('should set focus to first row when no currently focused item', () => {
9651024
const wrapper = getWrapper({
9661025
gridColumnCount,
@@ -973,6 +1032,7 @@ describe('components/table/makeSelectable', () => {
9731032
shortcut.handler({ preventDefault: sandbox.stub() });
9741033
expect(wrapper.state('focusedIndex')).toEqual(0);
9751034
});
1035+
9761036
test('should not set focus to first row if target has role of slider', () => {
9771037
const wrapper = getWrapper({
9781038
gridColumnCount,
@@ -985,6 +1045,7 @@ describe('components/table/makeSelectable', () => {
9851045
shortcut.handler({ target: { role: 'slider' } });
9861046
expect(wrapper.state('focusedIndex')).toEqual(undefined);
9871047
});
1048+
9881049
test('should call event.preventDefault() and set focus to next row item', () => {
9891050
const wrapper = getWrapper({
9901051
gridColumnCount,
@@ -997,6 +1058,7 @@ describe('components/table/makeSelectable', () => {
9971058
shortcut.handler({ preventDefault: sandbox.mock() });
9981059
expect(wrapper.state('focusedIndex')).toEqual(gridColumnCount);
9991060
});
1061+
10001062
test('should not focus on an index higher than the highest index in the table', () => {
10011063
const wrapper = getWrapper({
10021064
gridColumnCount,
@@ -1009,9 +1071,18 @@ describe('components/table/makeSelectable', () => {
10091071
shortcut.handler({ preventDefault: sandbox.stub() });
10101072
expect(wrapper.state('focusedIndex')).toEqual(4);
10111073
});
1074+
1075+
test.each([['flyout-overlay'], ['dropdown-menu-element']])(
1076+
'should not set focus if element with class %s is rendered',
1077+
className => {
1078+
testClassNamePreventsArrowNavigation(className, hotKey, true);
1079+
},
1080+
);
10121081
});
1082+
10131083
describe('up', () => {
10141084
const hotKey = 'up';
1085+
10151086
test('should not set focus to first row if target has role of slider', () => {
10161087
const wrapper = getWrapper({
10171088
gridColumnCount,
@@ -1024,6 +1095,7 @@ describe('components/table/makeSelectable', () => {
10241095
shortcut.handler({ target: { role: 'slider' } });
10251096
expect(wrapper.state('focusedIndex')).toEqual(undefined);
10261097
});
1098+
10271099
test('should call event.preventDefault() and call onSelect with new focused item', () => {
10281100
const wrapper = getWrapper({
10291101
gridColumnCount,
@@ -1036,6 +1108,7 @@ describe('components/table/makeSelectable', () => {
10361108
shortcut.handler({ preventDefault: sandbox.mock() });
10371109
expect(wrapper.state('focusedIndex')).toEqual(0);
10381110
});
1111+
10391112
test('should call event.preventDefault() and set focus to previous row item', () => {
10401113
const wrapper = getWrapper({
10411114
gridColumnCount,
@@ -1048,6 +1121,7 @@ describe('components/table/makeSelectable', () => {
10481121
shortcut.handler({ preventDefault: sandbox.mock() });
10491122
expect(wrapper.state('focusedIndex')).toEqual(0);
10501123
});
1124+
10511125
test('should not focus on an index lower than 0', () => {
10521126
const wrapper = getWrapper({
10531127
gridColumnCount,
@@ -1060,6 +1134,13 @@ describe('components/table/makeSelectable', () => {
10601134
shortcut.handler({ preventDefault: sandbox.stub() });
10611135
expect(wrapper.state('focusedIndex')).toEqual(0);
10621136
});
1137+
1138+
test.each([['flyout-overlay'], ['dropdown-menu-element']])(
1139+
'should not set focus if element with class %s is rendered',
1140+
className => {
1141+
testClassNamePreventsArrowNavigation(className, hotKey, true);
1142+
},
1143+
);
10631144
});
10641145

10651146
describe('shift+right', () => {

‎src/components/table/makeSelectable.js‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ function makeSelectable(BaseTable) {
148148
key: 'down',
149149
description: <FormattedMessage {...messages.downDescription} />,
150150
handler: event => {
151-
if (this.isTargetQuickSearch(event)) {
151+
if (this.shouldNotAllowArrowKeyNavigation(event)) {
152152
return;
153153
}
154154

@@ -167,7 +167,7 @@ function makeSelectable(BaseTable) {
167167
key: 'up',
168168
description: <FormattedMessage {...messages.upDescription} />,
169169
handler: event => {
170-
if (this.isTargetQuickSearch(event)) {
170+
if (this.shouldNotAllowArrowKeyNavigation(event)) {
171171
return;
172172
}
173173

@@ -221,7 +221,7 @@ function makeSelectable(BaseTable) {
221221
key: 'right',
222222
description: <FormattedMessage {...messages.downDescription} />,
223223
handler: event => {
224-
if (this.isTargetSlider(event) || this.isTargetQuickSearch(event)) {
224+
if (this.isTargetSlider(event) || this.shouldNotAllowArrowKeyNavigation(event)) {
225225
return;
226226
}
227227

@@ -240,7 +240,7 @@ function makeSelectable(BaseTable) {
240240
key: 'left',
241241
description: <FormattedMessage {...messages.upDescription} />,
242242
handler: event => {
243-
if (this.isTargetSlider(event) || this.isTargetQuickSearch(event)) {
243+
if (this.isTargetSlider(event) || this.shouldNotAllowArrowKeyNavigation(event)) {
244244
return;
245245
}
246246

@@ -257,7 +257,7 @@ function makeSelectable(BaseTable) {
257257
key: 'down',
258258
description: <FormattedMessage {...messages.downDescription} />,
259259
handler: event => {
260-
if (this.isTargetSlider(event) || this.isTargetQuickSearch(event)) {
260+
if (this.isTargetSlider(event) || this.shouldNotAllowArrowKeyNavigation(event)) {
261261
return;
262262
}
263263

@@ -276,7 +276,7 @@ function makeSelectable(BaseTable) {
276276
key: 'up',
277277
description: <FormattedMessage {...messages.upDescription} />,
278278
handler: event => {
279-
if (this.isTargetSlider(event) || this.isTargetQuickSearch(event)) {
279+
if (this.isTargetSlider(event) || this.shouldNotAllowArrowKeyNavigation(event)) {
280280
return;
281281
}
282282

@@ -644,6 +644,13 @@ function makeSelectable(BaseTable) {
644644
return false;
645645
};
646646

647+
isFlyoutOpen = () => document.querySelector('.flyout-overlay') !== null;
648+
649+
isDropdownMenuOpen = () => document.querySelector('.dropdown-menu-element') !== null;
650+
651+
shouldNotAllowArrowKeyNavigation = event =>
652+
this.isTargetQuickSearch(event) || this.isFlyoutOpen() || this.isDropdownMenuOpen();
653+
647654
render() {
648655
const { className, data } = this.props;
649656
const { focusedIndex } = this.state;

0 commit comments

Comments
 (0)