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(native-filters): Show/Highlight errored/focused status #15276

Merged
merged 17 commits into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions superset-frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions superset-frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
"d3-color": "^1.2.0",
"d3-scale": "^2.1.2",
"dom-to-image": "^2.6.0",
"emotion-rgba": "0.0.9",
"fontsource-fira-code": "^3.0.5",
"fontsource-inter": "^3.0.5",
"geolib": "^2.0.24",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ const FilterValue: React.FC<FilterProps> = ({
<Loading position="inline-centered" />
) : (
<SuperChart
height={20}
height={50}
width="100%"
formData={formData}
// For charts that don't have datasource we need workaround for empty placeholder
Expand All @@ -203,7 +203,6 @@ const FilterValue: React.FC<FilterProps> = ({
filterState={{
...filter.dataMask?.filterState,
validateMessage: isMissingRequiredValue && t('Value is required'),
validateStatus: isMissingRequiredValue && 'error',
}}
ownState={filter.dataMask?.ownState}
enableNoResults={metadata?.enableNoResults}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
SetDataMaskHook,
SuperChart,
AppSection,
t,
} from '@superset-ui/core';
import { FormInstance } from 'antd/lib/form';
import Loading from 'src/components/Loading';
Expand Down Expand Up @@ -56,7 +57,10 @@ const DefaultValue: FC<DefaultValueProps> = ({
setLoading(true);
}
}, [hasDataset, queriesData]);

const value = formFilter.defaultDataMask?.filterState.value;
const isMissingRequiredValue =
(value === null || value === undefined) &&
formFilter?.controlValues?.enableEmptyFilter;
return loading ? (
<Loading position="inline-centered" />
) : (
Expand All @@ -73,7 +77,10 @@ const DefaultValue: FC<DefaultValueProps> = ({
chartType={formFilter?.filterType}
hooks={{ setDataMask }}
enableNoResults={enableNoResults}
filterState={formFilter.defaultDataMask?.filterState}
filterState={{
...formFilter.defaultDataMask?.filterState,
validateMessage: isMissingRequiredValue && t('Value is required'),
}}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -749,9 +749,7 @@ const FiltersConfigForm = (
if (hasValue) {
return Promise.resolve();
}
return Promise.reject(
new Error(t('Default value is required')),
);
return Promise.reject();
},
},
]}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,13 @@ test('renders a numerical range filter type', () => {
expect(screen.getByText(FILTER_NAME_REGEX)).toBeInTheDocument();
expect(screen.getByText(DATASET_REGEX)).toBeInTheDocument();
expect(screen.getByText(COLUMN_REGEX)).toBeInTheDocument();
expect(screen.getByText(REQUIRED_REGEX)).toBeInTheDocument();

expect(getCheckbox(DEFAULT_VALUE_REGEX)).not.toBeChecked();
expect(getCheckbox(APPLY_INSTANTLY_REGEX)).not.toBeChecked();
expect(getCheckbox(PRE_FILTER_REGEX)).not.toBeChecked();

expect(queryCheckbox(MULTIPLE_REGEX)).not.toBeInTheDocument();
expect(queryCheckbox(REQUIRED_REGEX)).not.toBeInTheDocument();
expect(queryCheckbox(HIERARCHICAL_REGEX)).not.toBeInTheDocument();
expect(queryCheckbox(FIRST_ITEM_REGEX)).not.toBeInTheDocument();
expect(queryCheckbox(INVERSE_SELECTION_REGEX)).not.toBeInTheDocument();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,19 @@
* specific language governing permissions and limitations
* under the License.
*/
import { ensureIsArray, ExtraFormData, t, tn } from '@superset-ui/core';
import { ensureIsArray, ExtraFormData, styled, t, tn } from '@superset-ui/core';
import React, { useEffect, useState } from 'react';
import { Select } from 'src/common/components';
import { Styles, StyledSelect } from '../common';
import { PluginFilterGroupByProps } from './types';
import FormItem from '../../../components/Form/FormItem';

const { Option } = Select;

const Error = styled.div`
color: ${({ theme }) => theme.colors.error.base};
`;

export default function PluginFilterGroupBy(props: PluginFilterGroupByProps) {
const {
data,
Expand Down Expand Up @@ -70,28 +75,36 @@ export default function PluginFilterGroupBy(props: PluginFilterGroupByProps) {
: tn('%s option', '%s options', columns.length, columns.length);
return (
<Styles height={height} width={width}>
<StyledSelect
allowClear
value={value}
placeholder={placeholderText}
mode={multiSelect ? 'multiple' : undefined}
// @ts-ignore
onChange={handleChange}
onBlur={unsetFocusedFilter}
onFocus={setFocusedFilter}
ref={inputRef}
<FormItem
validateStatus={filterState.validateMessage && 'error'}
extra={<Error>{filterState.validateMessage}</Error>}
>
{columns.map(
(row: { column_name: string; verbose_name: string | null }) => {
const { column_name: columnName, verbose_name: verboseName } = row;
return (
<Option key={columnName} value={columnName}>
{verboseName ?? columnName}
</Option>
);
},
)}
</StyledSelect>
<StyledSelect
allowClear
value={value}
placeholder={placeholderText}
mode={multiSelect ? 'multiple' : undefined}
// @ts-ignore
onChange={handleChange}
onBlur={unsetFocusedFilter}
onFocus={setFocusedFilter}
ref={inputRef}
>
{columns.map(
(row: { column_name: string; verbose_name: string | null }) => {
const {
column_name: columnName,
verbose_name: verboseName,
} = row;
return (
<Option key={columnName} value={columnName}>
{verboseName ?? columnName}
</Option>
);
},
)}
</StyledSelect>
</FormItem>
</Styles>
);
}
12 changes: 12 additions & 0 deletions superset-frontend/src/filters/components/GroupBy/controlPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ const config: ControlPanelConfig = {
},
},
],
[
{
name: 'enableEmptyFilter',
config: {
type: 'CheckboxControl',
label: t('Required'),
default: false,
renderTrigger: true,
description: t('User must select a value for this filter.'),
},
},
],
],
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,61 @@
* specific language governing permissions and limitations
* under the License.
*/
import { getNumberFormatter, NumberFormats, t } from '@superset-ui/core';
import {
getNumberFormatter,
NumberFormats,
styled,
t,
} from '@superset-ui/core';
import React, { useEffect, useState } from 'react';
import { Slider } from 'src/common/components';
import { rgba } from 'emotion-rgba';
import { PluginFilterRangeProps } from './types';
import { Styles } from '../common';
import { getRangeExtraFormData } from '../../utils';
import FormItem from '../../../components/Form/FormItem';

const Error = styled.div`
color: ${({ theme }) => theme.colors.error.base};
`;

const Wrapper = styled.div<{ validateStatus?: string }>`
border: 1px solid transparent;
&:focus {
border: 1px solid
${({ theme, validateStatus }) =>
theme.colors[validateStatus ? 'error' : 'primary'].base};
outline: 0;
box-shadow: 0 0 0 3px
${({ theme, validateStatus }) =>
rgba(theme.colors[validateStatus ? 'error' : 'primary'].base, 0.2)};
}
& .ant-slider {
& .ant-slider-track {
background-color: ${({ theme, validateStatus }) =>
validateStatus && theme.colors.error.light1};
}
& .ant-slider-handle {
border: ${({ theme, validateStatus }) =>
validateStatus && `2px solid ${theme.colors.error.light1}`};
&:focus {
box-shadow: 0 0 0 3px
${({ theme, validateStatus }) =>
rgba(theme.colors[validateStatus ? 'error' : 'primary'].base, 0.2)};
}
}
&:hover {
& .ant-slider-track {
background-color: ${({ theme, validateStatus }) =>
validateStatus && theme.colors.error.base};
}
& .ant-slider-handle {
border: ${({ theme, validateStatus }) =>
validateStatus && `2px solid ${theme.colors.error.base}`};
}
}
}
`;

export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
const {
Expand All @@ -32,15 +81,14 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
setDataMask,
setFocusedFilter,
unsetFocusedFilter,
inputRef,
filterState,
} = props;
const numberFormatter = getNumberFormatter(NumberFormats.SMART_NUMBER);

const [row] = data;
// @ts-ignore
const { min, max }: { min: number; max: number } = row;
const { groupby, defaultValue } = formData;
const { groupby, defaultValue, inputRef } = formData;
const [col = ''] = groupby || [];
const [value, setValue] = useState<[number, number]>(
defaultValue ?? [min, max],
Expand Down Expand Up @@ -111,19 +159,31 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
{Number.isNaN(Number(min)) || Number.isNaN(Number(max)) ? (
<h4>{t('Chosen non-numeric column')}</h4>
) : (
<div onMouseEnter={setFocusedFilter} onMouseLeave={unsetFocusedFilter}>
<Slider
range
min={min}
max={max}
value={value ?? [min, max]}
onAfterChange={handleAfterChange}
onChange={handleChange}
tipFormatter={value => numberFormatter(value)}
<FormItem
validateStatus={filterState.validateMessage && 'error'}
extra={<Error>{filterState.validateMessage}</Error>}
>
<Wrapper
tabIndex={-1}
ref={inputRef}
marks={marks}
/>
</div>
validateStatus={filterState.validateMessage}
onFocus={setFocusedFilter}
onBlur={unsetFocusedFilter}
onMouseEnter={setFocusedFilter}
onMouseLeave={unsetFocusedFilter}
>
<Slider
range
min={min}
max={max}
value={value ?? [min, max]}
onAfterChange={handleAfterChange}
onChange={handleChange}
tipFormatter={value => numberFormatter(value)}
marks={marks}
/>
</Wrapper>
</FormItem>
)}
</Styles>
);
Expand Down
18 changes: 18 additions & 0 deletions superset-frontend/src/filters/components/Range/controlPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,24 @@ const config: ControlPanelConfig = {
],
],
},
{
label: t('UI Configuration'),
expanded: true,
controlSetRows: [
[
{
name: 'enableEmptyFilter',
config: {
type: 'CheckboxControl',
label: t('Required'),
default: false,
renderTrigger: true,
description: t('User must select a value for this filter.'),
},
},
],
],
},
],
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
return (
<Styles height={height} width={width}>
<FormItem
validateStatus={filterState.validateStatus}
validateStatus={filterState.validateMessage && 'error'}
extra={<Error>{filterState.validateMessage}</Error>}
>
<StyledSelect
Expand Down
Loading