villebro commented on a change in pull request #17181:
URL: https://github.com/apache/superset/pull/17181#discussion_r735279883
##########
File path: superset-frontend/src/dashboard/components/Dashboard.jsx
##########
@@ -235,14 +235,16 @@ class Dashboard extends React.PureComponent {
);
[...allKeys].forEach(filterKey => {
if (
- !currFilterKeys.includes(filterKey) &&
- appliedFilterKeys.includes(filterKey)
- ) {
// filterKey is removed?
- affectedChartIds.push(...appliedFilters[filterKey].scope);
- } else if (!appliedFilterKeys.includes(filterKey)) {
+ (!currFilterKeys.includes(filterKey) &&
+ appliedFilterKeys.includes(filterKey)) ||
// filterKey is newly added?
- affectedChartIds.push(...activeFilters[filterKey].scope);
+ !appliedFilterKeys.includes(filterKey)
+ ) {
+ // check if there are values in filter, if no, there is was added only
ownState, so no need reload other charts
+ if (Object.keys(appliedFilters[filterKey]?.values ?? []).length) {
+ affectedChartIds.push(...appliedFilters[filterKey].scope);
+ }
Review comment:
This if statement is becoming pretty loaded. I wonder if we could make
this easier to read by breaking this up into small helper functions like
`isFilterKeyRemoved(filterKey)`, `isFilterKeyNewlyAdded(filterKey)` and finally
`hasFilterKeyValues(filterKey)`? Then the if statement might be easier to parse
by other developers.
--
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]
