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.

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

Jump to bottom

chore: Improves the native filters UI/UX - iteration 6 #14932

Merged
merged 1 commit into from Jun 2, 2021
Merged

chore: Improves the native filters UI/UX - iteration 6 #14932

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 18 additions & 2 deletions superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTabs.tsx
View file
Open in desktop
Expand Up @@ -24,7 +24,7 @@ import Icon from 'src/components/Icon';
import { FilterRemoval } from './types';
import { REMOVAL_DELAY_SECS } from './utils';

export const FILTER_WIDTH = 200;
export const FILTER_WIDTH = 180;

export const StyledSpan = styled.span`
cursor: pointer;
Expand Down Expand Up @@ -115,6 +115,7 @@ const FilterTabsContainer = styled(LineEditableTabs)`
padding-right: ${theme.gridUnit}px;
padding-bottom: ${theme.gridUnit * 3}px;
padding-left: ${theme.gridUnit * 3}px;
width: 270px;
}

// extra selector specificity:
Expand Down Expand Up @@ -185,6 +186,8 @@ const FilterTabs: FC<FilterTabsProps> = ({
children,
}) => (
<FilterTabsContainer
id="native-filters-tabs"
type="editable-card"
tabPosition="left"
onChange={onChange}
activeKey={currentFilterId}
Expand All @@ -193,7 +196,20 @@ const FilterTabs: FC<FilterTabsProps> = ({
tabBarExtraContent={{
left: <StyledHeader>{t('Filters')}</StyledHeader>,
right: (
<StyledAddFilterBox onClick={() => onEdit('', 'add')}>
<StyledAddFilterBox
onClick={() => {
onEdit('', 'add');
setTimeout(() => {
const element = document.getElementById('native-filters-tabs');
if (element) {
const navList = element.getElementsByClassName(
'ant-tabs-nav-list',
)[0];
navList.scrollTop = navList.scrollHeight;
}
}, 0);
}}
>
<PlusOutlined />{' '}
<span data-test="add-filter-button" aria-label="Add filter">
{t('Add filter')}
Expand Down
172 changes: 99 additions & 73 deletions ...board/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
View file
Open in desktop
Expand Up @@ -229,6 +229,7 @@ const FILTERS_WITH_ADHOC_FILTERS = ['filter_select', 'filter_range'];

const BASIC_CONTROL_ITEMS = ['enableEmptyFilter', 'multiSelect'];

// TODO: Rename the filter plugins and remove this mapping
const FILTER_TYPE_NAME_MAPPING = {
[t('Select filter')]: t('Value'),
[t('Range filter')]: t('Numerical range'),
Expand All @@ -254,7 +255,12 @@ const FiltersConfigForm = (
ref: React.RefObject<any>,
) => {
const [metrics, setMetrics] = useState<Metric[]>([]);
const [activeTabKey, setActiveTabKey] = useState<string | undefined>();
const [activeTabKey, setActiveTabKey] = useState<string>(
FilterTabs.configuration.key,
);
const [activeFilterPanelKey, setActiveFilterPanelKey] = useState<
string | string[]
>(FilterPanels.basic.key);
const [hasDefaultValue, setHasDefaultValue] = useState(
!!filterToEdit?.defaultDataMask?.filterState?.value,
);
Expand Down Expand Up @@ -410,10 +416,6 @@ const FiltersConfigForm = (
[],
);

if (removed) {
return <RemovedFilter onClick={() => restoreFilter(filterId)} />;
}

const parentFilterOptions = parentFilters.map(filter => ({
value: filter.id,
label: filter.title,
Expand All @@ -423,6 +425,14 @@ const FiltersConfigForm = (
({ value }) => value === filterToEdit?.cascadeParentIds[0],
);

const hasParentFilter = !!parentFilter;

const hasPreFilter =
!!filterToEdit?.adhoc_filters || !!filterToEdit?.time_range;

const hasSorting =
typeof filterToEdit?.controlValues?.sortAscending === 'boolean';
Comment on lines +428 to +434
Copy link
Member

Choose a reason for hiding this comment

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

Not important for iteration 6, but it could be a good idea to add TODOs here, to make it easier to formulate a plan for automating this based on control properties in the future.


const showDefaultValue = !hasDataset || (!isDataDirty && hasFilledDataset);

const controlItems = formFilter
Expand All @@ -447,9 +457,27 @@ const FiltersConfigForm = (
forceUpdate();
};

let hasCheckedAdvancedControl = hasParentFilter || hasPreFilter || hasSorting;
if (!hasCheckedAdvancedControl) {
hasCheckedAdvancedControl = Object.keys(controlItems)
.filter(key => !BASIC_CONTROL_ITEMS.includes(key))
.some(key => controlItems[key].checked);
}

useEffect(() => {
const activeFilterPanelKey = [FilterPanels.basic.key];
if (hasCheckedAdvancedControl) {
activeFilterPanelKey.push(FilterPanels.advanced.key);
}
setActiveFilterPanelKey(activeFilterPanelKey);
}, [hasCheckedAdvancedControl]);

if (removed) {
return <RemovedFilter onClick={() => restoreFilter(filterId)} />;
}

return (
<StyledTabs
defaultActiveKey={FilterTabs.configuration.key}
activeKey={activeTabKey}
onChange={activeKey => setActiveTabKey(activeKey)}
centered
Expand Down Expand Up @@ -559,7 +587,8 @@ const FiltersConfigForm = (
</StyledRowContainer>
)}
<StyledCollapse
defaultActiveKey={FilterPanels.basic.key}
activeKey={activeFilterPanelKey}
onChange={key => setActiveFilterPanelKey(key)}
expandIconPosition="right"
>
<Collapse.Panel
Expand Down Expand Up @@ -630,7 +659,7 @@ const FiltersConfigForm = (
</CollapsibleControl>
{Object.keys(controlItems)
.filter(key => BASIC_CONTROL_ITEMS.includes(key))
.map(key => controlItems[key])}
.map(key => controlItems[key].element)}
<StyledRowFormItem
name={['filters', filterId, 'isInstant']}
initialValue={filterToEdit?.isInstant || false}
Expand All @@ -650,7 +679,7 @@ const FiltersConfigForm = (
{isCascadingFilter && (
<CollapsibleControl
title={t('Filter is hierarchical')}
checked={!!parentFilter}
checked={hasParentFilter}
onChange={checked => {
if (checked) {
// execute after render
Expand Down Expand Up @@ -687,13 +716,11 @@ const FiltersConfigForm = (
)}
{Object.keys(controlItems)
.filter(key => !BASIC_CONTROL_ITEMS.includes(key))
.map(key => controlItems[key])}
.map(key => controlItems[key].element)}
{hasDataset && hasAdditionalFilters && (
<CollapsibleControl
title={t('Pre-filter available values')}
checked={
!!filterToEdit?.adhoc_filters || !!filterToEdit?.time_range
}
checked={hasPreFilter}
onChange={checked => {
if (checked) {
// execute after render
Expand Down Expand Up @@ -757,72 +784,71 @@ const FiltersConfigForm = (
</StyledRowFormItem>
</CollapsibleControl>
)}
<CollapsibleControl
title={t('Sort filter values')}
onChange={checked => onSortChanged(checked || undefined)}
checked={
typeof filterToEdit?.controlValues?.sortAscending ===
'boolean'
}
>
<StyledRowContainer>
<StyledFormItem
name={[
'filters',
filterId,
'controlValues',
'sortAscending',
]}
initialValue={filterToEdit?.controlValues?.sortAscending}
label={<StyledLabel>{t('Sort type')}</StyledLabel>}
>
<Select
form={form}
filterId={filterId}
name="sortAscending"
options={[
{
value: true,
label: t('Sort ascending'),
},
{
value: false,
label: t('Sort descending'),
},
]}
onChange={({ value }: { value: boolean }) =>
onSortChanged(value)
}
/>
</StyledFormItem>
{hasMetrics && (
{formFilter?.filterType !== 'filter_range' && (
<CollapsibleControl
title={t('Sort filter values')}
onChange={checked => onSortChanged(checked || undefined)}
checked={hasSorting}
>
<StyledRowContainer>
<StyledFormItem
name={['filters', filterId, 'sortMetric']}
initialValue={filterToEdit?.sortMetric}
label={<StyledLabel>{t('Sort Metric')}</StyledLabel>}
data-test="field-input"
name={[
'filters',
filterId,
'controlValues',
'sortAscending',
]}
initialValue={filterToEdit?.controlValues?.sortAscending}
label={<StyledLabel>{t('Sort type')}</StyledLabel>}
>
<SelectControl
<Select
form={form}
filterId={filterId}
name="sortMetric"
options={metrics.map((metric: Metric) => ({
value: metric.metric_name,
label: metric.verbose_name ?? metric.metric_name,
}))}
onChange={(value: string | null): void => {
if (value !== undefined) {
setNativeFilterFieldValues(form, filterId, {
sortMetric: value,
});
forceUpdate();
}
}}
name="sortAscending"
options={[
{
value: true,
label: t('Sort ascending'),
},
{
value: false,
label: t('Sort descending'),
},
]}
onChange={({ value }: { value: boolean }) =>
onSortChanged(value)
}
/>
</StyledFormItem>
)}
</StyledRowContainer>
</CollapsibleControl>
{hasMetrics && (
<StyledFormItem
name={['filters', filterId, 'sortMetric']}
initialValue={filterToEdit?.sortMetric}
label={<StyledLabel>{t('Sort Metric')}</StyledLabel>}
data-test="field-input"
>
<SelectControl
form={form}
filterId={filterId}
name="sortMetric"
options={metrics.map((metric: Metric) => ({
value: metric.metric_name,
label: metric.verbose_name ?? metric.metric_name,
}))}
onChange={(value: string | null): void => {
if (value !== undefined) {
setNativeFilterFieldValues(form, filterId, {
sortMetric: value,
});
forceUpdate();
}
}}
/>
</StyledFormItem>
)}
</StyledRowContainer>
</CollapsibleControl>
)}
</Collapse.Panel>
)}
</StyledCollapse>
Expand Down
8 changes: 6 additions & 2 deletions ...components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
View file
Open in desktop
Expand Up @@ -83,8 +83,12 @@ beforeEach(() => {
jest.clearAllMocks();
});

function renderControlItems(controlItemsMap: {}): any {
return render(<>{Object.values(controlItemsMap).map(value => value)}</>);
function renderControlItems(
controlItemsMap: ReturnType<typeof getControlItemsMap>,
) {
return render(
<>{Object.values(controlItemsMap).map(value => value.element)}</>,
);
}

test('Should render null when has no "formFilter"', () => {
Expand Down
15 changes: 9 additions & 6 deletions ...oard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx
View file
Open in desktop
Expand Up @@ -50,7 +50,10 @@ export default function getControlItemsMap({
const controlPanelRegistry = getChartControlPanelRegistry();
const controlItems =
getControlItems(controlPanelRegistry.get(filterType)) ?? [];
const map = {};
const map: Record<
string,
{ element: React.ReactNode; checked: boolean }
> = {};

controlItems
.filter(
Expand All @@ -59,6 +62,9 @@ export default function getControlItemsMap({
controlItem.name !== 'sortAscending',
)
.forEach(controlItem => {
const initialValue =
filterToEdit?.controlValues?.[controlItem.name] ??
controlItem?.config?.default;
const element = (
<Tooltip
key={controlItem.name}
Expand All @@ -72,10 +78,7 @@ export default function getControlItemsMap({
<StyledRowFormItem
key={controlItem.name}
name={['filters', filterId, 'controlValues', controlItem.name]}
initialValue={
filterToEdit?.controlValues?.[controlItem.name] ??
controlItem?.config?.default
}
initialValue={initialValue}
valuePropName="checked"
colon={false}
>
Expand Down Expand Up @@ -104,7 +107,7 @@ export default function getControlItemsMap({
</StyledRowFormItem>
</Tooltip>
);
map[controlItem.name] = element;
map[controlItem.name] = { element, checked: initialValue };
});
return map;
}
6 changes: 5 additions & 1 deletion ...frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
View file
Open in desktop
Expand Up @@ -183,7 +183,11 @@ export function FiltersConfigModal({
filterIds
.filter(filterId => filterId !== id && !removedFilters[filterId])
.filter(filterId =>
CASCADING_FILTERS.includes(formValues.filters[filterId]?.filterType),
CASCADING_FILTERS.includes(
formValues.filters[filterId]
? formValues.filters[filterId].filterType
: filterConfigMap[filterId]?.filterType,
),
)
.map(id => ({
id,
Expand Down