kgabryje commented on a change in pull request #15424:
URL: https://github.com/apache/superset/pull/15424#discussion_r660472374
##########
File path: superset-frontend/src/dashboard/components/FiltersBadge/index.tsx
##########
@@ -16,29 +16,132 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React from 'react';
+import React, { useCallback, useMemo } from 'react';
import cx from 'classnames';
import Icon from 'src/components/Icon';
import Icons from 'src/components/Icons';
+import { uniqWith } from 'lodash';
+import { useDispatch, useSelector } from 'react-redux';
import DetailsPanelPopover from './DetailsPanel';
import { Pill } from './Styles';
-import { Indicator } from './selectors';
+import {
+ Indicator,
+ IndicatorStatus,
+ selectIndicatorsForChart,
+ selectNativeIndicatorsForChart,
+} from './selectors';
+import { setDirectPathToChild } from '../../actions/dashboardState';
+import {
+ ChartsState,
+ DashboardInfo,
+ DashboardLayout,
+ RootState,
+} from '../../types';
+import { Filters } from '../../reducers/types';
+import { DataMaskStateWithId } from '../../../dataMask/types';
export interface FiltersBadgeProps {
- appliedCrossFilterIndicators: Indicator[];
- appliedIndicators: Indicator[];
- unsetIndicators: Indicator[];
- incompatibleIndicators: Indicator[];
- onHighlightFilterSource: (path: string[]) => void;
+ chartId: number;
}
-const FiltersBadge = ({
- appliedCrossFilterIndicators,
- appliedIndicators,
- unsetIndicators,
- incompatibleIndicators,
- onHighlightFilterSource,
-}: FiltersBadgeProps) => {
+const sortByStatus = (indicators: Indicator[]): Indicator[] => {
+ const statuses = [
+ IndicatorStatus.Applied,
+ IndicatorStatus.Unset,
+ IndicatorStatus.Incompatible,
+ ];
+ return indicators.sort(
+ (a, b) =>
+ statuses.indexOf(a.status as IndicatorStatus) -
+ statuses.indexOf(b.status as IndicatorStatus),
+ );
+};
+
+export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
+ const dispatch = useDispatch();
+ const datasources = useSelector<RootState, any>(state => state.datasources);
+ const dashboardFilters = useSelector<RootState, any>(
+ state => state.dashboardFilters,
+ );
+ const nativeFilters = useSelector<RootState, Filters>(
+ state => state.nativeFilters?.filters,
+ );
+ const dashboardInfo = useSelector<RootState, DashboardInfo>(
+ state => state.dashboardInfo,
+ );
+ const charts = useSelector<RootState, ChartsState>(state => state.charts);
+ const present = useSelector<RootState, DashboardLayout>(
+ state => state.dashboardLayout.present,
+ );
+ const dataMask = useSelector<RootState, DataMaskStateWithId>(
+ state => state.dataMask,
+ );
+
+ const onHighlightFilterSource = useCallback(
+ (path: string[]) => {
+ dispatch(setDirectPathToChild(path));
+ },
+ [dispatch],
+ );
+
+ const chart = charts[chartId];
+ const dashboardIndicators = useMemo(
+ () =>
+ selectIndicatorsForChart(chartId, dashboardFilters, datasources, chart),
+ [
+ chartId,
+ JSON.stringify(chart),
Review comment:
It turned out that it wasn't necessary here, but we do use that trick
throughout our codebase. ESLint complains about that, but I think that we can
live with that if we must use complex objects in the dependency array
##########
File path: superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts
##########
@@ -178,14 +174,15 @@ export const selectIndicatorsForChart = (
};
export const selectNativeIndicatorsForChart = (
- nativeFilters: NativeFiltersState,
+ nativeFilters: Filters,
dataMask: DataMaskStateWithId,
chartId: number,
- charts: any,
+ chart: any,
dashboardLayout: Layout,
chartConfiguration: ChartConfiguration = {},
): Indicator[] => {
- const chart = charts[chartId];
+ // no indicators before chart is rendered
+ if (chart.chartStatus !== 'rendered') return [];
Review comment:
👍
##########
File path: superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts
##########
@@ -147,11 +144,10 @@ export const selectIndicatorsForChart = (
chartId: number,
filters: { [key: number]: Filter },
datasources: { [key: string]: Datasource },
- charts: any,
+ chart: any,
): Indicator[] => {
- const chart = charts[chartId];
- // no indicators if chart is loading
- if (chart.chartStatus === 'loading') return [];
+ // no indicators before chart is rendered
+ if (chart.chartStatus !== 'rendered') return [];
Review comment:
Thanks for finding this! I used a different solution so that we don't
rerun the function when chartStatus changes from `success` to `rendered`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
