feat(native-filters): Re-arrange controls in FilterBar (#18784)

* feat(native-filters): Re-arrange controls in FilterBar

* Typo fix

* Add tests

* Fix tests

* Address PR reviews

* Add 1px bottom padding to icon

* Fix test

* Tiny refactor

* Change buttons background

* Add hover state to Clear all button

* Fix Clear All button logic so it clears all selections

* Remove redundant useEffect

* Set zindex higher than loading icon

* Fix scrolling issue
This commit is contained in:
Kamil Gabryjelski 2022-02-22 17:45:53 +01:00 committed by GitHub
parent 4c16586067
commit 9d5c0505cf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 308 additions and 174 deletions

View File

@ -343,7 +343,7 @@ export const nativeFilters = {
collapse: dataTestLocator('filter-bar__collapse-button'),
filterName: dataTestLocator('filter-control-name'),
filterContent: '.ant-select-selection-item',
createFilterButton: dataTestLocator('create-filter'),
createFilterButton: dataTestLocator('filter-bar__create-filter'),
timeRangeFilterContent: dataTestLocator('time-range-trigger'),
},
createFilterButton: dataTestLocator('filter-bar__create-filter'),

View File

@ -202,14 +202,16 @@ export default function Button(props: ButtonProps) {
},
'&[disabled], &[disabled]:hover': {
color: grayscale.base,
backgroundColor: backgroundColorDisabled,
borderColor: borderColorDisabled,
backgroundColor:
buttonStyle === 'link' ? 'transparent' : backgroundColorDisabled,
borderColor:
buttonStyle === 'link' ? 'transparent' : borderColorDisabled,
},
marginLeft: 0,
'& + .superset-button': {
marginLeft: theme.gridUnit * 2,
},
'& :first-of-type': {
'& > :first-of-type': {
marginRight: firstChildMargin,
},
}}

View File

