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 2020/06/22 06:32:32 UTC
[GitHub] [incubator-superset] ktmud commented on a change in pull request #10114: fix: dashboard filter scope bug
ktmud commented on a change in pull request #10114:
URL: https://github.com/apache/incubator-superset/pull/10114#discussion_r443269971
##########
File path: superset-frontend/spec/javascripts/dashboard/util/getFilterScopeFromNodesTree_spec.js
##########
@@ -157,6 +159,107 @@ describe('getFilterScopeFromNodesTree', () => {
},
];
+ // this is an other commonly used layout for dashboard:
Review comment:
typo: "an other" -> "another"
##########
File path: superset-frontend/spec/javascripts/dashboard/util/getFilterScopeFromNodesTree_spec.js
##########
@@ -157,6 +159,107 @@ describe('getFilterScopeFromNodesTree', () => {
},
];
+ // this is an other commonly used layout for dashboard:
Review comment:
Would be nice if there's a short note that visualizes the dashboard layout:
```js
// - filter0
// - tab1
// - filter1
// - chart1
// - tab2
// - filter2
// - chart2
// - tab2-1
// - chart3
// - chart4
// - tab2-2
// - chart5
// - chart6
//
// The test case expects it's possible to apply filter2 to filter0
```
On a separate note, `chartsImmune` should probably be refactored to `chartsInScope` so the data structure can better align with the UI . Negative condition sometimes is difficult for a human brain to comprehend. ESLint has a rule for it: https://eslint.org/docs/rules/no-negated-condition . (Not that we need to do it soon, of course, but something to keep in mind when it's time to revamp the filters)
----------------------------------------------------------------
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