Skip to content

Commit de33718

Browse files
authored
simplify popover open state logic (#85379) (#85408)
1 parent 3f53ab8 commit de33718

File tree

4 files changed

+29
-42
lines changed

4 files changed

+29
-42
lines changed

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filter_popover.test.tsx

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66

77
import React, { MouseEventHandler } from 'react';
8-
import { shallow } from 'enzyme';
8+
import { shallow, mount } from 'enzyme';
99
import { act } from 'react-dom/test-utils';
1010
import { EuiPopover, EuiLink } from '@elastic/eui';
1111
import { createMockedIndexPattern } from '../../../mocks';
@@ -28,8 +28,7 @@ const defaultProps = {
2828
Button: ({ onClick }: { onClick: MouseEventHandler }) => (
2929
<EuiLink onClick={onClick}>trigger</EuiLink>
3030
),
31-
isOpenByCreation: true,
32-
setIsOpenByCreation: jest.fn(),
31+
initiallyOpen: true,
3332
};
3433

3534
describe('filter popover', () => {
@@ -39,16 +38,14 @@ describe('filter popover', () => {
3938
},
4039
}));
4140
it('should be open if is open by creation', () => {
42-
const setIsOpenByCreation = jest.fn();
43-
const instance = shallow(
44-
<FilterPopover {...defaultProps} setIsOpenByCreation={setIsOpenByCreation} />
45-
);
41+
const instance = mount(<FilterPopover {...defaultProps} />);
42+
instance.update();
4643
expect(instance.find(EuiPopover).prop('isOpen')).toEqual(true);
4744
act(() => {
4845
instance.find(EuiPopover).prop('closePopover')!();
4946
});
5047
instance.update();
51-
expect(setIsOpenByCreation).toHaveBeenCalledWith(false);
48+
expect(instance.find(EuiPopover).prop('isOpen')).toEqual(false);
5249
});
5350
it('should call setFilter when modifying QueryInput', () => {
5451
const setFilter = jest.fn();

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filter_popover.tsx

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66
import './filter_popover.scss';
77

8-
import React, { MouseEventHandler, useState } from 'react';
8+
import React, { MouseEventHandler, useEffect, useState } from 'react';
99
import useDebounce from 'react-use/lib/useDebounce';
1010
import { EuiPopover, EuiSpacer } from '@elastic/eui';
1111
import { i18n } from '@kbn/i18n';
@@ -19,23 +19,24 @@ export const FilterPopover = ({
1919
setFilter,
2020
indexPattern,
2121
Button,
22-
isOpenByCreation,
23-
setIsOpenByCreation,
22+
initiallyOpen,
2423
}: {
2524
filter: FilterValue;
2625
setFilter: Function;
2726
indexPattern: IndexPattern;
2827
Button: React.FunctionComponent<{ onClick: MouseEventHandler }>;
29-
isOpenByCreation: boolean;
30-
setIsOpenByCreation: Function;
28+
initiallyOpen: boolean;
3129
}) => {
3230
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
3331
const inputRef = React.useRef<HTMLInputElement>();
3432

33+
// set popover open on start to work around EUI bug
34+
useEffect(() => {
35+
setIsPopoverOpen(initiallyOpen);
36+
// eslint-disable-next-line react-hooks/exhaustive-deps
37+
}, []);
38+
3539
const closePopover = () => {
36-
if (isOpenByCreation) {
37-
setIsOpenByCreation(false);
38-
}
3940
if (isPopoverOpen) {
4041
setIsPopoverOpen(false);
4142
}
@@ -59,15 +60,12 @@ export const FilterPopover = ({
5960
data-test-subj="indexPattern-filters-existingFilterContainer"
6061
anchorClassName="eui-fullWidth"
6162
panelClassName="lnsIndexPatternDimensionEditor__filtersEditor"
62-
isOpen={isOpenByCreation || isPopoverOpen}
63+
isOpen={isPopoverOpen}
6364
ownFocus
6465
closePopover={() => closePopover()}
6566
button={
6667
<Button
6768
onClick={() => {
68-
if (isOpenByCreation) {
69-
setIsOpenByCreation(false);
70-
}
7169
setIsPopoverOpen((open) => !open);
7270
}}
7371
/>

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ export const FilterList = ({
203203
<>
204204
<DragDropBuckets
205205
onDragEnd={updateFilters}
206-
onDragStart={() => setIsOpenByCreation(false)}
206+
onDragStart={() => {}}
207207
droppableId="FILTERS_DROPPABLE_AREA"
208208
items={localFilters}
209209
>
@@ -227,8 +227,7 @@ export const FilterList = ({
227227
>
228228
<FilterPopover
229229
data-test-subj="indexPattern-filters-existingFilterContainer"
230-
isOpenByCreation={idx === localFilters.length - 1 && isOpenByCreation}
231-
setIsOpenByCreation={setIsOpenByCreation}
230+
initiallyOpen={idx === localFilters.length - 1 && isOpenByCreation}
232231
indexPattern={indexPattern}
233232
filter={filter}
234233
setFilter={(f: FilterValue) => {

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import './advanced_editor.scss';
88

9-
import React, { useState, MouseEventHandler } from 'react';
9+
import React, { useState, MouseEventHandler, useEffect } from 'react';
1010
import { i18n } from '@kbn/i18n';
1111
import {
1212
EuiFlexGroup,
@@ -47,18 +47,22 @@ export const RangePopover = ({
4747
range,
4848
setRange,
4949
Button,
50-
isOpenByCreation,
51-
setIsOpenByCreation,
50+
initiallyOpen,
5251
}: {
5352
range: LocalRangeType;
5453
setRange: (newRange: LocalRangeType) => void;
5554
Button: React.FunctionComponent<{ onClick: MouseEventHandler }>;
56-
isOpenByCreation: boolean;
57-
setIsOpenByCreation: (open: boolean) => void;
55+
initiallyOpen: boolean;
5856
}) => {
5957
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
6058
const [tempRange, setTempRange] = useState(range);
6159

60+
// set popover open on start to work around EUI bug
61+
useEffect(() => {
62+
setIsPopoverOpen(initiallyOpen);
63+
// eslint-disable-next-line react-hooks/exhaustive-deps
64+
}, []);
65+
6266
const saveRangeAndReset = (newRange: LocalRangeType, resetRange = false) => {
6367
if (resetRange) {
6468
// reset the temporary range for later use
@@ -86,9 +90,6 @@ export const RangePopover = ({
8690
});
8791

8892
const onSubmit = () => {
89-
if (isOpenByCreation) {
90-
setIsOpenByCreation(false);
91-
}
9293
if (isPopoverOpen) {
9394
setIsPopoverOpen(false);
9495
}
@@ -98,14 +99,11 @@ export const RangePopover = ({
9899
<EuiPopover
99100
display="block"
100101
ownFocus
101-
isOpen={isOpenByCreation || isPopoverOpen}
102+
isOpen={isPopoverOpen}
102103
closePopover={onSubmit}
103104
button={
104105
<Button
105106
onClick={() => {
106-
if (isOpenByCreation) {
107-
setIsOpenByCreation(false);
108-
}
109107
setIsPopoverOpen((isOpen) => !isOpen);
110108
}}
111109
/>
@@ -255,11 +253,7 @@ export const AdvancedRangeEditor = ({
255253
<>
256254
<DragDropBuckets
257255
onDragEnd={setLocalRanges}
258-
onDragStart={() => {
259-
if (isOpenByCreation) {
260-
setIsOpenByCreation(false);
261-
}
262-
}}
256+
onDragStart={() => {}}
263257
droppableId="RANGES_DROPPABLE_AREA"
264258
items={localRanges}
265259
>
@@ -283,8 +277,7 @@ export const AdvancedRangeEditor = ({
283277
>
284278
<RangePopover
285279
range={range}
286-
isOpenByCreation={idx === lastIndex && isOpenByCreation}
287-
setIsOpenByCreation={setIsOpenByCreation}
280+
initiallyOpen={idx === lastIndex && isOpenByCreation}
288281
setRange={(newRange: LocalRangeType) => {
289282
const newRanges = [...localRanges];
290283
if (newRange.id === newRanges[idx].id) {

0 commit comments

Comments
 (0)