diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx index edd6e30b02fc..ec5056831531 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/index.tsx @@ -34,6 +34,7 @@ * control interface. */ import React from 'react'; +import { isEmpty } from 'lodash'; import { FeatureFlag, t, @@ -43,7 +44,6 @@ import { SequentialScheme, legacyValidateInteger, validateNonEmpty, - JsonArray, ComparisionType, } from '@superset-ui/core'; @@ -352,7 +352,7 @@ const order_desc: SharedControlConfig<'CheckboxControl'> = { visibility: ({ controls }) => Boolean( controls?.timeseries_limit_metric.value && - (controls?.timeseries_limit_metric.value as JsonArray).length, + !isEmpty(controls?.timeseries_limit_metric.value), ), }; diff --git a/superset-frontend/src/explore/components/Control.test.tsx b/superset-frontend/src/explore/components/Control.test.tsx new file mode 100644 index 000000000000..3921d4374663 --- /dev/null +++ b/superset-frontend/src/explore/components/Control.test.tsx @@ -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 = {}) => ( + + + +); + +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); + }); +}); diff --git a/superset-frontend/src/explore/components/Control.tsx b/superset-frontend/src/explore/components/Control.tsx index a8ade357da72..804c3d6b10da 100644 --- a/superset-frontend/src/explore/components/Control.tsx +++ b/superset-frontend/src/explore/components/Control.tsx @@ -16,12 +16,14 @@ * specific language governing permissions and limitations * under the License. */ -import React, { ReactNode, useCallback, useState } from 'react'; +import React, { ReactNode, useCallback, useState, useEffect } from 'react'; +import { isEqual } from 'lodash'; import { ControlType, ControlComponentProps as BaseControlComponentProps, } from '@superset-ui/chart-controls'; import { styled, JsonValue, QueryFormData } from '@superset-ui/core'; +import { usePrevious } from 'src/hooks/usePrevious'; import ErrorBoundary from 'src/components/ErrorBoundary'; import { ExploreActions } from 'src/explore/actions/exploreActions'; import controlMap from './controls'; @@ -42,6 +44,8 @@ export type ControlProps = { validationErrors?: any[]; hidden?: boolean; renderTrigger?: boolean; + default?: JsonValue; + isVisible?: boolean; }; /** @@ -60,15 +64,36 @@ export default function Control(props: ControlProps) { name, type, hidden, + isVisible, } = props; const [hovered, setHovered] = useState(false); + const wasVisible = usePrevious(isVisible); const onChange = useCallback( (value: any, errors: any[]) => setControlValue(name, value, errors), [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; if (!ControlComponent) { diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx index 650e5f00c0c1..832d3552f6eb 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx @@ -283,16 +283,17 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { validationErrors?: any[]; }; - // if visibility check says the config is not visible, don't render it - if (visibility && !visibility.call(config, props, controlData)) { - return null; - } + const isVisible = visibility + ? visibility.call(config, props, controlData) + : undefined; + return ( ); diff --git a/superset-frontend/src/explore/components/ControlRow.test.tsx b/superset-frontend/src/explore/components/ControlRow.test.tsx index 638b6772d7e4..0b5707867654 100644 --- a/superset-frontend/src/explore/components/ControlRow.test.tsx +++ b/superset-frontend/src/explore/components/ControlRow.test.tsx @@ -17,20 +17,51 @@ * under the License. */ 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'; +const MockControl = (props: { + children: React.ReactElement; + type?: string; + isVisible?: boolean; +}) =>
{props.children}
; describe('ControlSetRow', () => { it('renders a single row with one element', () => { - const { getAllByText } = render( - My Control 1

]} />, - ); - expect(getAllByText('My Control 1').length).toBe(1); + render(My Control 1

]} />); + expect(screen.getAllByText('My Control 1').length).toBe(1); }); it('renders a single row with two elements', () => { - const { getAllByText } = render( + render( My Control 1

,

My Control 2

]} />, ); - 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( + My Control 1

, + +

My Control 2

+
, + ]} + />, + ); + expect(screen.getAllByText(/My Control/)).toHaveLength(2); + }); + + it('renders a single row with one elements if is invisible', () => { + render( + My Control 1

, + +

My Control 2

+
, + ]} + />, + ); + expect(screen.getAllByText(/My Control/)).toHaveLength(2); }); }); diff --git a/superset-frontend/src/explore/components/ControlRow.tsx b/superset-frontend/src/explore/components/ControlRow.tsx index 4a1dfd578984..5721b5de2893 100644 --- a/superset-frontend/src/explore/components/ControlRow.tsx +++ b/superset-frontend/src/explore/components/ControlRow.tsx @@ -16,26 +16,34 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useCallback } from 'react'; const NUM_COLUMNS = 12; type Control = React.ReactElement | null; 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 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 (
{controls.map((control, i) => (