Skip to content
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.

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

feat(dashboard): Add description to the native filter #17025

Merged
merged 11 commits into from
Oct 27, 2021
2 changes: 2 additions & 0 deletions superset-frontend/spec/fixtures/mockNativeFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export const nativeFilters: NativeFiltersState = {
inverseSelection: false,
},
type: NativeFilterType.NATIVE_FILTER,
description: '',
},
'NATIVE_FILTER-x9QPw0so1': {
id: 'NATIVE_FILTER-x9QPw0so1',
Expand Down Expand Up @@ -81,6 +82,7 @@ export const nativeFilters: NativeFiltersState = {
inverseSelection: false,
},
type: NativeFilterType.NATIVE_FILTER,
description: '2 letter code',
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export const nativeFiltersInfo: NativeFiltersState = {
isRequired: false,
},
type: NativeFilterType.NATIVE_FILTER,
description: 'test description',
},
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,27 @@
* under the License.
*/
import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import { fireEvent, render, screen } from 'spec/helpers/testing-library';
import { mockStore } from 'spec/fixtures/mockStore';
import { Provider } from 'react-redux';
import { nativeFiltersInfo } from 'spec/javascripts/dashboard/fixtures/mockNativeFilters';
import CascadeFilterControl, { CascadeFilterControlProps } from '.';

