Skip to content

Commit d02e3a9

Browse files
authored
[Lens] refactor DimensionContainer and fix flyout bug (#79277) (#79430)
1 parent 03050c6 commit d02e3a9

File tree

5 files changed

+146
-245
lines changed

5 files changed

+146
-245
lines changed

x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,7 @@
1313
animation: euiFlyout $euiAnimSpeedNormal $euiAnimSlightResistance;
1414
}
1515

16-
.lnsDimensionContainer--noAnimation {
17-
animation: none;
18-
}
19-
2016
.lnsDimensionContainer__footer,
2117
.lnsDimensionContainer__header {
2218
padding: $euiSizeS;
2319
}
24-
25-
.lnsDimensionContainer__trigger {
26-
width: 100%;
27-
display: block;
28-
word-break: break-word;
29-
}

x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx

Lines changed: 25 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -16,89 +16,42 @@ import {
1616
EuiOutsideClickDetector,
1717
} from '@elastic/eui';
1818

19-
import classNames from 'classnames';
2019
import { i18n } from '@kbn/i18n';
21-
import { VisualizationDimensionGroupConfig } from '../../../types';
22-
import { DimensionContainerState } from './types';
2320

2421
export function DimensionContainer({
25-
dimensionContainerState,
26-
setDimensionContainerState,
27-
groups,
28-
accessor,
29-
groupId,
30-
trigger,
22+
isOpen,
23+
groupLabel,
24+
handleClose,
3125
panel,
32-
panelTitle,
3326
}: {
34-
dimensionContainerState: DimensionContainerState;
35-
setDimensionContainerState: (newState: DimensionContainerState) => void;
36-
groups: VisualizationDimensionGroupConfig[];
37-
accessor: string;
38-
groupId: string;
39-
trigger: React.ReactElement;
27+
isOpen: boolean;
28+
handleClose: () => void;
4029
panel: React.ReactElement;
41-
panelTitle: React.ReactNode;
30+
groupLabel: string;
4231
}) {
43-
const [openByCreation, setIsOpenByCreation] = useState(
44-
dimensionContainerState.openId === accessor
45-
);
4632
const [focusTrapIsEnabled, setFocusTrapIsEnabled] = useState(false);
47-
const [flyoutIsVisible, setFlyoutIsVisible] = useState(false);
48-
49-
const noMatch = dimensionContainerState.isOpen
50-
? !groups.some((d) => d.accessors.includes(accessor))
51-
: false;
5233

5334
const closeFlyout = () => {
54-
setDimensionContainerState({
55-
isOpen: false,
56-
openId: null,
57-
addingToGroupId: null,
58-
});
59-
setIsOpenByCreation(false);
35+
handleClose();
6036
setFocusTrapIsEnabled(false);
61-
setFlyoutIsVisible(false);
62-
};
63-
64-
const openFlyout = () => {
65-
setFlyoutIsVisible(true);
66-
setTimeout(() => {
67-
setFocusTrapIsEnabled(true);
68-
}, 255);
6937
};
7038

71-
const flyoutShouldBeOpen =
72-
dimensionContainerState.isOpen &&
73-
(dimensionContainerState.openId === accessor ||
74-
(noMatch && dimensionContainerState.addingToGroupId === groupId));
75-
7639
useEffect(() => {
77-
if (flyoutShouldBeOpen) {
78-
openFlyout();
40+
if (isOpen) {
41+
// without setTimeout here the flyout pushes content when animating
42+
setTimeout(() => {
43+
setFocusTrapIsEnabled(true);
44+
}, 255);
7945
}
80-
});
46+
}, [isOpen]);
8147

