You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/04/26 03:41:43 UTC
[GitHub] [superset] villebro commented on a change in pull request #14334: fix(native-filters): Fix indicators
villebro commented on a change in pull request #14334:
URL: https://github.com/apache/superset/pull/14334#discussion_r619952434
##########
File path: superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts
##########
@@ -220,57 +221,62 @@ export const selectNativeIndicatorsForChart = (
return IndicatorStatus.Unset;
};
- const nativeFilterIndicators = Object.values(nativeFilters.filters).map(
- nativeFilter => {
- const isAffectedByScope = getTreeCheckedItems(
- nativeFilter.scope,
- dashboardLayout,
- ).some(
- layoutItem => dashboardLayout[layoutItem]?.meta?.chartId === chartId,
- );
- const column = nativeFilter.targets[0]?.column?.name;
- let value = dataMask[nativeFilter.id]?.filterState?.value ?? null;
- if (!Array.isArray(value) && value !== null) {
- value = [value];
- }
- return {
- column,
- name: nativeFilter.name,
- path: [nativeFilter.id],
- status: getStatus({ value, isAffectedByScope, column }),
- value,
- };
- },
- );
+ let nativeFilterIndicators: any = [];
+ if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)) {
+ nativeFilterIndicators = Object.values(nativeFilters.filters).map(
+ nativeFilter => {
+ const isAffectedByScope = getTreeCheckedItems(
+ nativeFilter.scope,
+ dashboardLayout,
+ ).some(
+ layoutItem => dashboardLayout[layoutItem]?.meta?.chartId === chartId,
+ );
+ const column = nativeFilter.targets[0]?.column?.name;
+ let value = dataMask[nativeFilter.id]?.filterState?.value ?? null;
+ if (!Array.isArray(value) && value !== null) {
+ value = [value];
+ }
+ return {
+ column,
+ name: nativeFilter.name,
+ path: [nativeFilter.id],
+ status: getStatus({ value, isAffectedByScope, column }),
+ value,
+ };
+ },
+ );
+ }
- const crossFilterIndicators = Object.values(chartConfiguration).map(
- chartConfig => {
- const scope = chartConfig?.crossFilters?.scope;
- const isAffectedByScope = getTreeCheckedItems(
- scope,
- dashboardLayout,
- ).some(
- layoutItem => dashboardLayout[layoutItem]?.meta?.chartId === chartId,
- );
+ let crossFilterIndicators: any = [];
+ if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
+ crossFilterIndicators = Object.values(chartConfiguration).map(
Review comment:
Right now it shows all charts that have cross filter behavior as potentially emitting charts, which is not the case unless they've been enabled. I'd almost vote for adding a `.filter(filter => filter.status === IndicatorStatus.CrossFilterApplied)` at the end of this `map` to remove any cross filters that aren't applied for now, as listing them among unset native filters seems confusing.
##########
File path: superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts
##########
@@ -220,57 +221,62 @@ export const selectNativeIndicatorsForChart = (
return IndicatorStatus.Unset;
};
- const nativeFilterIndicators = Object.values(nativeFilters.filters).map(
- nativeFilter => {
- const isAffectedByScope = getTreeCheckedItems(
- nativeFilter.scope,
- dashboardLayout,
- ).some(
- layoutItem => dashboardLayout[layoutItem]?.meta?.chartId === chartId,
- );
- const column = nativeFilter.targets[0]?.column?.name;
- let value = dataMask[nativeFilter.id]?.filterState?.value ?? null;
- if (!Array.isArray(value) && value !== null) {
- value = [value];
- }
- return {
- column,
- name: nativeFilter.name,
- path: [nativeFilter.id],
- status: getStatus({ value, isAffectedByScope, column }),
- value,
- };
- },
- );
+ let nativeFilterIndicators: any = [];
+ if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)) {
Review comment:
I think this is correct - filter box indicators are handled separately by `selectIndicatorsForChart`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org