feat(plugin-chart-echarts): Emit cross filters for pie and boxplot #1010
Conversation
|
We would appreciate it if you could provide us with more info about this issue/pr! |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/superset/superset-ui/2o9sZBv6DJ3y2PerQhJtgaF52HYZ |
maloun96
force-pushed
the
enable-emitting-filter
branch
from
f2a8f3c
to
1217c7c
Compare
Codecov Report
@@ Coverage Diff @@
## master #1010 +/- ##
==========================================
- Coverage 27.95% 27.90% -0.06%
==========================================
Files 414 417 +3
Lines 8535 8602 +67
Branches 1218 1237 +19
==========================================
+ Hits 2386 2400 +14
- Misses 5992 6043 +51
- Partials 157 159 +2
Continue to review full report at Codecov.
|
|
I'd like to better understand the motivation for this change. There is currently a project to enable cross-filtering of charts on Dashboards, but this PR doesn't look like it is compatible with that feature. Will cross-filtering support your use case? |
| (values: string[]) => { | ||
| const groupbyValues = values.map(value => labelMap[value]); | ||
|
|
||
| setDataMask({ |
There was a problem hiding this comment.
We need to check if emitFilter is enabled before calling the hook.
| config: { | ||
| type: 'CheckboxControl', | ||
| label: t('Enable emitting filters'), | ||
| default: false, |
There was a problem hiding this comment.
Can we pick the default value from DEFAULT_FORM_DATA?
| (values: string[]) => { | ||
| const groupbyValues = values.map(value => labelMap[value]); | ||
|
|
||
| setDataMask({ |
There was a problem hiding this comment.
Here we also need to check if emitFilter is enabled.
| currentState: { | ||
| value: groupbyValues ?? null, | ||
| }, | ||
| }, | ||
| ownFilters: { | ||
| currentState: { | ||
| selectedValues: values, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
| currentState: { | |
| value: groupbyValues ?? null, | |
| }, | |
| }, | |
| ownFilters: { | |
| currentState: { | |
| selectedValues: values, | |
| }, | |
| }, | |
| currentState: { | |
| value: groupbyValues.length ? groupbyValues : null, | |
| }, | |
| }, | |
| ownFilters: { | |
| currentState: { | |
| selectedValues: values.length ? values : null, | |
| }, | |
| }, |
| currentState: { | ||
| value: groupbyValues ?? null, | ||
| }, | ||
| }, | ||
| ownFilters: { | ||
| currentState: { | ||
| selectedValues: values, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
| currentState: { | |
| value: groupbyValues ?? null, | |
| }, | |
| }, | |
| ownFilters: { | |
| currentState: { | |
| selectedValues: values, | |
| }, | |
| }, | |
| currentState: { | |
| value: groupbyValues.length ? groupbyValues : null, | |
| }, | |
| }, | |
| ownFilters: { | |
| currentState: { | |
| selectedValues: values.length ? values : null, | |
| }, | |
| }, |
villebro
changed the title
This PR has been done in cooperation with me and Simcha, and implements the cross filter hook (I did the groundwork here which this PR builds on: #1009). So it's fully compatible with the cross filtering feature that's been worked on lately. |
…pache-superset#1010) * feat(plugin-chart-echarts): emit filter events when clicking on slice * feat(plugin-chart-echarts): enable emitting filter * clean up filter state * check eventHandlers by emitFilter flag * feat(plugin-chart-echarts): multiple choices for filter * WIP * feat(plugin-chart-echarts): select multiple values * add conditional control item for emitting filter * add cross filter for box plot chart * refactor: combine values and indexes single object * fix: eslint * Add default emitFilter for BoxPlot. Check emit filter before handle crossFilter Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>
Add checkbox control to disable/enable the emitting filter for the pie chart
Screen.Recording.2021-03-16.at.13.00.02.mov
Screen.Recording.2021-03-18.at.12.33.47.mov