@ -51,19 +51,20 @@ import FilterBar from 'src/dashboard/components/nativeFilters/FilterBar';
import Loading from 'src/components/Loading';
import { EmptyStateBig } from 'src/components/EmptyState';
import { useUiConfig } from 'src/components/UiConfigContext';
import {
BUILDER_SIDEPANEL_WIDTH,
CLOSED_FILTER_BAR_WIDTH,
FILTER_BAR_HEADER_HEIGHT,
FILTER_BAR_TABS_HEIGHT,
HEADER_HEIGHT,
MAIN_HEADER_HEIGHT,
OPEN_FILTER_BAR_WIDTH,
TABS_HEIGHT,
} from 'src/dashboard/constants';
import { shouldFocusTabs, getRootLevelTabsComponent } from './utils';
import DashboardContainer from './DashboardContainer';
import { useNativeFilters } from './state';
const MAIN_HEADER_HEIGHT = 53;
const TABS_HEIGHT = 50;
const HEADER_HEIGHT = 72;
const CLOSED_FILTER_BAR_WIDTH = 32;
const OPEN_FILTER_BAR_WIDTH = 260;
const FILTER_BAR_HEADER_HEIGHT = 80;
const FILTER_BAR_TABS_HEIGHT = 46;
const BUILDER_SIDEPANEL_WIDTH = 374;
type DashboardBuilderProps = {};
const StyledDiv = styled.div`

View File

@ -0,0 +1,79 @@
/**
* 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 React from 'react';
import userEvent from '@testing-library/user-event';
import { render, screen } from 'spec/helpers/testing-library';
import { ActionButtons } from './index';
const createProps = () => ({
onApply: jest.fn(),
onClearAll: jest.fn(),
dataMaskSelected: {
DefaultsID: {
filterState: {
value: null,
},
},
},
dataMaskApplied: {
DefaultsID: {
id: 'DefaultsID',
filterState: {
value: null,
},
},
},
isApplyDisabled: false,
});
test('should render the "Apply" button', () => {
const mockedProps = createProps();
render(<ActionButtons {...mockedProps} />, { useRedux: true });
expect(screen.getByText('Apply filters')).toBeInTheDocument();
expect(screen.getByText('Apply filters').parentElement).toBeEnabled();
});
test('should render the "Clear all" button as disabled', () => {
const mockedProps = createProps();
render(<ActionButtons {...mockedProps} />, { useRedux: true });
const clearBtn = screen.getByText('Clear all');
expect(clearBtn.parentElement).toBeDisabled();
});
test('should render the "Apply" button as disabled', () => {
const mockedProps = createProps();
const applyDisabledProps = {
...mockedProps,
isApplyDisabled: true,
};
render(<ActionButtons {...applyDisabledProps} />, { useRedux: true });
const applyBtn = screen.getByText('Apply filters');
expect(applyBtn.parentElement).toBeDisabled();
userEvent.click(applyBtn);
expect(mockedProps.onApply).not.toHaveBeenCalled();
});
test('should apply', () => {
const mockedProps = createProps();
render(<ActionButtons {...mockedProps} />, { useRedux: true });
const applyBtn = screen.getByText('Apply filters');
expect(mockedProps.onApply).not.toHaveBeenCalled();
userEvent.click(applyBtn);
expect(mockedProps.onApply).toHaveBeenCalled();
});

View File

@ -0,0 +1,125 @@
/**
* 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 React, { useMemo } from 'react';
import {
css,
DataMaskState,
DataMaskStateWithId,
styled,
t,
} from '@superset-ui/core';
import Button from 'src/components/Button';
import { isNullish } from 'src/utils/common';
import { OPEN_FILTER_BAR_WIDTH } from 'src/dashboard/constants';
import { getFilterBarTestId } from '../index';
interface ActionButtonsProps {
onApply: () => void;
onClearAll: () => void;
dataMaskSelected: DataMaskState;
dataMaskApplied: DataMaskStateWithId;
isApplyDisabled: boolean;
}
const ActionButtonsContainer = styled.div`
${({ theme }) => css`
display: flex;
flex-direction: column;
align-items: center;
position: fixed;
z-index: 100;
// filter bar width minus 1px for border
width: ${OPEN_FILTER_BAR_WIDTH - 1}px;
bottom: 0;
padding: ${theme.gridUnit * 4}px;
padding-top: ${theme.gridUnit * 6}px;
background: linear-gradient(transparent, white 25%);
pointer-events: none;
& > button {
pointer-events: auto;
}
& > .filter-apply-button {
margin-bottom: ${theme.gridUnit * 3}px;
}
&& > .filter-clear-all-button {
color: ${theme.colors.grayscale.base};
margin-left: 0;
&:hover {
color: ${theme.colors.primary.dark1};
}
&[disabled],
&[disabled]:hover {
color: ${theme.colors.grayscale.light1};
}
}
`};
`;
export const ActionButtons = ({
onApply,
onClearAll,
dataMaskApplied,
dataMaskSelected,
isApplyDisabled,
}: ActionButtonsProps) => {
const isClearAllEnabled = useMemo(
() =>
Object.values(dataMaskApplied).some(
filter =>
!isNullish(dataMaskSelected[filter.id]?.filterState?.value) ||
(!dataMaskSelected[filter.id] &&
!isNullish(filter.filterState?.value)),
),
[dataMaskApplied, dataMaskSelected],
);
return (
<ActionButtonsContainer>
<Button
disabled={isApplyDisabled}
buttonStyle="primary"
htmlType="submit"
className="filter-apply-button"
onClick={onApply}
{...getFilterBarTestId('apply-button')}
>
{t('Apply filters')}
</Button>
<Button
disabled={!isClearAllEnabled}
buttonStyle="link"
buttonSize="small"
className="filter-clear-all-button"
onClick={onClearAll}
{...getFilterBarTestId('clear-button')}
>
{t('Clear all')}
</Button>
</ActionButtonsContainer>
);
};

View File

@ -241,9 +241,9 @@ describe('FilterBar', () => {
expect(screen.getByText('Clear all')).toBeInTheDocument();
});
it('should render the "Apply" option', () => {
it('should render the "Apply filters" option', () => {
renderWrapper();
expect(screen.getByText('Apply')).toBeInTheDocument();
expect(screen.getByText('Apply filters')).toBeInTheDocument();
});
it('should render the collapse icon', () => {

View File

@ -42,6 +42,9 @@ import { buildCascadeFiltersTree } from './utils';
const Wrapper = styled.div`
padding: ${({ theme }) => theme.gridUnit * 4}px;
// 108px padding to make room for buttons with position: absolute
padding-bottom: ${({ theme }) => theme.gridUnit * 27}px;
&:hover {
cursor: pointer;
}

