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/02/10 10:59:51 UTC

[GitHub] [superset] villebro opened a new pull request #13052: fix(explore): pie chart label bugs

villebro opened a new pull request #13052:
URL: https://github.com/apache/superset/pull/13052


   ### SUMMARY
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   Bump `superset-ui` packages to version 0.17.8 and fix a the following Pie Chart bugs:
   - label sometimes disappears on hover
   - custom label type changing doesn't work
   
   Also updates some code that depends on changes to `superset-ui/core`.
   
   ### TEST PLAN
   Verify that both up and down migration works, and that pie chart works properly.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue: closes #12989
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


----------------------------------------------------------------
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

[GitHub] [superset] simcha90 commented on a change in pull request #13052: fix(explore): pie chart label bugs

Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #13052:
URL: https://github.com/apache/superset/pull/13052#discussion_r573726255



##########
File path: superset-frontend/src/explore/components/controls/VizTypeControl.jsx
##########
@@ -166,15 +166,21 @@ const VizTypeControl = props => {
   const filterString = filter.toLowerCase();
 
   const filteredTypes = DEFAULT_ORDER.filter(type => registry.has(type))
-    .filter(type => !registry.get(type).isNativeFilter)
+    .filter(type => {
+      const behaviors = registry.get(type)?.behaviors || [];
+      return behaviors.includes(Behavior.CROSS_FILTER) || !behaviors.length;
+    })
     .map(type => ({
       key: type,
       value: registry.get(type),
     }))
     .concat(
       registry
         .entries()
-        .filter(entry => !entry.value.isNativeFilter)
+        .filter(entry => {

Review comment:
       may be move it to some utils like
   ```
   const getVisibleCharts = ({ behaviours } = {}) => behaviors?.includes(Behavior.CROSS_FILTER) || !behaviors?.length;
   
   and use it like:
   
   .filter(type =>getVisibleCharts(registry.get(type)))
   and
   .filter(entry =>getVisibleCharts(entry.value))
   ```
   




----------------------------------------------------------------
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

[GitHub] [superset] villebro merged pull request #13052: fix(explore): pie chart label bugs

Posted by GitBox <gi...@apache.org>.
villebro merged pull request #13052:
URL: https://github.com/apache/superset/pull/13052


   


----------------------------------------------------------------
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

[GitHub] [superset] codecov-io commented on pull request #13052: fix(explore): pie chart label bugs

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #13052:
URL: https://github.com/apache/superset/pull/13052#issuecomment-777287399


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13052?src=pr&el=h1) Report
   > Merging [#13052](https://codecov.io/gh/apache/superset/pull/13052?src=pr&el=desc) (4fbb2a7) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `8.79%`.
   > The diff coverage is `33.33%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13052/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13052?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #13052      +/-   ##
   ==========================================
   + Coverage   53.06%   61.85%   +8.79%     
   ==========================================
     Files         489      546      +57     
     Lines       17314    20157    +2843     
     Branches     4482     5272     +790     
   ==========================================
   + Hits         9187    12469    +3282     
   + Misses       8127     7475     -652     
   - Partials        0      213     +213     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `61.85% <33.33%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13052?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ontend/src/components/ListViewCard/ImageLoader.tsx](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL0ltYWdlTG9hZGVyLnRzeA==) | `86.36% <0.00%> (+11.36%)` | :arrow_up: |
   | [...d/src/filters/components/Range/AntdRangeFilter.tsx](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9SYW5nZS9BbnRkUmFuZ2VGaWx0ZXIudHN4) | `0.00% <0.00%> (ø)` | |
   | [...set-frontend/src/filters/components/Range/index.ts](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9SYW5nZS9pbmRleC50cw==) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
   | [...src/filters/components/Select/AntdSelectFilter.tsx](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvQW50ZFNlbGVjdEZpbHRlci50c3g=) | `0.00% <ø> (-96.78%)` | :arrow_down: |
   | [...et-frontend/src/filters/components/Select/index.ts](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvaW5kZXgudHM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `71.65% <ø> (-2.66%)` | :arrow_down: |
   | [...rontend/src/views/CRUD/dashboard/DashboardCard.tsx](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGFzaGJvYXJkL0Rhc2hib2FyZENhcmQudHN4) | `75.67% <ø> (-0.33%)` | :arrow_down: |
   | [...rontend/src/views/CRUD/dashboard/DashboardList.tsx](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGFzaGJvYXJkL0Rhc2hib2FyZExpc3QudHN4) | `73.87% <0.00%> (+7.56%)` | :arrow_up: |
   | [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `55.97% <ø> (-13.00%)` | :arrow_down: |
   | [superset-frontend/src/views/CRUD/utils.tsx](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvdXRpbHMudHN4) | `61.36% <ø> (+29.31%)` | :arrow_up: |
   | ... and [468 more](https://codecov.io/gh/apache/superset/pull/13052/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13052?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13052?src=pr&el=footer). Last update [3e0681b...4fbb2a7](https://codecov.io/gh/apache/superset/pull/13052?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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

[GitHub] [superset] simcha90 commented on a change in pull request #13052: fix(explore): pie chart label bugs

Posted by GitBox <gi...@apache.org>.
simcha90 commented on a change in pull request #13052:
URL: https://github.com/apache/superset/pull/13052#discussion_r573726255



##########
File path: superset-frontend/src/explore/components/controls/VizTypeControl.jsx
##########
@@ -166,15 +166,21 @@ const VizTypeControl = props => {
   const filterString = filter.toLowerCase();
 
   const filteredTypes = DEFAULT_ORDER.filter(type => registry.has(type))
-    .filter(type => !registry.get(type).isNativeFilter)
+    .filter(type => {
+      const behaviors = registry.get(type)?.behaviors || [];
+      return behaviors.includes(Behavior.CROSS_FILTER) || !behaviors.length;
+    })
     .map(type => ({
       key: type,
       value: registry.get(type),
     }))
     .concat(
       registry
         .entries()
-        .filter(entry => !entry.value.isNativeFilter)
+        .filter(entry => {

Review comment:
       may be move it to some utils like
   ```
   getVisibleCharts = ({ behaviours } = {}) => {
         return behaviors?.includes(Behavior.CROSS_FILTER) || !behaviors?.length;
       }
   }
   and use it like:
   
   ```
   .filter(type =>getVisibleCharts(registry.get(type)))
   and
   .filter(entry =>getVisibleCharts(entry.value))
   ```
   

##########
File path: superset-frontend/src/explore/components/controls/VizTypeControl.jsx
##########
@@ -166,15 +166,21 @@ const VizTypeControl = props => {
   const filterString = filter.toLowerCase();
 
   const filteredTypes = DEFAULT_ORDER.filter(type => registry.has(type))
-    .filter(type => !registry.get(type).isNativeFilter)
+    .filter(type => {
+      const behaviors = registry.get(type)?.behaviors || [];
+      return behaviors.includes(Behavior.CROSS_FILTER) || !behaviors.length;
+    })
     .map(type => ({
       key: type,
       value: registry.get(type),
     }))
     .concat(
       registry
         .entries()
-        .filter(entry => !entry.value.isNativeFilter)
+        .filter(entry => {

Review comment:
       may be move it to some utils like
   ```
   getVisibleCharts = ({ behaviours } = {}) => {
         return behaviors?.includes(Behavior.CROSS_FILTER) || !behaviors?.length;
       }
   }
   and use it like:
   
   .filter(type =>getVisibleCharts(registry.get(type)))
   and
   .filter(entry =>getVisibleCharts(entry.value))
   ```
   

##########
File path: superset-frontend/src/explore/components/controls/VizTypeControl.jsx
##########
@@ -166,15 +166,21 @@ const VizTypeControl = props => {
   const filterString = filter.toLowerCase();
 
   const filteredTypes = DEFAULT_ORDER.filter(type => registry.has(type))
-    .filter(type => !registry.get(type).isNativeFilter)
+    .filter(type => {
+      const behaviors = registry.get(type)?.behaviors || [];
+      return behaviors.includes(Behavior.CROSS_FILTER) || !behaviors.length;
+    })
     .map(type => ({
       key: type,
       value: registry.get(type),
     }))
     .concat(
       registry
         .entries()
-        .filter(entry => !entry.value.isNativeFilter)
+        .filter(entry => {

Review comment:
       may be move it to some utils like
   ```
   getVisibleCharts = ({ behaviours } = {}) => {
         return behaviors?.includes(Behavior.CROSS_FILTER) || !behaviors?.length;
       }
   }
   
   and use it like:
   
   .filter(type =>getVisibleCharts(registry.get(type)))
   and
   .filter(entry =>getVisibleCharts(entry.value))
   ```
   




----------------------------------------------------------------
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