Skip to content

Commit

Permalink
[Embeddables Rebuild] [Controls] Fix data control editor type selector (
Browse files Browse the repository at this point in the history
#187390)

Closes #187382

## Summary

This PR separates out the previously memoized
`CompatibleControlTypesComponent` into a separate component that accepts
**props** for the fields that it is dependant on rather than relying on
the dependencies to the `useMemo` function. This is because, previously,
we had an extra dependency in the dependency array (`controlType`) that
was causing the memoized component to render too many times and it was
causing a weird bug where the old "disabled" menu item wasn't getting
unmounted properly.

| Before | After |
|--------|--------|
| ![Jul-02-2024
13-21-44](https://github.com/elastic/kibana/assets/8698078/240b561e-f3b7-4519-bfe1-caf550927310)
| ![Jul-02-2024
13-12-29](https://github.com/elastic/kibana/assets/8698078/4f9b4eb6-2ce3-471e-a5b8-4b92179c48bc)
|

By switching to a component with explicit props, unnecessary
dependencies should hopefully be avoided in the future.


### 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 3, 2024
1 parent e4a44fd commit 5e353a3
Showing 1 changed file with 77 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,22 @@ import {
EuiTitle,
EuiToolTip,
} from '@elastic/eui';
import {
ControlWidth,
DEFAULT_CONTROL_GROW,
DEFAULT_CONTROL_WIDTH,
} from '@kbn/controls-plugin/common';
import { CONTROL_WIDTH_OPTIONS } from '@kbn/controls-plugin/public';
import { DataControlFieldRegistry } from '@kbn/controls-plugin/public/types';
import { DataViewField } from '@kbn/data-views-plugin/common';
import { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public';
import { useBatchedPublishingSubjects } from '@kbn/presentation-publishing';
import {
LazyDataViewPicker,
LazyFieldPicker,
withSuspense,
} from '@kbn/presentation-util-plugin/public';

import {
ControlWidth,
DEFAULT_CONTROL_GROW,
DEFAULT_CONTROL_WIDTH,
} from '@kbn/controls-plugin/common';
import { CONTROL_WIDTH_OPTIONS } from '@kbn/controls-plugin/public';
import { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public';
import { getAllControlTypes, getControlFactory } from '../control_factory_registry';
import { ControlGroupApi } from '../control_group/types';
import { ControlStateManager } from '../types';
Expand All @@ -69,6 +70,65 @@ export interface ControlEditorProps<
const FieldPicker = withSuspense(LazyFieldPicker, null);
const DataViewPicker = withSuspense(LazyDataViewPicker, null);

const CompatibleControlTypesComponent = ({
fieldRegistry,
selectedFieldName,
selectedControlType,
setSelectedControlType,
}: {
fieldRegistry?: DataControlFieldRegistry;
selectedFieldName: string;
selectedControlType?: string;
setSelectedControlType: (type: string) => void;
}) => {
const dataControlFactories = useMemo(() => {
return getAllControlTypes()
.map((type) => getControlFactory(type))
.filter((factory) => {
return isDataControlFactory(factory);
});
}, []);

return (
<EuiKeyPadMenu data-test-subj={`controlTypeMenu`} aria-label={'type'}>
{dataControlFactories.map((factory) => {
const disabled =
fieldRegistry && selectedFieldName
? !fieldRegistry[selectedFieldName]?.compatibleControlTypes.includes(factory.type)
: true;
const keyPadMenuItem = (
<EuiKeyPadMenuItem
key={factory.type}
id={`create__${factory.type}`}
aria-label={factory.getDisplayName()}
data-test-subj={`create__${factory.type}`}
isSelected={factory.type === selectedControlType}
disabled={disabled}
onClick={() => setSelectedControlType(factory.type)}
label={factory.getDisplayName()}
>
<EuiIcon type={factory.getIconType()} size="l" />
</EuiKeyPadMenuItem>
);

return disabled ? (
<EuiToolTip
key={`disabled__${factory.type}`}
content={DataControlEditorStrings.manageControl.dataSource.getControlTypeErrorMessage({
fieldSelected: Boolean(selectedFieldName),
controlType: factory.getDisplayName(),
})}
>
{keyPadMenuItem}
</EuiToolTip>
) : (
keyPadMenuItem
);
})}
</EuiKeyPadMenu>
);
};

export const DataControlEditor = ({
controlId,
controlType,
Expand Down Expand Up @@ -139,57 +199,6 @@ export const DataControlEditor = ({
);
}, [selectedFieldName, setControlEditorValid, selectedDataView, selectedControlType]);

const dataControlFactories = useMemo(() => {
return getAllControlTypes()
.map((type) => getControlFactory(type))
.filter((factory) => {
return isDataControlFactory(factory);
});
}, []);

const CompatibleControlTypesComponent = useMemo(() => {
return (
<EuiKeyPadMenu data-test-subj={`controlTypeMenu`} aria-label={'type'}>
{dataControlFactories.map((factory) => {
const disabled =
fieldRegistry && selectedFieldName
? !fieldRegistry[selectedFieldName]?.compatibleControlTypes.includes(factory.type)
: true;
const keyPadMenuItem = (
<EuiKeyPadMenuItem
key={factory.type}
id={`create__${factory.type}`}
aria-label={factory.getDisplayName()}
data-test-subj={`create__${factory.type}`}
isSelected={factory.type === selectedControlType}
disabled={disabled}
onClick={() => setSelectedControlType(factory.type)}
label={factory.getDisplayName()}
>
<EuiIcon type={factory.getIconType()} size="l" />
</EuiKeyPadMenuItem>
);

return disabled ? (
<EuiToolTip
key={`disabled__${controlType}`}
content={DataControlEditorStrings.manageControl.dataSource.getControlTypeErrorMessage(
{
fieldSelected: Boolean(selectedFieldName),
controlType,
}
)}
>
{keyPadMenuItem}
</EuiToolTip>
) : (
keyPadMenuItem
);
})}
</EuiKeyPadMenu>
);
}, [selectedFieldName, fieldRegistry, selectedControlType, controlType, dataControlFactories]);

const CustomSettingsComponent = useMemo(() => {
if (!selectedControlType || !selectedFieldName || !fieldRegistry) return;

Expand Down Expand Up @@ -254,6 +263,7 @@ export const DataControlEditor = ({
selectedDataViewId={selectedDataViewId}
onChangeDataViewId={(newDataViewId) => {
stateManager.dataViewId.next(newDataViewId);
setSelectedControlType(undefined);
}}
trigger={{
label:
Expand Down Expand Up @@ -300,7 +310,15 @@ export const DataControlEditor = ({
<EuiFormRow
label={DataControlEditorStrings.manageControl.dataSource.getControlTypeTitle()}
>
{CompatibleControlTypesComponent}
{/* wrapping in `div` so that focus gets passed properly to the form row */}
<div>
<CompatibleControlTypesComponent
fieldRegistry={fieldRegistry}
selectedFieldName={selectedFieldName}
selectedControlType={selectedControlType}
setSelectedControlType={setSelectedControlType}
/>
</div>
</EuiFormRow>
</EuiDescribedFormGroup>
<EuiDescribedFormGroup
Expand Down

0 comments on commit 5e353a3

Please sign in to comment.