const mockedProps = {
const createProps = (defaultsId = nativeFiltersInfo.filters.DefaultsID) => ({
filter: {
...nativeFiltersInfo.filters.DefaultsID,
...defaultsId,
cascadeChildren: [
{
...nativeFiltersInfo.filters.DefaultsID,
...defaultsId,
name: 'test child filter 1',
cascadeChildren: [],
},
{
...nativeFiltersInfo.filters.DefaultsID,
...defaultsId,
name: 'test child filter 2',
cascadeChildren: [
{
...nativeFiltersInfo.filters.DefaultsID,
...defaultsId,
name: 'test child of a child filter',
cascadeChildren: [],
},
Expand All @@ -46,7 +46,7 @@ const mockedProps = {
],
},
onFilterSelectionChange: jest.fn(),
};
});

const setup = (props: CascadeFilterControlProps) => (
<Provider store={mockStore}>
Expand All @@ -55,22 +55,41 @@ const setup = (props: CascadeFilterControlProps) => (
);

test('should render', () => {
const { container } = render(setup(mockedProps));
const { container } = render(setup(createProps()));
expect(container).toBeInTheDocument();
});

test('should render the filter name', () => {
render(setup(mockedProps));
render(setup(createProps()));
expect(screen.getByText('test')).toBeInTheDocument();
});

test('should render the children filter names', () => {
render(setup(mockedProps));
render(setup(createProps()));
expect(screen.getByText('test child filter 1')).toBeInTheDocument();
expect(screen.getByText('test child filter 2')).toBeInTheDocument();
});

test('should render the child of a child filter name', () => {
render(setup(mockedProps));
render(setup(createProps()));
expect(screen.getByText('test child of a child filter')).toBeInTheDocument();
});

test('should render tooltip if description is not empty', async () => {
render(setup(createProps()));
expect(screen.getByText('test')).toBeInTheDocument();
const toolTip = screen.getByText('test')?.parentElement?.querySelector('i');
expect(toolTip).not.toBe(null);
fireEvent.mouseOver(toolTip as HTMLElement);
expect(await screen.findByText('test description')).toBeInTheDocument();
});

test('should not render tooltip if description is empty', () => {
render(
setup(
createProps({ ...nativeFiltersInfo.filters.DefaultsID, description: '' }),
),
);
const toolTip = screen.getByText('test')?.parentElement?.querySelector('i');
expect(toolTip).toBe(null);
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
*/
import React, { useMemo } from 'react';
import { styled } from '@superset-ui/core';
import { Form, FormItem } from 'src/components/Form';
import { FormItem as StyledFormItem, Form } from 'src/components/Form';
import { Tooltip } from 'src/components/Tooltip';
import { theme as supersetTheme } from 'src/preamble';
michael-s-molina marked this conversation as resolved.
Show resolved Hide resolved
import { checkIsMissingRequiredValue } from '../utils';
import FilterValue from './FilterValue';
import { FilterProps } from './types';
import { checkIsMissingRequiredValue } from '../utils';

const StyledIcon = styled.div`
position: absolute;
Expand Down Expand Up @@ -50,8 +52,59 @@ const StyledFilterControlContainer = styled(Form)`
width: 100%;
padding-right: ${({ theme }) => theme.gridUnit * 11}px;
}
.ant-form-item-tooltip {
margin-bottom: ${({ theme }) => theme.gridUnit}px;
}
`;

const FormItem = styled(StyledFormItem)`
.ant-form-item-label {
label.ant-form-item-required:not(.ant-form-item-required-mark-optional) {
&::after {
display: none;
}
}
}
`;

const ToolTipContainer = styled.div`
font-size: ${({ theme }) => theme.typography.sizes.m}px;
display: flex;
`;

const RequiredFieldIndicator = () => (
<span
css={{
color: supersetTheme.colors.error.base,
michael-s-molina marked this conversation as resolved.
Show resolved Hide resolved
fontSize: `${supersetTheme.typography.sizes.s}px`,
paddingLeft: `${supersetTheme.gridUnit}px`,
michael-s-molina marked this conversation as resolved.
Show resolved Hide resolved
}}
>
*
</span>
);

const DescriptoinToolTip = ({ description }: { description: string }) => (
michael-s-molina marked this conversation as resolved.
Show resolved Hide resolved
<ToolTipContainer>
<Tooltip
title={description}
placement="right"
overlayInnerStyle={{
display: '-webkit-box',
overflow: 'hidden',
WebkitLineClamp: 20,
WebkitBoxOrient: 'vertical',
textOverflow: 'ellipsis',
}}
>
<i
className="fa fa-info-circle text-muted"
css={{ paddingLeft: `${supersetTheme.gridUnit}px`, cursor: 'pointer' }}
michael-s-molina marked this conversation as resolved.
Show resolved Hide resolved
/>
</Tooltip>
</ToolTipContainer>
);

const FilterControl: React.FC<FilterProps> = ({
dataMaskSelected,
filter,
Expand All @@ -66,17 +119,22 @@ const FilterControl: React.FC<FilterProps> = ({
filter,
filter.dataMask?.filterState,
);
const isRequired = !!filter.controlValues?.enableEmptyFilter;

const label = useMemo(
() => (
<StyledFilterControlTitleBox>
<StyledFilterControlTitle data-test="filter-control-name">
{name}
</StyledFilterControlTitle>
{isRequired && <RequiredFieldIndicator />}
{filter.description && filter.description.trim() && (
michael-s-molina marked this conversation as resolved.
Show resolved Hide resolved
<DescriptoinToolTip description={filter.description} />
michael-s-molina marked this conversation as resolved.
Show resolved Hide resolved
)}
<StyledIcon data-test="filter-icon">{icon}</StyledIcon>
</StyledFilterControlTitleBox>
),
[icon, name],
[name, isRequired, filter.description, icon],
);

return (
Expand All @@ -97,4 +155,5 @@ const FilterControl: React.FC<FilterProps> = ({
</StyledFilterControlContainer>
);
};

export default React.memo(FilterControl);
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
import {
ColumnMeta,
InfoTooltipWithTrigger,
Metric,
} from '@superset-ui/chart-controls';
import {
AdhocFilter,
Behavior,
Expand All @@ -26,15 +31,11 @@ import {
JsonResponse,
styled,
SupersetApiError,
t,
SupersetClient,
t,
} from '@superset-ui/core';
import {
ColumnMeta,
InfoTooltipWithTrigger,
Metric,
} from '@superset-ui/chart-controls';
import { FormInstance } from 'antd/lib/form';
import { isEmpty, isEqual } from 'lodash';
import React, {
forwardRef,
useCallback,
Expand All @@ -44,53 +45,55 @@ import React, {
useState,
} from 'react';
import { useSelector } from 'react-redux';
import { isEqual, isEmpty } from 'lodash';
import { FormItem } from 'src/components/Form';
import { Input } from 'src/common/components';
import { getChartDataRequest } from 'src/chart/chartAction';
import { Input, TextArea } from 'src/common/components';
import { Select } from 'src/components';
import { cacheWrapper } from 'src/utils/cacheWrapper';
import AdhocFilterControl from 'src/explore/components/controls/FilterControl/AdhocFilterControl';
import DateFilterControl from 'src/explore/components/controls/DateFilterControl';
import { addDangerToast } from 'src/components/MessageToasts/actions';
import { ClientErrorObject } from 'src/utils/getClientErrorObject';
import Collapse from 'src/components/Collapse';
import { getChartDataRequest } from 'src/chart/chartAction';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
import { waitForAsyncData } from 'src/middleware/asyncEvent';
import Tabs from 'src/components/Tabs';
import BasicErrorAlert from 'src/components/ErrorMessage/BasicErrorAlert';
import { FormItem } from 'src/components/Form';
import Icons from 'src/components/Icons';
import { Tooltip } from 'src/components/Tooltip';
import Loading from 'src/components/Loading';
import { addDangerToast } from 'src/components/MessageToasts/actions';
import { Radio } from 'src/components/Radio';
import BasicErrorAlert from 'src/components/ErrorMessage/BasicErrorAlert';
import Tabs from 'src/components/Tabs';
import { Tooltip } from 'src/components/Tooltip';
import {
Chart,
ChartsState,
DatasourcesState,
RootState,
} from 'src/dashboard/types';
import Loading from 'src/components/Loading';
import DateFilterControl from 'src/explore/components/controls/DateFilterControl';
import AdhocFilterControl from 'src/explore/components/controls/FilterControl/AdhocFilterControl';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
import { waitForAsyncData } from 'src/middleware/asyncEvent';
import { cacheWrapper } from 'src/utils/cacheWrapper';
import { ClientErrorObject } from 'src/utils/getClientErrorObject';
import {
Filter,
NativeFilterType,
} from 'src/dashboard/components/nativeFilters/types';
import { getFormData } from 'src/dashboard/components/nativeFilters/utils';
import {
CASCADING_FILTERS,
getFiltersConfigModalTestId,
} from '../FiltersConfigModal';
import { FilterRemoval, NativeFiltersForm } from '../types';
import { CollapsibleControl } from './CollapsibleControl';
import { ColumnSelect } from './ColumnSelect';
import { NativeFiltersForm, FilterRemoval } from '../types';
import DatasetSelect from './DatasetSelect';
import DefaultValue from './DefaultValue';
import FilterScope from './FilterScope/FilterScope';
import getControlItemsMap from './getControlItemsMap';
import RemovedFilter from './RemovedFilter';
import { useBackendFormUpdate, useDefaultValue } from './state';
import {
FILTER_SUPPORTED_TYPES,
hasTemporalColumns,
mostUsedDataset,
setNativeFilterFieldValues,
useForceUpdate,
mostUsedDataset,
} from './utils';
import { useBackendFormUpdate, useDefaultValue } from './state';
import { getFormData } from '../../utils';
import { Filter, NativeFilterType } from '../../types';
import getControlItemsMap from './getControlItemsMap';
import FilterScope from './FilterScope/FilterScope';
import RemovedFilter from './RemovedFilter';
import DefaultValue from './DefaultValue';
import { CollapsibleControl } from './CollapsibleControl';
import {
CASCADING_FILTERS,
getFiltersConfigModalTestId,
} from '../FiltersConfigModal';
import DatasetSelect from './DatasetSelect';

const TabPane = styled(Tabs.TabPane)`
padding: ${({ theme }) => theme.gridUnit * 4}px 0px;
Expand Down Expand Up @@ -966,6 +969,13 @@ const FiltersConfigForm = (
{Object.keys(controlItems)
.filter(key => BASIC_CONTROL_ITEMS.includes(key))
.map(key => controlItems[key].element)}
<StyledFormItem
name={['filters', filterId, 'description']}
initialValue={filterToEdit?.description}
label={<StyledLabel>{t('Description')}</StyledLabel>}
>
<TextArea />
</StyledFormItem>
</Collapse.Panel>
{hasAdvancedSection && (
<Collapse.Panel
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const filterMock: Filter = {
targets: [{}],
controlValues: {},
type: NativeFilterType.NATIVE_FILTER,
description: '',
};

const createProps: () => ControlItemsProps = () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export interface NativeFiltersFormItem {
time_range?: string;
granularity_sqla?: string;
type: NativeFilterType;
description: string;
hierarchicalFilter?: boolean;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ export const createHandleSave = (
scope: formInputs.scope,
sortMetric: formInputs.sortMetric,
type: formInputs.type,
description: formInputs.description || '',
};
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export interface Filter {
tabsInScope?: string[];
chartsInScope?: number[];
type: NativeFilterType;
description: string;
}

export type FilterConfiguration = Filter[];
Expand Down