View File

@ -48,6 +48,8 @@ const FilterSetsWrapper = styled.div`
align-items: center;
justify-content: center;
grid-template-columns: 1fr;
// 108px padding to make room for buttons with position: absolute
padding-bottom: ${({ theme }) => theme.gridUnit * 27}px;
& button.superset-button {
margin-left: 0;

View File

@ -23,24 +23,6 @@ import Header from './index';
const createProps = () => ({
toggleFiltersBar: jest.fn(),
onApply: jest.fn(),
onClearAll: jest.fn(),
dataMaskSelected: {
DefaultsID: {
filterState: {
value: null,
},
},
},
dataMaskApplied: {
DefaultsID: {
id: 'DefaultsID',
filterState: {
value: null,
},
},
},
isApplyDisabled: false,
});
test('should render', () => {
@ -55,48 +37,6 @@ test('should render the "Filters" heading', () => {
expect(screen.getByText('Filters')).toBeInTheDocument();
});
test('should render the "Clear all" option', () => {
const mockedProps = createProps();
render(<Header {...mockedProps} />, { useRedux: true });
expect(screen.getByText('Clear all')).toBeInTheDocument();
});
test('should render the "Apply" button', () => {
const mockedProps = createProps();
render(<Header {...mockedProps} />, { useRedux: true });
expect(screen.getByText('Apply')).toBeInTheDocument();
expect(screen.getByText('Apply').parentElement).toBeEnabled();
});
test('should render the "Clear all" button as disabled', () => {
const mockedProps = createProps();
render(<Header {...mockedProps} />, { useRedux: true });
const clearBtn = screen.getByText('Clear all');
expect(clearBtn.parentElement).toBeDisabled();
});
test('should render the "Apply" button as disabled', () => {
const mockedProps = createProps();
const applyDisabledProps = {
...mockedProps,
isApplyDisabled: true,
};
render(<Header {...applyDisabledProps} />, { useRedux: true });
const applyBtn = screen.getByText('Apply');
expect(applyBtn.parentElement).toBeDisabled();
userEvent.click(applyBtn);
expect(mockedProps.onApply).not.toHaveBeenCalled();
});
test('should apply', () => {
const mockedProps = createProps();
render(<Header {...mockedProps} />, { useRedux: true });
const applyBtn = screen.getByText('Apply');
expect(mockedProps.onApply).not.toHaveBeenCalled();
userEvent.click(applyBtn);
expect(mockedProps.onApply).toHaveBeenCalled();
});
test('should render the expand button', () => {
const mockedProps = createProps();
render(<Header {...mockedProps} />, { useRedux: true });

View File

@ -17,22 +17,15 @@
* under the License.
*/
/* eslint-disable no-param-reassign */
import {
DataMaskState,
DataMaskStateWithId,
Filter,
styled,
t,
useTheme,
} from '@superset-ui/core';
import { css, Filter, styled, t, useTheme } from '@superset-ui/core';
import React, { FC } from 'react';
import Icons from 'src/components/Icons';
import Button from 'src/components/Button';
import { useSelector } from 'react-redux';
import FilterConfigurationLink from 'src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink';
import { useFilters } from 'src/dashboard/components/nativeFilters/FilterBar/state';
import { RootState } from 'src/dashboard/types';
import { getFilterBarTestId } from '..';
import { RootState } from '../../../../types';
const TitleArea = styled.h4`
display: flex;
@ -46,20 +39,6 @@ const TitleArea = styled.h4`
}
`;
const ActionButtons = styled.div`
display: grid;
flex-direction: row;
justify-content: center;
align-items: center;
grid-gap: 10px;
grid-template-columns: 1fr 1fr;
${({ theme }) => `padding: 0 ${theme.gridUnit * 2}px`};
.btn {
flex: 1;
}
`;
const HeaderButton = styled(Button)`
padding: 0;
`;
@ -71,21 +50,28 @@ const Wrapper = styled.div`
type HeaderProps = {
toggleFiltersBar: (arg0: boolean) => void;
onApply: () => void;
onClearAll: () => void;
dataMaskSelected: DataMaskState;
dataMaskApplied: DataMaskStateWithId;
isApplyDisabled: boolean;
};
const Header: FC<HeaderProps> = ({
onApply,
onClearAll,
isApplyDisabled,
dataMaskSelected,
dataMaskApplied,
toggleFiltersBar,
}) => {
const AddFiltersButtonContainer = styled.div`
${({ theme }) => css`
margin-top: ${theme.gridUnit * 2}px;
& button > [role='img']:first-of-type {
margin-right: ${theme.gridUnit}px;
line-height: 0;
}
span[role='img'] {
padding-bottom: 1px;
}
.ant-btn > .anticon + span {
margin-left: 0;
}
`}
`;
const Header: FC<HeaderProps> = ({ toggleFiltersBar }) => {
const theme = useTheme();
const filters = useFilters();
const filterValues = Object.values<Filter>(filters);
@ -96,27 +82,10 @@ const Header: FC<HeaderProps> = ({
({ dashboardInfo }) => dashboardInfo.id,
);
const isClearAllDisabled = Object.values(dataMaskApplied).every(
filter =>
dataMaskSelected[filter.id]?.filterState?.value === null ||
(!dataMaskSelected[filter.id] && filter.filterState?.value === null),
);
return (
<Wrapper>
<TitleArea>
<span>{t('Filters')}</span>
{canEdit && (
<FilterConfigurationLink
dashboardId={dashboardId}
createNewOnOpen={filterValues.length === 0}
>
<Icons.Edit
data-test="create-filter"
iconColor={theme.colors.grayscale.base}
/>
</FilterConfigurationLink>
)}
<HeaderButton
{...getFilterBarTestId('collapse-button')}
buttonStyle="link"
@ -126,29 +95,16 @@ const Header: FC<HeaderProps> = ({
<Icons.Expand iconColor={theme.colors.grayscale.base} />
</HeaderButton>
</TitleArea>
<ActionButtons className="filter-action-buttons">
<Button
disabled={isClearAllDisabled}
buttonStyle="tertiary"
buttonSize="small"
className="filter-clear-all-button"
onClick={onClearAll}
{...getFilterBarTestId('clear-button')}
>
{t('Clear all')}
</Button>
<Button
disabled={isApplyDisabled}
buttonStyle="primary"
htmlType="submit"
buttonSize="small"
className="filter-apply-button"
onClick={onApply}
{...getFilterBarTestId('apply-button')}
>
{t('Apply')}
</Button>
</ActionButtons>
{canEdit && (
<AddFiltersButtonContainer>
<FilterConfigurationLink
dashboardId={dashboardId}
createNewOnOpen={filterValues.length === 0}
>
<Icons.PlusSmall /> {t('Add/Edit Filters')}
</FilterConfigurationLink>
</AddFiltersButtonContainer>
)}
</Wrapper>
);
};

View File

@ -61,6 +61,7 @@ import EditSection from './FilterSets/EditSection';
import Header from './Header';
import FilterControls from './FilterControls/FilterControls';
import { RootState } from '../../../types';
import { ActionButtons } from './ActionButtons';
export const FILTER_BAR_TEST_ID = 'filter-bar';
export const getFilterBarTestId = testWithId(FILTER_BAR_TEST_ID);
@ -168,7 +169,7 @@ const publishDataMask = debounce(
const { search } = location;
const previousParams = new URLSearchParams(search);
const newParams = new URLSearchParams();
let dataMaskKey = '';
let dataMaskKey: string;
previousParams.forEach((value, key) => {
if (key !== URL_PARAMS.nativeFilters.name) {
newParams.append(key, value);
@ -254,7 +255,7 @@ const FilterBar: React.FC<FiltersBarProps> = ({
};
});
},
[dataMaskSelected, dispatch, setDataMaskSelected, tab],
[dataMaskSelected, dispatch, setDataMaskSelected],
);
useEffect(() => {
@ -284,10 +285,6 @@ const FilterBar: React.FC<FiltersBarProps> = ({
}
}, [JSON.stringify(filters), JSON.stringify(previousFilters)]);
useEffect(() => {
setDataMaskSelected(() => dataMaskApplied);
}, [JSON.stringify(dataMaskApplied), setDataMaskSelected]);
const dataMaskAppliedText = JSON.stringify(dataMaskApplied);
useEffect(() => {
publishDataMask(history, dashboardId, updateKey, dataMaskApplied, tabId);
@ -309,9 +306,14 @@ const FilterBar: React.FC<FiltersBarProps> = ({
filterIds.forEach(filterId => {
if (dataMaskSelected[filterId]) {
dispatch(clearDataMask(filterId));
setDataMaskSelected(draft => {
if (draft[filterId].filterState?.value !== undefined) {
draft[filterId].filterState!.value = undefined;
}
});
}
});
}, [dataMaskSelected, dispatch]);
}, [dataMaskSelected, dispatch, setDataMaskSelected]);
const openFiltersBar = useCallback(
() => toggleFiltersBar(true),
@ -350,14 +352,7 @@ const FilterBar: React.FC<FiltersBarProps> = ({
<StyledFilterIcon {...getFilterBarTestId('filter-icon')} iconSize="l" />
</CollapsedBar>
<Bar className={cx({ open: filtersOpen })} width={width}>
<Header
toggleFiltersBar={toggleFiltersBar}
onApply={handleApply}
onClearAll={handleClearAll}
isApplyDisabled={isApplyDisabled}
dataMaskSelected={dataMaskSelected}
dataMaskApplied={dataMaskApplied}
/>
<Header toggleFiltersBar={toggleFiltersBar} />
{!isInitialized ? (
<div css={{ height }}>
<Loading />
@ -384,11 +379,26 @@ const FilterBar: React.FC<FiltersBarProps> = ({
filterSetId={editFilterSetId}
/>
)}
<FilterControls
dataMaskSelected={dataMaskSelected}
directPathToChild={directPathToChild}
onFilterSelectionChange={handleFilterSelectionChange}
/>
{filterValues.length === 0 ? (
<FilterBarEmptyStateContainer>
<EmptyStateSmall
title={t('No filters are currently added')}
image="filter.svg"
description={
canEdit &&
t(
'Click the button above to add a filter to the dashboard',
)
}
/>
</FilterBarEmptyStateContainer>
) : (
<FilterControls
dataMaskSelected={dataMaskSelected}
directPathToChild={directPathToChild}
onFilterSelectionChange={handleFilterSelectionChange}
/>
)}
</Tabs.TabPane>
<Tabs.TabPane
disabled={!!editFilterSetId}
@ -416,9 +426,7 @@ const FilterBar: React.FC<FiltersBarProps> = ({
image="filter.svg"
description={
canEdit &&
t(
'Click the pencil icon above to add a filter to the dashboard',
)
t('Click the button above to add a filter to the dashboard')
}
/>
</FilterBarEmptyStateContainer>
@ -431,6 +439,13 @@ const FilterBar: React.FC<FiltersBarProps> = ({
)}
</div>
)}
<ActionButtons
onApply={handleApply}
onClearAll={handleClearAll}
dataMaskSelected={dataMaskSelected}
dataMaskApplied={dataMaskApplied}
isApplyDisabled={isApplyDisabled}
/>
</Bar>
</BarWrapper>
);

View File

@ -33,3 +33,12 @@ export const PLACEHOLDER_DATASOURCE: Datasource = {
main_dttm_col: '',
description: '',
};
export const MAIN_HEADER_HEIGHT = 53;
export const TABS_HEIGHT = 50;
export const HEADER_HEIGHT = 72;
export const CLOSED_FILTER_BAR_WIDTH = 32;
export const OPEN_FILTER_BAR_WIDTH = 260;
export const FILTER_BAR_HEADER_HEIGHT = 80;
export const FILTER_BAR_TABS_HEIGHT = 46;
export const BUILDER_SIDEPANEL_WIDTH = 374;

View File

@ -144,3 +144,5 @@ export const detectOS = () => {
return 'Unknown OS';
};
export const isNullish = value => value === null || value === undefined;