82-
useEffect(() => {
83-
if (!flyoutShouldBeOpen) {
84-
if (flyoutIsVisible) {
85-
setFlyoutIsVisible(false);
86-
}
87-
if (focusTrapIsEnabled) {
88-
setFocusTrapIsEnabled(false);
89-
}
90-
}
91-
}, [flyoutShouldBeOpen, flyoutIsVisible, focusTrapIsEnabled]);
92-
93-
const flyout = flyoutIsVisible && (
48+
return isOpen ? (
9449
<EuiFocusTrap disabled={!focusTrapIsEnabled} clickOutsideDisables={true}>
95-
<EuiOutsideClickDetector onOutsideClick={closeFlyout} isDisabled={!flyoutIsVisible}>
50+
<EuiOutsideClickDetector onOutsideClick={closeFlyout} isDisabled={!isOpen}>
9651
<div
9752
role="dialog"
9853
aria-labelledby="lnsDimensionContainerTitle"
99-
className={classNames('lnsDimensionContainer', {
100-
'lnsDimensionContainer--noAnimation': openByCreation,
101-
})}
54+
className="lnsDimensionContainer"
10255
>
10356
<EuiFlyoutHeader hasBorder className="lnsDimensionContainer__header">
10457
<EuiTitle size="xs">
@@ -109,7 +62,14 @@ export function DimensionContainer({
10962
iconType="sortLeft"
11063
flush="left"
11164
>
112-
<strong>{panelTitle}</strong>
65+
<strong>
66+
{i18n.translate('xpack.lens.configure.configurePanelTitle', {
67+
defaultMessage: '{groupLabel} configuration',
68+
values: {
69+
groupLabel,
70+
},
71+
})}
72+
</strong>
11373
</EuiButtonEmpty>
11474
</EuiTitle>
11575
</EuiFlyoutHeader>
@@ -126,12 +86,5 @@ export function DimensionContainer({
12686
</div>
12787
</EuiOutsideClickDetector>
12888
</EuiFocusTrap>
129-
);
130-
131-
return (
132-
<>
133-
{trigger}
134-
{flyout}
135-
</>
136-
);
89+
) : null;
13790
}

x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
} from '../../mocks';
1515
import { ChildDragDropProvider } from '../../../drag_drop';
1616
import { EuiFormRow } from '@elastic/eui';
17-
import { mount } from 'enzyme';
1817
import { mountWithIntl } from 'test_utils/enzyme_helpers';
1918
import { Visualization } from '../../../types';
2019
import { LayerPanel } from './layer_panel';
@@ -211,7 +210,7 @@ describe('LayerPanel', () => {
211210
groupId: 'a',
212211
accessors: ['newid'],
213212
filterOperations: () => true,
214-
supportsMoreColumns: false,
213+
supportsMoreColumns: true,
215214
dataTestSubj: 'lnsGroup',
216215
enableDimensionEditor: true,
217216
},
@@ -220,11 +219,14 @@ describe('LayerPanel', () => {
220219
mockVisualization.renderDimensionEditor = jest.fn();
221220

222221
const component = mountWithIntl(<LayerPanel {...getDefaultProps()} />);
222+
act(() => {
223+
component.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click');
224+
});
225+
component.update();
223226

224-
const group = component.find('DimensionContainer');
225-
const panel = mount(group.prop('panel'));
226-
227-
expect(panel.children()).toHaveLength(2);
227+
const group = component.find('DimensionContainer').first();
228+
const panel: React.ReactElement = group.prop('panel');
229+
expect(panel.props.children).toHaveLength(2);
228230
});
229231

230232
it('should keep the DimensionContainer open when configuring a new dimension', () => {
@@ -263,11 +265,8 @@ describe('LayerPanel', () => {
263265
});
264266

265267
const component = mountWithIntl(<LayerPanel {...getDefaultProps()} />);
266-
267-
const group = component.find('DimensionContainer');
268-
const triggerButton = mountWithIntl(group.prop('trigger'));
269268
act(() => {
270-
triggerButton.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click');
269+
component.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click');
271270
});
272271
component.update();
273272

@@ -312,10 +311,8 @@ describe('LayerPanel', () => {
312311

313312
const component = mountWithIntl(<LayerPanel {...getDefaultProps()} />);
314313

315-
const group = component.find('DimensionContainer');
316-
const triggerButton = mountWithIntl(group.prop('trigger'));
317314
act(() => {
318-
triggerButton.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click');
315+
component.find('[data-test-subj="lns-empty-dimension"]').first().simulate('click');
319316
});
320317
component.update();
321318
expect(component.find('EuiFlyoutHeader').exists()).toBe(true);

0 commit comments

Comments
 (0)