mirror of
https://github.com/apache/superset.git
synced 2024-09-06 13:57:40 -04:00
fix(explore): clean data when hidding control (#19039)
This commit is contained in:
parent
2daa071633
commit
0e29871493
@ -34,6 +34,7 @@
|
|||||||
* control interface.
|
* control interface.
|
||||||
*/
|
*/
|
||||||
import React from 'react';
|
import React from 'react';
|
||||||
|
import { isEmpty } from 'lodash';
|
||||||
import {
|
import {
|
||||||
FeatureFlag,
|
FeatureFlag,
|
||||||
t,
|
t,
|
||||||
@ -43,7 +44,6 @@ import {
|
|||||||
SequentialScheme,
|
SequentialScheme,
|
||||||
legacyValidateInteger,
|
legacyValidateInteger,
|
||||||
validateNonEmpty,
|
validateNonEmpty,
|
||||||
JsonArray,
|
|
||||||
ComparisionType,
|
ComparisionType,
|
||||||
} from '@superset-ui/core';
|
} from '@superset-ui/core';
|
||||||
|
|
||||||
@ -352,7 +352,7 @@ const order_desc: SharedControlConfig<'CheckboxControl'> = {
|
|||||||
visibility: ({ controls }) =>
|
visibility: ({ controls }) =>
|
||||||
Boolean(
|
Boolean(
|
||||||
controls?.timeseries_limit_metric.value &&
|
controls?.timeseries_limit_metric.value &&
|
||||||
(controls?.timeseries_limit_metric.value as JsonArray).length,
|
!isEmpty(controls?.timeseries_limit_metric.value),
|
||||||
),
|
),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
94
superset-frontend/src/explore/components/Control.test.tsx
Normal file
94
superset-frontend/src/explore/components/Control.test.tsx
Normal file
@ -0,0 +1,94 @@
|
|||||||
|
/**
|
||||||
|
* Licensed to the Apache Software Foundation (ASF) under one
|
||||||
|
* or more contributor license agreements. See the NOTICE file
|
||||||
|
* distributed with this work for additional information
|
||||||
|
* regarding copyright ownership. The ASF licenses this file
|
||||||
|
* to you under the Apache License, Version 2.0 (the
|
||||||
|
* "License"); you may not use this file except in compliance
|
||||||
|
* with the License. You may obtain a copy of the License at
|
||||||
|
*
|
||||||
|
* http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
*
|
||||||
|
* Unless required by applicable law or agreed to in writing,
|
||||||
|
* software distributed under the License is distributed on an
|
||||||
|
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||||
|
* KIND, either express or implied. See the License for the
|
||||||
|
* specific language governing permissions and limitations
|
||||||
|
* under the License.
|
||||||
|
*/
|
||||||
|
import { mount } from 'enzyme';
|
||||||
|
import {
|
||||||
|
ThemeProvider,
|
||||||
|
supersetTheme,
|
||||||
|
promiseTimeout,
|
||||||
|
} from '@superset-ui/core';
|
||||||
|
import React from 'react';
|
||||||
|
import { render, screen } from 'spec/helpers/testing-library';
|
||||||
|
import Control, { ControlProps } from 'src/explore/components/Control';
|
||||||
|
|
||||||
|
const defaultProps: ControlProps = {
|
||||||
|
type: 'CheckboxControl',
|
||||||
|
name: 'checkbox',
|
||||||
|
value: true,
|
||||||
|
actions: {
|
||||||
|
setControlValue: jest.fn(),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const setup = (overrides = {}) => (
|
||||||
|
<ThemeProvider theme={supersetTheme}>
|
||||||
|
<Control {...defaultProps} {...overrides} />
|
||||||
|
</ThemeProvider>
|
||||||
|
);
|
||||||
|
|
||||||
|
describe('Control', () => {
|
||||||
|
it('render a control', () => {
|
||||||
|
render(setup());
|
||||||
|
|
||||||
|
const checkbox = screen.getByRole('checkbox');
|
||||||
|
expect(checkbox).toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('render null if type is not exit', () => {
|
||||||
|
render(
|
||||||
|
setup({
|
||||||
|
type: undefined,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(screen.queryByRole('checkbox')).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('render null if type is not valid', () => {
|
||||||
|
render(
|
||||||
|
setup({
|
||||||
|
type: 'UnkownControl',
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(screen.queryByRole('checkbox')).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('render null if isVisible is false', () => {
|
||||||
|
render(
|
||||||
|
setup({
|
||||||
|
isVisible: false,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(screen.queryByRole('checkbox')).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('call setControlValue if isVisible is false', () => {
|
||||||
|
const wrapper = mount(
|
||||||
|
setup({
|
||||||
|
isVisible: true,
|
||||||
|
default: false,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
wrapper.setProps({
|
||||||
|
isVisible: false,
|
||||||
|
default: false,
|
||||||
|
});
|
||||||
|
promiseTimeout(() => {
|
||||||
|
expect(defaultProps.actions.setControlValue).toBeCalled();
|
||||||
|
}, 100);
|
||||||
|
});
|
||||||
|
});
|
@ -16,12 +16,14 @@
|
|||||||
* specific language governing permissions and limitations
|
* specific language governing permissions and limitations
|
||||||
* under the License.
|
* under the License.
|
||||||
*/
|
*/
|
||||||
import React, { ReactNode, useCallback, useState } from 'react';
|
import React, { ReactNode, useCallback, useState, useEffect } from 'react';
|
||||||
|
import { isEqual } from 'lodash';
|
||||||
import {
|
import {
|
||||||
ControlType,
|
ControlType,
|
||||||
ControlComponentProps as BaseControlComponentProps,
|
ControlComponentProps as BaseControlComponentProps,
|
||||||
} from '@superset-ui/chart-controls';
|
} from '@superset-ui/chart-controls';
|
||||||
import { styled, JsonValue, QueryFormData } from '@superset-ui/core';
|
import { styled, JsonValue, QueryFormData } from '@superset-ui/core';
|
||||||
|
import { usePrevious } from 'src/hooks/usePrevious';
|
||||||
import ErrorBoundary from 'src/components/ErrorBoundary';
|
import ErrorBoundary from 'src/components/ErrorBoundary';
|
||||||
import { ExploreActions } from 'src/explore/actions/exploreActions';
|
import { ExploreActions } from 'src/explore/actions/exploreActions';
|
||||||
import controlMap from './controls';
|
import controlMap from './controls';
|
||||||
@ -42,6 +44,8 @@ export type ControlProps = {
|
|||||||
validationErrors?: any[];
|
validationErrors?: any[];
|
||||||
hidden?: boolean;
|
hidden?: boolean;
|
||||||
renderTrigger?: boolean;
|
renderTrigger?: boolean;
|
||||||
|
default?: JsonValue;
|
||||||
|
isVisible?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -60,15 +64,36 @@ export default function Control(props: ControlProps) {
|
|||||||
name,
|
name,
|
||||||
type,
|
type,
|
||||||
hidden,
|
hidden,
|
||||||
|
isVisible,
|
||||||
} = props;
|
} = props;
|
||||||
|
|
||||||
const [hovered, setHovered] = useState(false);
|
const [hovered, setHovered] = useState(false);
|
||||||
|
const wasVisible = usePrevious(isVisible);
|
||||||
const onChange = useCallback(
|
const onChange = useCallback(
|
||||||
(value: any, errors: any[]) => setControlValue(name, value, errors),
|
(value: any, errors: any[]) => setControlValue(name, value, errors),
|
||||||
[name, setControlValue],
|
[name, setControlValue],
|
||||||
);
|
);
|
||||||
|
|
||||||
if (!type) return null;
|
useEffect(() => {
|
||||||
|
if (
|
||||||
|
wasVisible === true &&
|
||||||
|
isVisible === false &&
|
||||||
|
props.default !== undefined &&
|
||||||
|
!isEqual(props.value, props.default)
|
||||||
|
) {
|
||||||
|
// reset control value if setting to invisible
|
||||||
|
setControlValue?.(name, props.default);
|
||||||
|
}
|
||||||
|
}, [
|
||||||
|
name,
|
||||||
|
wasVisible,
|
||||||
|
isVisible,
|
||||||
|
setControlValue,
|
||||||
|
props.value,
|
||||||
|
props.default,
|
||||||
|
]);
|
||||||
|
|
||||||
|
if (!type || isVisible === false) return null;
|
||||||
|
|
||||||
const ControlComponent = typeof type === 'string' ? controlMap[type] : type;
|
const ControlComponent = typeof type === 'string' ? controlMap[type] : type;
|
||||||
if (!ControlComponent) {
|
if (!ControlComponent) {
|
||||||
|
@ -283,16 +283,17 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
|
|||||||
validationErrors?: any[];
|
validationErrors?: any[];
|
||||||
};
|
};
|
||||||
|
|
||||||
// if visibility check says the config is not visible, don't render it
|
const isVisible = visibility
|
||||||
if (visibility && !visibility.call(config, props, controlData)) {
|
? visibility.call(config, props, controlData)
|
||||||
return null;
|
: undefined;
|
||||||
}
|
|
||||||
return (
|
return (
|
||||||
<Control
|
<Control
|
||||||
key={`control-${name}`}
|
key={`control-${name}`}
|
||||||
name={name}
|
name={name}
|
||||||
validationErrors={validationErrors}
|
validationErrors={validationErrors}
|
||||||
actions={props.actions}
|
actions={props.actions}
|
||||||
|
isVisible={isVisible}
|
||||||
{...restProps}
|
{...restProps}
|
||||||
/>
|
/>
|
||||||
);
|
);
|
||||||
|
@ -17,20 +17,51 @@
|
|||||||
* under the License.
|
* under the License.
|
||||||
*/
|
*/
|
||||||
import React from 'react';
|
import React from 'react';
|
||||||
import { render } from 'spec/helpers/testing-library';
|
import { render, screen } from 'spec/helpers/testing-library';
|
||||||
import ControlSetRow from 'src/explore/components/ControlRow';
|
import ControlSetRow from 'src/explore/components/ControlRow';
|
||||||
|
|
||||||
|
const MockControl = (props: {
|
||||||
|
children: React.ReactElement;
|
||||||
|
type?: string;
|
||||||
|
isVisible?: boolean;
|
||||||
|
}) => <div>{props.children}</div>;
|
||||||
describe('ControlSetRow', () => {
|
describe('ControlSetRow', () => {
|
||||||
it('renders a single row with one element', () => {
|
it('renders a single row with one element', () => {
|
||||||
const { getAllByText } = render(
|
render(<ControlSetRow controls={[<p>My Control 1</p>]} />);
|
||||||
<ControlSetRow controls={[<p>My Control 1</p>]} />,
|
expect(screen.getAllByText('My Control 1').length).toBe(1);
|
||||||
);
|
|
||||||
expect(getAllByText('My Control 1').length).toBe(1);
|
|
||||||
});
|
});
|
||||||
it('renders a single row with two elements', () => {
|
it('renders a single row with two elements', () => {
|
||||||
const { getAllByText } = render(
|
render(
|
||||||
<ControlSetRow controls={[<p>My Control 1</p>, <p>My Control 2</p>]} />,
|
<ControlSetRow controls={[<p>My Control 1</p>, <p>My Control 2</p>]} />,
|
||||||
);
|
);
|
||||||
expect(getAllByText(/My Control/)).toHaveLength(2);
|
expect(screen.getAllByText(/My Control/)).toHaveLength(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('renders a single row with one elements if is HiddenControl', () => {
|
||||||
|
render(
|
||||||
|
<ControlSetRow
|
||||||
|
controls={[
|
||||||
|
<p>My Control 1</p>,
|
||||||
|
<MockControl type="HiddenControl">
|
||||||
|
<p>My Control 2</p>
|
||||||
|
</MockControl>,
|
||||||
|
]}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(screen.getAllByText(/My Control/)).toHaveLength(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('renders a single row with one elements if is invisible', () => {
|
||||||
|
render(
|
||||||
|
<ControlSetRow
|
||||||
|
controls={[
|
||||||
|
<p>My Control 1</p>,
|
||||||
|
<MockControl isVisible={false}>
|
||||||
|
<p>My Control 2</p>
|
||||||
|
</MockControl>,
|
||||||
|
]}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(screen.getAllByText(/My Control/)).toHaveLength(2);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
@ -16,26 +16,34 @@
|
|||||||
* specific language governing permissions and limitations
|
* specific language governing permissions and limitations
|
||||||
* under the License.
|
* under the License.
|
||||||
*/
|
*/
|
||||||
import React from 'react';
|
import React, { useCallback } from 'react';
|
||||||
|
|
||||||
const NUM_COLUMNS = 12;
|
const NUM_COLUMNS = 12;
|
||||||
|
|
||||||
type Control = React.ReactElement | null;
|
type Control = React.ReactElement | null;
|
||||||
|
|
||||||
export default function ControlRow({ controls }: { controls: Control[] }) {
|
export default function ControlRow({ controls }: { controls: Control[] }) {
|
||||||
// ColorMapControl renders null and should not be counted
|
const isHiddenControl = useCallback(
|
||||||
|
(control: Control) =>
|
||||||
|
control?.props.type === 'HiddenControl' ||
|
||||||
|
control?.props.isVisible === false,
|
||||||
|
[],
|
||||||
|
);
|
||||||
|
// Invisible control should not be counted
|
||||||
// in the columns number
|
// in the columns number
|
||||||
const countableControls = controls.filter(
|
const countableControls = controls.filter(
|
||||||
control => !['ColorMapControl'].includes(control?.props.type),
|
control => !isHiddenControl(control),
|
||||||
);
|
);
|
||||||
const colSize = NUM_COLUMNS / countableControls.length;
|
const colSize = countableControls.length
|
||||||
|
? NUM_COLUMNS / countableControls.length
|
||||||
|
: NUM_COLUMNS;
|
||||||
return (
|
return (
|
||||||
<div className="row">
|
<div className="row">
|
||||||
{controls.map((control, i) => (
|
{controls.map((control, i) => (
|
||||||
<div
|
<div
|
||||||
className={`col-lg-${colSize} col-xs-12`}
|
className={`col-lg-${colSize} col-xs-12`}
|
||||||
style={{
|
style={{
|
||||||
display: control?.props.type === 'HiddenControl' ? 'none' : 'block',
|
display: isHiddenControl(control) ? 'none' : 'block',
|
||||||
}}
|
}}
|
||||||
key={i}
|
key={i}
|
||||||
>
|
>
|
||||||
|
Loading…
Reference in New Issue
Block a user