fix(dashboard): cross filter chart highlight when filters badge icon clicked (#16233)

* fix(dashboard): cross filter chart highlight when filters badge icon pressed

* Fix tests

* Fix tests

* break out label logic

Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>
This commit is contained in:
Kamil Gabryjelski 2021-08-13 10:02:28 +02:00 committed by GitHub
parent 5d3d6b6eae
commit 517a678cd7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 54 additions and 26 deletions

View File

@ -96,6 +96,7 @@ test('Should render "appliedCrossFilterIndicators"', () => {
<DetailsPanel {...props}> <DetailsPanel {...props}>
<div data-test="details-panel-content">Content</div> <div data-test="details-panel-content">Content</div>
</DetailsPanel>, </DetailsPanel>,
{ useRedux: true },
); );
userEvent.click(screen.getByTestId('details-panel-content')); userEvent.click(screen.getByTestId('details-panel-content'));
@ -129,6 +130,7 @@ test('Should render "appliedIndicators"', () => {
<DetailsPanel {...props}> <DetailsPanel {...props}>
<div data-test="details-panel-content">Content</div> <div data-test="details-panel-content">Content</div>
</DetailsPanel>, </DetailsPanel>,
{ useRedux: true },
); );
userEvent.click(screen.getByTestId('details-panel-content')); userEvent.click(screen.getByTestId('details-panel-content'));
@ -160,6 +162,7 @@ test('Should render "incompatibleIndicators"', () => {
<DetailsPanel {...props}> <DetailsPanel {...props}>
<div data-test="details-panel-content">Content</div> <div data-test="details-panel-content">Content</div>
</DetailsPanel>, </DetailsPanel>,
{ useRedux: true },
); );
userEvent.click(screen.getByTestId('details-panel-content')); userEvent.click(screen.getByTestId('details-panel-content'));
@ -193,6 +196,7 @@ test('Should render "unsetIndicators"', () => {
<DetailsPanel {...props}> <DetailsPanel {...props}>
<div data-test="details-panel-content">Content</div> <div data-test="details-panel-content">Content</div>
</DetailsPanel>, </DetailsPanel>,
{ useRedux: true },
); );
userEvent.click(screen.getByTestId('details-panel-content')); userEvent.click(screen.getByTestId('details-panel-content'));
@ -227,6 +231,7 @@ test('Should render empty', () => {
<DetailsPanel {...props}> <DetailsPanel {...props}>
<div data-test="details-panel-content">Content</div> <div data-test="details-panel-content">Content</div>
</DetailsPanel>, </DetailsPanel>,
{ useRedux: true },
); );
expect(screen.getByTestId('details-panel-content')).toBeInTheDocument(); expect(screen.getByTestId('details-panel-content')).toBeInTheDocument();

View File

@ -17,6 +17,7 @@
* under the License. * under the License.
*/ */
import React, { useEffect, useState } from 'react'; import React, { useEffect, useState } from 'react';
import { useSelector } from 'react-redux';
import { Global, css } from '@emotion/react'; import { Global, css } from '@emotion/react';
import { t, useTheme } from '@superset-ui/core'; import { t, useTheme } from '@superset-ui/core';
import { import {
@ -35,6 +36,7 @@ import {
} from 'src/dashboard/components/FiltersBadge/Styles'; } from 'src/dashboard/components/FiltersBadge/Styles';
import { Indicator } from 'src/dashboard/components/FiltersBadge/selectors'; import { Indicator } from 'src/dashboard/components/FiltersBadge/selectors';
import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator'; import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator';
import { RootState } from 'src/dashboard/types';
export interface DetailsPanelProps { export interface DetailsPanelProps {
appliedCrossFilterIndicators: Indicator[]; appliedCrossFilterIndicators: Indicator[];
@ -55,6 +57,9 @@ const DetailsPanelPopover = ({
}: DetailsPanelProps) => { }: DetailsPanelProps) => {
const [visible, setVisible] = useState(false); const [visible, setVisible] = useState(false);
const theme = useTheme(); const theme = useTheme();
const activeTabs = useSelector<RootState>(
state => state.dashboardState?.activeTabs,
);
// we don't need to clean up useEffect, setting { once: true } removes the event listener after handle function is called // we don't need to clean up useEffect, setting { once: true } removes the event listener after handle function is called
useEffect(() => { useEffect(() => {
@ -65,6 +70,11 @@ const DetailsPanelPopover = ({
} }
}, [visible]); }, [visible]);
// if tabs change, popover doesn't close automatically
useEffect(() => {
setVisible(false);
}, [activeTabs]);
const getDefaultActivePanel = () => { const getDefaultActivePanel = () => {
const result = []; const result = [];
if (appliedCrossFilterIndicators.length) { if (appliedCrossFilterIndicators.length) {

View File

@ -20,7 +20,12 @@ import { NO_TIME_RANGE, TIME_FILTER_MAP } from 'src/explore/constants';
import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters';
import { ChartConfiguration, Filters } from 'src/dashboard/reducers/types'; import { ChartConfiguration, Filters } from 'src/dashboard/reducers/types';
import { DataMaskStateWithId, DataMaskType } from 'src/dataMask/types'; import { DataMaskStateWithId, DataMaskType } from 'src/dataMask/types';
import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; import {
ensureIsArray,
FeatureFlag,
FilterState,
isFeatureEnabled,
} from '@superset-ui/core';
import { Layout } from '../../types'; import { Layout } from '../../types';
import { getTreeCheckedItems } from '../nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils'; import { getTreeCheckedItems } from '../nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils';
@ -50,6 +55,16 @@ type Filter = {
datasourceId: string; datasourceId: string;
}; };
const extractLabel = (filter?: FilterState): string | null => {
if (filter?.label) {
return filter.label;
}
if (filter?.value) {
return ensureIsArray(filter?.value).join(', ');
}
return null;
};
const selectIndicatorValue = ( const selectIndicatorValue = (
columnKey: string, columnKey: string,
filter: Filter, filter: Filter,
@ -182,16 +197,16 @@ export const selectNativeIndicatorsForChart = (
const rejectedColumns = getRejectedColumns(chart); const rejectedColumns = getRejectedColumns(chart);
const getStatus = ({ const getStatus = ({
value, label,
column, column,
type = DataMaskType.NativeFilters, type = DataMaskType.NativeFilters,
}: { }: {
value: any; label: string | null;
column?: string; column?: string;
type?: DataMaskType; type?: DataMaskType;
}): IndicatorStatus => { }): IndicatorStatus => {
// a filter is only considered unset if it's value is null // a filter is only considered unset if it's value is null
const hasValue = value !== null; const hasValue = label !== null;
if (type === DataMaskType.CrossFilters && hasValue) { if (type === DataMaskType.CrossFilters && hasValue) {
return IndicatorStatus.CrossFilterApplied; return IndicatorStatus.CrossFilterApplied;
} }
@ -220,19 +235,14 @@ export const selectNativeIndicatorsForChart = (
) )
.map(nativeFilter => { .map(nativeFilter => {
const column = nativeFilter.targets[0]?.column?.name; const column = nativeFilter.targets[0]?.column?.name;
let value = const filterState = dataMask[nativeFilter.id]?.filterState;
dataMask[nativeFilter.id]?.filterState?.label ?? const label = extractLabel(filterState);
dataMask[nativeFilter.id]?.filterState?.value ??
null;
if (!Array.isArray(value) && value !== null) {
value = [value];
}
return { return {
column, column,
name: nativeFilter.name, name: nativeFilter.name,
path: [nativeFilter.id], path: [nativeFilter.id],
status: getStatus({ value, column }), status: getStatus({ label, column }),
value, value: label,
}; };
}); });
} }
@ -249,23 +259,26 @@ export const selectNativeIndicatorsForChart = (
), ),
) )
.map(chartConfig => { .map(chartConfig => {
let value = const filterState = dataMask[chartConfig.id]?.filterState;
dataMask[chartConfig.id]?.filterState?.label ?? const label = extractLabel(filterState);
dataMask[chartConfig.id]?.filterState?.value ?? const filtersState = filterState?.filters;
null; const column = filtersState && Object.keys(filtersState)[0];
if (!Array.isArray(value) && value !== null) {
value = [value]; const dashboardLayoutItem = Object.values(dashboardLayout).find(
} layoutItem => layoutItem?.meta?.chartId === chartConfig.id,
);
return { return {
name: Object.values(dashboardLayout).find( column,
layoutItem => layoutItem?.meta?.chartId === chartConfig.id, name: dashboardLayoutItem?.meta?.sliceName as string,
)?.meta?.sliceName as string, path: [
path: [`${chartConfig.id}`], ...(dashboardLayoutItem?.parents ?? []),
dashboardLayoutItem?.id,
],
status: getStatus({ status: getStatus({
value, label,
type: DataMaskType.CrossFilters, type: DataMaskType.CrossFilters,
}), }),
value, value: label,
}; };
}) })
.filter(filter => filter.status === IndicatorStatus.CrossFilterApplied); .filter(filter => filter.status === IndicatorStatus.CrossFilterApplied);