Skip to content

refactor(explore): improve typing for Dnd controls #16362

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

Merged
ktmud merged 1 commit into from
Aug 26, 2021
Merged

refactor(explore): improve typing for Dnd controls #16362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions superset-frontend/src/dashboard/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,15 @@ export type DashboardLayoutState = { present: DashboardLayout };
export type DashboardState = {
preselectNativeFilters?: JsonObject;
editMode: boolean;
isPublished: boolean;
directPathToChild: string[];
activeTabs: ActiveTabs;
fullSizeChartId: number | null;
isRefreshing: boolean;
hasUnsavedChanges: boolean;
};
export type DashboardInfo = {
id: number;
common: {
flash_messages: string[];
conf: JsonObject;
Expand Down
8 changes: 8 additions & 0 deletions superset-frontend/src/explore/components/Control.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
import React, { ReactNode, useCallback, useState } from 'react';
import { ControlType } from '@superset-ui/chart-controls';
import { ControlComponentProps as BaseControlComponentProps } from '@superset-ui/chart-controls/lib/shared-controls/components/types';
Copy link
Copy Markdown
Member

@kgabryje kgabryje Aug 26, 2021

Choose a reason for hiding this comment

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

Non-blocking - should we export that from chart-controls in the next release of superset-ui, so that we can import from '@superset-ui/chart-controls'?

Copy link
Copy Markdown
Member Author

@ktmud ktmud Aug 26, 2021

Choose a reason for hiding this comment

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

Agreed. I'll do that the next time I touch superset-ui.

import { JsonValue, QueryFormData } from '@superset-ui/core';
import ErrorBoundary from 'src/components/ErrorBoundary';
import { ExploreActions } from 'src/explore/actions/exploreActions';
Expand All @@ -43,6 +44,13 @@ export type ControlProps = {
renderTrigger?: boolean;
};

/**
*
*/
export type ControlComponentProps<
ValueType extends JsonValue = JsonValue
> = Omit<ControlProps, 'value'> & BaseControlComponentProps<ValueType>;

export default function Control(props: ControlProps) {
const {
actions: { setControlValue },
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/explore/components/ControlHeader.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import Icons from 'src/components/Icons';

const propTypes = {
name: PropTypes.string,
label: PropTypes.string,
description: PropTypes.string,
label: PropTypes.node,
description: PropTypes.node,
validationErrors: PropTypes.array,
renderTrigger: PropTypes.bool,
rightNode: PropTypes.node,
Expand Down
9 changes: 2 additions & 7 deletions superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const datasource = {
datasource_name: 'table1',
description: 'desc',
};
const props = {
const props: DatasourcePanelProps = {
datasource,
controls: {
datasource: {
Expand All @@ -57,12 +57,7 @@ const props = {
},
},
actions: {
setControlValue: () => ({
type: 'type',
controlName: 'control',
value: 'val',
validationErrors: [],
}),
setControlValue: jest.fn(),
},
};

Expand Down
117 changes: 56 additions & 61 deletions superset-frontend/src/explore/components/DatasourcePanel/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, {
useCallback,
useEffect,
useMemo,
useRef,
useState,
} from 'react';
import React, { useEffect, useMemo, useRef, useState } from 'react';
import { ControlConfig, DatasourceMeta } from '@superset-ui/chart-controls';
import { debounce } from 'lodash';
import { matchSorter, rankings } from 'match-sorter';
Expand Down Expand Up @@ -193,60 +187,61 @@ export default function DataSourcePanel({
const DEFAULT_MAX_COLUMNS_LENGTH = 50;
const DEFAULT_MAX_METRICS_LENGTH = 50;

const search = useCallback(
debounce((value: string) => {
if (value === '') {
setList({ columns, metrics });
return;
}
setList({
columns: matchSorter(columns, value, {
keys: [
{
key: 'verbose_name',
threshold: rankings.CONTAINS,
},
{
key: 'column_name',
threshold: rankings.CONTAINS,
},
{
key: item =>
[item.description, item.expression].map(
x => x?.replace(/[_\n\s]+/g, ' ') || '',
),
threshold: rankings.CONTAINS,
maxRanking: rankings.CONTAINS,
},
],
keepDiacritics: true,
}),
metrics: matchSorter(metrics, value, {
keys: [
{
key: 'verbose_name',
threshold: rankings.CONTAINS,
},
{
key: 'metric_name',
threshold: rankings.CONTAINS,
},
{
key: item =>
[item.description, item.expression].map(
x => x?.replace(/[_\n\s]+/g, ' ') || '',
),
threshold: rankings.CONTAINS,
maxRanking: rankings.CONTAINS,
},
],
keepDiacritics: true,
baseSort: (a, b) =>
Number(b.item.is_certified) - Number(a.item.is_certified) ||
String(a.rankedValue).localeCompare(b.rankedValue),
}),
});
}, FAST_DEBOUNCE),
const search = useMemo(
() =>
debounce((value: string) => {
if (value === '') {
setList({ columns, metrics });
return;
}
setList({
columns: matchSorter(columns, value, {
keys: [
{
key: 'verbose_name',
threshold: rankings.CONTAINS,
},
{
key: 'column_name',
threshold: rankings.CONTAINS,
},
{
key: item =>
[item.description, item.expression].map(
x => x?.replace(/[_\n\s]+/g, ' ') || '',
),
threshold: rankings.CONTAINS,
maxRanking: rankings.CONTAINS,
},
],
keepDiacritics: true,
}),
metrics: matchSorter(metrics, value, {
keys: [
{
key: 'verbose_name',
threshold: rankings.CONTAINS,
},
{
key: 'metric_name',
threshold: rankings.CONTAINS,
},
{
key: item =>
[item.description, item.expression].map(
x => x?.replace(/[_\n\s]+/g, ' ') || '',
),
threshold: rankings.CONTAINS,
maxRanking: rankings.CONTAINS,
},
],
keepDiacritics: true,
baseSort: (a, b) =>
Number(b.item.is_certified) - Number(a.item.is_certified) ||
String(a.rankedValue).localeCompare(b.rankedValue),
}),
});
}, FAST_DEBOUNCE),
[columns, metrics],
);

Expand Down
10 changes: 7 additions & 3 deletions ...-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@
*/
import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import { LabelProps } from 'src/explore/components/controls/DndColumnSelectControl/types';
import { DndColumnSelect } from 'src/explore/components/controls/DndColumnSelectControl/DndColumnSelect';
import {
DndColumnSelect,
DndColumnSelectProps,
} from 'src/explore/components/controls/DndColumnSelectControl/DndColumnSelect';

const defaultProps: LabelProps = {
const defaultProps: DndColumnSelectProps = {
type: 'DndColumnSelect',
name: 'Filter',
onChange: jest.fn(),
options: { string: { column_name: 'Column A' } },
actions: { setControlValue: jest.fn() },
};

test('renders with default props', () => {
Expand Down
13 changes: 9 additions & 4 deletions superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,20 @@ import React, { useCallback, useMemo, useState } from 'react';
import { FeatureFlag, isFeatureEnabled, tn } from '@superset-ui/core';
import { ColumnMeta } from '@superset-ui/chart-controls';
import { isEmpty } from 'lodash';
import { LabelProps } from 'src/explore/components/controls/DndColumnSelectControl/types';
import DndSelectLabel from 'src/explore/components/controls/DndColumnSelectControl/DndSelectLabel';
import OptionWrapper from 'src/explore/components/controls/DndColumnSelectControl/OptionWrapper';
import { OptionSelector } from 'src/explore/components/controls/DndColumnSelectControl/utils';
import { DatasourcePanelDndItem } from 'src/explore/components/DatasourcePanel/types';
import { DndItemType } from 'src/explore/components/DndItemType';
import { useComponentDidUpdate } from 'src/common/hooks/useComponentDidUpdate';
import ColumnSelectPopoverTrigger from './ColumnSelectPopoverTrigger';
import { DndControlProps } from './types';

export const DndColumnSelect = (props: LabelProps) => {
export type DndColumnSelectProps = DndControlProps<string> & {
options: Record<string, ColumnMeta>;
};

export function DndColumnSelect(props: DndColumnSelectProps) {
const {
value,
options,
Expand Down Expand Up @@ -68,6 +72,7 @@ export const DndColumnSelect = (props: LabelProps) => {
) {
onChange(optionSelectorValues);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [JSON.stringify(value), JSON.stringify(optionSelector.getValues())]);

// useComponentDidUpdate to avoid running this for the first render, to avoid
Expand Down Expand Up @@ -203,7 +208,7 @@ export const DndColumnSelect = (props: LabelProps) => {

return (
<div>
<DndSelectLabel<string | string[], ColumnMeta[]>
<DndSelectLabel
onDrop={onDrop}
canDrop={canDrop}
valuesRenderer={valuesRenderer}
Expand All @@ -229,4 +234,4 @@ export const DndColumnSelect = (props: LabelProps) => {
</ColumnSelectPopoverTrigger>
</div>
);
};
}
41 changes: 30 additions & 11 deletions ...-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,24 @@ import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetr
import AdhocFilter, {
EXPRESSION_TYPES,
} from 'src/explore/components/controls/FilterControl/AdhocFilter';
import { DndFilterSelect } from 'src/explore/components/controls/DndColumnSelectControl/DndFilterSelect';
import {
DndFilterSelect,
DndFilterSelectProps,
} from 'src/explore/components/controls/DndColumnSelectControl/DndFilterSelect';
import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants';
import { DEFAULT_FORM_DATA } from '@superset-ui/plugin-chart-echarts/lib/Timeseries/types';

const defaultProps = {
const defaultProps: DndFilterSelectProps = {
type: 'DndFilterSelect',
name: 'Filter',
value: [],
columns: [],
datasource: {},
formData: {},
datasource: PLACEHOLDER_DATASOURCE,
formData: null,
savedMetrics: [],
selectedMetrics: [],
onChange: jest.fn(),
options: { string: { column_name: 'Column' } },
actions: { setControlValue: jest.fn() },
};

test('renders with default props', () => {
Expand All @@ -53,9 +60,15 @@ test('renders with value', () => {
});

test('renders options with saved metric', () => {
render(<DndFilterSelect {...defaultProps} formData={['saved_metric']} />, {
useDnd: true,
});
render(
<DndFilterSelect
{...defaultProps}
formData={{ ...DEFAULT_FORM_DATA, metrics: ['saved_metric'] }}
/>,
{
useDnd: true,
},
);
expect(screen.getByText('Drop columns or metrics here')).toBeInTheDocument();
});

Expand Down Expand Up @@ -84,8 +97,14 @@ test('renders options with adhoc metric', () => {
expression: 'AVG(birth_names.num)',
metric_name: 'avg__num',
});
render(<DndFilterSelect {...defaultProps} formData={[adhocMetric]} />, {
useDnd: true,
});
render(
<DndFilterSelect
{...defaultProps}
formData={{ ...DEFAULT_FORM_DATA, metrics: [adhocMetric] }}
/>,
{
useDnd: true,
},
);
expect(screen.getByText('Drop columns or metrics here')).toBeInTheDocument();
});
Loading