Skip to content

Commit

Permalink
[Embeddable Rebuild] [Controls] Fix control state on edit (#188784)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes control editing so that, when the control type is changed,
extra state from the old type gets removed. Prior to this, controls were
keeping unrelated state - for example, switching from a range slider to
a search control would result in a search control with the "step"
property.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
Heenawter authored Jul 23, 2024
1 parent dc3d960 commit 457f08b
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ export const DataControlEditor = <State extends DataControlEditorState = DataCon
initialState.controlType
);
const [controlEditorValid, setControlEditorValid] = useState<boolean>(false);

/** TODO: Make `editorConfig` work when refactoring the `ControlGroupRenderer` */
// const editorConfig = controlGroup.getEditorConfig();

Expand Down Expand Up @@ -193,7 +194,6 @@ export const DataControlEditor = <State extends DataControlEditorState = DataCon
const CustomSettings = controlFactory.CustomOptionsComponent;

if (!CustomSettings) return;

return (
<EuiDescribedFormGroup
ratio="third"
Expand All @@ -210,13 +210,13 @@ export const DataControlEditor = <State extends DataControlEditorState = DataCon
data-test-subj="control-editor-custom-settings"
>
<CustomSettings
initialState={initialState}
currentState={editorState}
updateState={(newState) => setEditorState({ ...editorState, ...newState })}
setControlEditorValid={setControlEditorValid}
/>
</EuiDescribedFormGroup>
);
}, [fieldRegistry, selectedControlType, editorState, initialState]);
}, [fieldRegistry, selectedControlType, editorState]);

return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export const initializeDataControl = <EditorState extends object = {}>(
);
} else {
// replace the control with a new one of the updated type
controlGroup.replacePanel(controlId, { panelType: newType, initialState });
controlGroup.replacePanel(controlId, { panelType: newType, initialState: newState });
}
},
initialState: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ describe('RangesliderControlApi', () => {
const CustomSettings = factory.CustomOptionsComponent!;
const component = render(
<CustomSettings
initialState={{}}
currentState={{}}
updateState={jest.fn()}
setControlEditorValid={jest.fn()}
/>
Expand All @@ -263,7 +263,7 @@ describe('RangesliderControlApi', () => {
const CustomSettings = factory.CustomOptionsComponent!;
const component = render(
<CustomSettings
initialState={{}}
currentState={{}}
updateState={jest.fn()}
setControlEditorValid={setControlEditorValid}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import React, { useEffect, useMemo, useState } from 'react';
import React, { useEffect, useMemo } from 'react';
import deepEqual from 'react-fast-compare';

import { EuiFieldNumber, EuiFormRow } from '@elastic/eui';
Expand Down Expand Up @@ -38,17 +38,15 @@ export const getRangesliderControlFactory = (
isFieldCompatible: (field) => {
return field.aggregatable && field.type === 'number';
},
CustomOptionsComponent: ({ initialState, updateState, setControlEditorValid }) => {
const [step, setStep] = useState(initialState.step ?? 1);

CustomOptionsComponent: ({ currentState, updateState, setControlEditorValid }) => {
const step = currentState.step ?? 1;
return (
<>
<EuiFormRow fullWidth label={RangeSliderStrings.editor.getStepTitle()}>
<EuiFieldNumber
value={step}
onChange={(event) => {
const newStep = event.target.valueAsNumber;
setStep(newStep);
updateState({ step: newStep });
setControlEditorValid(newStep > 0);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import React, { useEffect, useState } from 'react';
import React, { useEffect } from 'react';
import deepEqual from 'react-fast-compare';
import { BehaviorSubject, combineLatest, debounceTime, distinctUntilChanged } from 'rxjs';

Expand Down Expand Up @@ -65,17 +65,15 @@ export const getSearchControlFactory = ({
(field.spec.esTypes ?? []).includes('text')
);
},
CustomOptionsComponent: ({ initialState, updateState }) => {
const [searchTechnique, setSearchTechnique] = useState(initialState.searchTechnique);

CustomOptionsComponent: ({ currentState, updateState }) => {
const searchTechnique = currentState.searchTechnique ?? DEFAULT_SEARCH_TECHNIQUE;
return (
<EuiFormRow label={'Searching'} data-test-subj="searchControl__searchOptionsRadioGroup">
<EuiRadioGroup
options={allSearchOptions}
idSelected={searchTechnique ?? DEFAULT_SEARCH_TECHNIQUE}
idSelected={searchTechnique}
onChange={(id) => {
const newSearchTechnique = id as SearchControlTechniques;
setSearchTechnique(newSearchTechnique);
updateState({ searchTechnique: newSearchTechnique });
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export interface DataControlFactory<
> extends ControlFactory<State, Api> {
isFieldCompatible: (field: DataViewField) => boolean;
CustomOptionsComponent?: React.FC<{
initialState: Omit<State, keyof DefaultDataControlState>;
currentState: Partial<State>;
updateState: (newState: Partial<State>) => void;
setControlEditorValid: (valid: boolean) => void;
}>;
Expand Down

0 comments on commit 457f08b

Please sign in to comment.