Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Lens] fix dimension label performance issues #69978

Merged
merged 7 commits into from
Jul 2, 2020
Merged

[Lens] fix dimension label performance issues #69978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1766ac3
perf: dimension label solved issues with typing
mbondyra Jun 25, 2020
bb4278f
states updates from the top
mbondyra Jun 25, 2020
dd95d28
Merge remote-tracking branch 'upstream/master' into dimension-label-p…
flash1293 Jun 26, 2020
f47ded5
Merge remote-tracking branch 'upstream/master' into dimension-label-p…
flash1293 Jun 26, 2020
66de115
memoize debounce call
flash1293 Jun 29, 2020
12f5988
Merge remote-tracking branch 'upstream/master' into dimension-label-p…
flash1293 Jun 29, 2020
b2f3623
Merge commit '80ae5648adbd5c69200cf10aa71439ae06fc3fcc' into dimensio…
mbondyra Jul 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ import { OperationMetadata } from '../../types';

jest.mock('../loader');
jest.mock('../state_helpers');
jest.mock('lodash', () => {
const original = jest.requireActual('lodash');

return {
...original,
debounce: (fn: unknown) => fn,
};
});

const expectedIndexPatterns = {
1: {
Expand Down
35 changes: 29 additions & 6 deletions x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/popover_editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import './popover_editor.scss';
import _ from 'lodash';
import React, { useState, useMemo } from 'react';
import React, { useState, useMemo, useEffect } from 'react';
import { i18n } from '@kbn/i18n';
import {
EuiFlexItem,
Expand Down Expand Up @@ -56,6 +56,31 @@ function asOperationOptions(operationTypes: OperationType[], compatibleWithCurre
}));
}

const LabelInput = ({ value, onChange }: { value: string; onChange: (value: string) => void }) => {
const [inputValue, setInputValue] = useState(value);

useEffect(() => {
setInputValue(value);
}, [value, setInputValue]);

const onChangeDebounced = useMemo(() => _.debounce(onChange, 256), [onChange]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a hook you could replace this with called useDebounce, via import { useDebounce } from 'react-use';. The main difference is that that hook clears itself on unmount.


const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const val = String(e.target.value);
setInputValue(val);
onChangeDebounced(val);
};

return (
<EuiFieldText
compressed
data-test-subj="indexPattern-label-edit"
value={inputValue}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this is only updating the value of the actual input field if the user types something in it - props changes coming in after the component got rendered the first time will be ignored.

A useEffect updating the local state when the global state management pushes an update down should work fine. This is a very similar case handling it the same way: https://github.com/elastic/kibana/pull/69672/files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Thanks!

onChange={handleInputChange}
/>
);
};

export function PopoverEditor(props: PopoverEditorProps) {
const {
selectedColumn,
Expand Down Expand Up @@ -320,11 +345,9 @@ export function PopoverEditor(props: PopoverEditorProps) {
})}
display="rowCompressed"
>
<EuiFieldText
compressed
data-test-subj="indexPattern-label-edit"
<LabelInput
value={selectedColumn.label}
onChange={(e) => {
onChange={(value) => {
setState({
...state,
layers: {
Expand All @@ -335,7 +358,7 @@ export function PopoverEditor(props: PopoverEditorProps) {
...state.layers[layerId].columns,
[columnId]: {
...selectedColumn,
label: e.target.value,
label: value,
customLabel: true,
},
},
Expand Down