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/12/15 05:09:32 UTC

[GitHub] [superset] corbinrobb opened a new pull request #17758: feat(timeseries-chart): [WIP] add percentage threshold input control

corbinrobb opened a new pull request #17758:
URL: https://github.com/apache/superset/pull/17758


   ### SUMMARY
   Implementation of the feature request  in issue #17281
   
   The time-series chart controls have the option to set show values `SHOW VALUE` and stack the series `STACK SERIES`. When both are checked there is also the option to show only the series totals `ONLY TOTAL` but if you want to see all of the values in the stacks you can uncheck `ONLY TOTAL`. 
   
   When `ONLY TOTAL` is unchecked and the data that's being displayed have small or similar values, the chart labels can become cluttered and tend to sit on top of each other making them difficult to read.
   
   The enhancement is to add an input where you can enter a minimum percentage threshold that would only display labels for values at or above the threshold. 
   
   There is an input just like this over in Pie Chart controls as mentioned and shown in the issue that is referenced at the top of this PR.
   
   ---
   ### SHORT DESCRIPTION
   - Added a `TextControl` Input for the percentage threshold to the Time-series echarts controls 
   - Added logic in `transformProps` to conditionally render the labels if above that threshold
   - Added props to the Time-series Chart storybook story to show these changes
   - Currently working on tests and will pull PR out of draft mode once added
   
   ---
   ### DETAILED DESCRIPTION
   ### Control Config:
   There was already a configuration called `showValueSection` that holds the other controls mentioned so I added the new control to that.
   
   I set the visibility to only be true if show_value is true, stack is true, and only_total is false. I also set the default to be 5% matching the control in the Pie Chart plugin because it seemed reasonable.
   
   ```javascript
   const percentageThresholdControl = {
     name: 'percentage_threshold',
     config: {
       type: 'TextControl',
       label: t('Percentage threshold'),
       renderTrigger: true,
       isFloat: true,
       default: 5,
       description: t(
         'Minimum threshold in percentage points for showing labels.',
       ),
       visibility: ({ controls }: ControlPanelsContainerProps) =>
         Boolean(controls?.show_value?.value) &&
         Boolean(controls?.stack?.value) &&
         Boolean(!controls?.only_total?.value),
     },
   };
   ```
   The `showValueSection` controls are used in these Time Series Charts.
   - Area - (Time-series Area Chart)
   - Bar - (Time-series Bar Chart v2)
   - Regular - (Time-series Chart)
   - Line - (Time-series Line Chart)
   - Stepped Line - (Time-series Stepped Line)
   - Smooth Line - (Time-series Smooth Line)
   
   All of these charts appear to benefit from this and they all share the same `transformProps` which is where the logic for these controls exist.
   
   **Some Thoughts:**
   There may also be a benefit to being able to use it outside of just stacked series and it would be easy to implement. If not used with just stacked series I think it should also have the option to switch between setting a plain `value_threshold` or `percentage_threshold`. Value threshold being "show labels for all values over x". I think this could be useful because it is not always fun to guess what percentage will hide the values you want hidden and it is even less fun when the values aren't stacked.
   
   ---
   ### The logic:
   We grab the prop that is passed through `transformProps` by adding it to the list of variables deconstructed out of the `formData` prop.
   
   Create an array matching the totalStackedValues array but with the calculated threshold value instead of the total.
   
   ```javascript
       totalStackedValues.push(values);
       thresholdValues.push((percentageThreshold / 100) * values);
   ```
   
   Pass that down to `transformSeries` when it is gets called on all the series. Then within that function go on down to where the `onlyTotal` prop is being checked
   
   BEFORE:
   ```javascript
           if (!stack || !onlyTotal || isSelectedLegend) {
               return formatter(numericValue);
           }
           if (seriesIndex === showValueIndexes[dataIndex]) {
             return formatter(totalStackedValues[dataIndex]);
           }
           return '';
   ```
   
   Breaking it down super simply, this 
   - returns the series values when NOT stacked or NOT set to show only totals
   - or
   - returns the series total when necessary
   
   
   With our `thresholdValues` we can take separate out `!onlyTotal`. Then check if the value is greater than or equal to the matching threshold value and return the value accordingly.
   
   AFTER:
   ```javascript
           if (!stack || isSelectedLegend) return formatter(numericValue);
           if (!onlyTotal) {
             if (numericValue >= thresholdValues[dataIndex]) {
               return formatter(numericValue);
             }
           }
           if (seriesIndex === showValueIndexes[dataIndex]) {
             return formatter(totalStackedValues[dataIndex]);
           }
           return '';
   ```
   
   ---
   ---
   ### BEFORE
   
   <img width="1271" alt="Screen Shot 2021-12-14 at 9 02 41 PM" src="https://user-images.githubusercontent.com/31329271/146122282-6968af72-3b45-4bdc-bdaa-6a7d8e9df000.png">
   
   <img width="1271" alt="Screen Shot 2021-12-14 at 9 06 54 PM" src="https://user-images.githubusercontent.com/31329271/146122306-10d9f46d-3e91-4d8b-9d1c-e17caa2ee444.png">
   
   
   ### AFTER
   
   <img width="1271" alt="Screen Shot 2021-12-14 at 9 12 22 PM" src="https://user-images.githubusercontent.com/31329271/146122233-c2f7891d-c5b9-4930-98a8-a1401a916d71.png">
   
   <img width="1271" alt="Screen Shot 2021-12-14 at 9 11 10 PM" src="https://user-images.githubusercontent.com/31329271/146122209-5a1b7a69-ede3-4be7-ba97-2cf45674b491.png">
   
   
   
   
   ### TESTING INSTRUCTIONS
   - Open Superset and navigate to Charts
   - Open up a chart that once stacked would have tightly packed and similar values. I found that Rise & Fall of Video Game Consoles - Area Chart is a good starting place for this
   - Change the Visualization Type to be one of these: (I suggest Time-series Chart because you can switch between all)
     - Time-series Area Chart
     - Time-series Bar Chart v2
     - Time-series Chart
     - Time-series Line Chart
     - Time-series Stepped Line
     - Time-series Smooth Line
   - Make sure you have the `groupby` value set to something with a lot of values. If using Rise & Fall of Video Game Consoles you can leave it on platform or change it to publisher if you are feeling extra spicy
   - Click the `Customize` tab at the top and scroll to `Chart Options`
   - If you are using Time-series Chart I would suggest changing the Series Style to `Bar`
   - Check `SHOW VALUE` and `STACK SERIES`, then uncheck `ONLY TOTAL`
   - If all goes well you should see the `PERCENTAGE THRESHOLD` input appear with a value of 5
   
   I would turn on `DATA ZOOM` too so you can get a better look at the values. 
   
   Other than that check it out however you see fit. Maybe check out all the charts mentioned above. 
   
   I found it most useful with the bar chart but it seems to be useful on all of them
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue:
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x] 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.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

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[bot] edited a comment on pull request #17758: feat(timeseries-chart): add percentage threshold input control

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17758:
URL: https://github.com/apache/superset/pull/17758#issuecomment-999102042


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17758](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5bc821f) into [master](https://codecov.io/gh/apache/superset/commit/63d9693f21786431ba7e2ec11d6658bcd3a1f9e9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (63d9693) will **decrease** coverage by `0.60%`.
   > The diff coverage is `80.00%`.
   
   > :exclamation: Current head 5bc821f differs from pull request most recent head 4f1f03f. Consider uploading reports for the commit 4f1f03f to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17758/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17758      +/-   ##
   ==========================================
   - Coverage   67.77%   67.16%   -0.61%     
   ==========================================
     Files        1602     1609       +7     
     Lines       64151    64786     +635     
     Branches     6773     6860      +87     
   ==========================================
   + Hits        43476    43514      +38     
   - Misses      18822    19418     +596     
   - Partials     1853     1854       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `53.95% <80.00%> (-0.89%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ugins/plugin-chart-echarts/src/Timeseries/types.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90eXBlcy50cw==) | `100.00% <ø> (ø)` | |
   | [...tend/plugins/plugin-chart-echarts/src/controls.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvY29udHJvbHMudHN4) | `71.42% <50.00%> (-2.26%)` | :arrow_down: |
   | [...lugin-chart-echarts/src/Timeseries/transformers.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1lcnMudHM=) | `52.50% <83.33%> (+10.25%)` | :arrow_up: |
   | [...gin-chart-echarts/src/Timeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1Qcm9wcy50cw==) | `60.97% <100.00%> (+7.22%)` | :arrow_up: |
   | [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `81.81% <0.00%> (-8.19%)` | :arrow_down: |
   | [...src/dashboard/components/PropertiesModal/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC9pbmRleC50c3g=) | `66.03% <0.00%> (-0.85%)` | :arrow_down: |
   | [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <0.00%> (ø)` | |
   | [superset-frontend/src/components/Icons/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvbnMvaW5kZXgudHN4) | `100.00% <0.00%> (ø)` | |
   | [superset-frontend/src/common/components/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL2luZGV4LnRzeA==) | `100.00% <0.00%> (ø)` | |
   | [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `100.00% <0.00%> (ø)` | |
   | ... and [31 more](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [63d9693...4f1f03f](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@superset.apache.org

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] corbinrobb commented on a change in pull request #17758: feat(timeseries-chart): [WIP] add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
##########
@@ -49,8 +53,8 @@ describe('EchartsTimeseries tranformProps', () => {
   };
 
   it('should tranform chart props for viz', () => {
-    const chartProps = new ChartProps(chartPropsConfig);
-    expect(transformProps(chartProps)).toEqual(
+    const chartProps: unknown = new ChartProps(chartPropsConfig);

Review comment:
       I am actually not sure what is going on with these. I set it to unknown because TS said I had to before it would let me do the type assertion here `chartProps as EchartsTimeseriesChartProps`. It throws an error when I try to set the type of chartProps to EchartsTimeseriesChartProps when it is defined because their formData prop types are incompatible. This is all strange because they don't actually appear to be incompatible because EchartsTimeseriesChartProps is an extension of ChartProps and ChartProps formData prop is set to be an ambiguous object.
   
   This all appears to be done intentionally and I am not sure what I should do to get rid of the errors because I haven't found a similar test file where this isn't causing TS errors. Give me a little time to look into this more because I am not sure what to do at the moment




-- 
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: notifications-unsubscribe@superset.apache.org

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] lyndsiWilliams commented on a change in pull request #17758: feat(timeseries-chart): [WIP] add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
##########
@@ -49,8 +53,8 @@ describe('EchartsTimeseries tranformProps', () => {
   };
 
   it('should tranform chart props for viz', () => {
-    const chartProps = new ChartProps(chartPropsConfig);
-    expect(transformProps(chartProps)).toEqual(
+    const chartProps: unknown = new ChartProps(chartPropsConfig);

Review comment:
       There are [a few prop types described in this component](https://github.com/apache/superset/blob/6f8927209b80dc61ba14fc66ff94871b36a83321/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts#L32-L37), would any of those work here instead of `unknown`?




-- 
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: notifications-unsubscribe@superset.apache.org

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] corbinrobb commented on a change in pull request #17758: feat(timeseries-chart): add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
##########
@@ -140,6 +142,7 @@ export default function transformProps(
       return prev + (value as number);
     }, 0);
     totalStackedValues.push(values);
+    thresholdValues.push((percentageThreshold / 100) * values);

Review comment:
       Good point! I'll get this added




-- 
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: notifications-unsubscribe@superset.apache.org

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[bot] commented on pull request #17758: feat(timeseries-chart): add percentage threshold input control

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17758:
URL: https://github.com/apache/superset/pull/17758#issuecomment-999102042


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17758](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5bc821f) into [master](https://codecov.io/gh/apache/superset/commit/63d9693f21786431ba7e2ec11d6658bcd3a1f9e9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (63d9693) will **decrease** coverage by `0.60%`.
   > The diff coverage is `80.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17758/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17758      +/-   ##
   ==========================================
   - Coverage   67.77%   67.16%   -0.61%     
   ==========================================
     Files        1602     1609       +7     
     Lines       64151    64786     +635     
     Branches     6773     6860      +87     
   ==========================================
   + Hits        43476    43514      +38     
   - Misses      18822    19418     +596     
   - Partials     1853     1854       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `53.95% <80.00%> (-0.89%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ugins/plugin-chart-echarts/src/Timeseries/types.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90eXBlcy50cw==) | `100.00% <ø> (ø)` | |
   | [...tend/plugins/plugin-chart-echarts/src/controls.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvY29udHJvbHMudHN4) | `71.42% <50.00%> (-2.26%)` | :arrow_down: |
   | [...lugin-chart-echarts/src/Timeseries/transformers.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1lcnMudHM=) | `52.50% <83.33%> (+10.25%)` | :arrow_up: |
   | [...gin-chart-echarts/src/Timeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1Qcm9wcy50cw==) | `60.97% <100.00%> (+7.22%)` | :arrow_up: |
   | [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `81.81% <0.00%> (-8.19%)` | :arrow_down: |
   | [...src/dashboard/components/PropertiesModal/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC9pbmRleC50c3g=) | `66.03% <0.00%> (-0.85%)` | :arrow_down: |
   | [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <0.00%> (ø)` | |
   | [superset-frontend/src/components/Icons/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvbnMvaW5kZXgudHN4) | `100.00% <0.00%> (ø)` | |
   | [superset-frontend/src/common/components/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL2luZGV4LnRzeA==) | `100.00% <0.00%> (ø)` | |
   | [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `100.00% <0.00%> (ø)` | |
   | ... and [31 more](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [63d9693...5bc821f](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@superset.apache.org

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 commented on a change in pull request #17758: feat(timeseries-chart): [WIP] add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
##########
@@ -140,6 +142,7 @@ export default function transformProps(
       return prev + (value as number);
     }, 0);
     totalStackedValues.push(values);
+    thresholdValues.push((percentageThreshold / 100) * values);

Review comment:
       Since the controls sometimes handle missing/unset values in funny ways, I'd probably add this, just in case:
   ```js
       thresholdValues.push(((percentageThreshold || 0) / 100) * values);
   ```

##########
File path: superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
##########
@@ -49,8 +53,8 @@ describe('EchartsTimeseries tranformProps', () => {
   };
 
   it('should tranform chart props for viz', () => {
-    const chartProps = new ChartProps(chartPropsConfig);
-    expect(transformProps(chartProps)).toEqual(
+    const chartProps: unknown = new ChartProps(chartPropsConfig);

Review comment:
       This specific line should be something like
   ```typescript
       const chartProps = new ChartProps<EchartsTimeseriesChartProps>(chartPropsConfig);
   ```
   However, it seems these types are slightly tangled up right now, and should probably be addressed in a follow-up PR, as I believe it's going to require some more type fixing. I'd also want to enable proper type checking for the test folders, but that also requires a separate PR.




-- 
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: notifications-unsubscribe@superset.apache.org

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 commented on a change in pull request #17758: feat(timeseries-chart): add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx
##########
@@ -136,10 +136,29 @@ const onlyTotalControl = {
   },
 };
 
+const percentageThresholdControl = {
+  name: 'percentage_threshold',
+  config: {
+    type: 'TextControl',
+    label: t('Percentage threshold'),
+    renderTrigger: true,
+    isFloat: true,
+    default: 5,

Review comment:
       This doesn't be in line with `DEFAULT_FORM_DATA.percentageThreshold` in `types.ts`, which is set to `0`. Perhaps we should import the defaults and use that here, something like
   ```javascript
   import { DEFAULT_LEGEND_FORM_DATA } from './types';
   
   const percentageThresholdControl = {
     name: ...,
     config: {
       ...,
       default: DEFAULT_FORM_DATA.percentageThreshold,
       ....
     },
   }
   ```




-- 
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: notifications-unsubscribe@superset.apache.org

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] lyndsiWilliams commented on a change in pull request #17758: feat(timeseries-chart): [WIP] add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
##########
@@ -82,19 +86,20 @@ describe('EchartsTimeseries tranformProps', () => {
   it('should add a formula annotation to viz', () => {
     const formula: FormulaAnnotationLayer = {
       name: 'My Formula',
-      annotationType: 'FORMULA',
+      annotationType: AnnotationType.Formula,
       value: 'x+1',
-      style: 'solid',
+      style: AnnotationStyle.Solid,
+      showLabel: true,
       show: true,
     };
-    const chartProps = new ChartProps({
+    const chartProps: unknown = new ChartProps({

Review comment:
       Same question as above, would any of [these prop types](https://github.com/apache/superset/blob/6f8927209b80dc61ba14fc66ff94871b36a83321/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts#L32-L37) work instead of `unknown` here?




-- 
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: notifications-unsubscribe@superset.apache.org

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] lyndsiWilliams commented on a change in pull request #17758: feat(timeseries-chart): [WIP] add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
##########
@@ -132,37 +137,40 @@ describe('EchartsTimeseries tranformProps', () => {
 
   it('should add an interval, event and timeseries annotation to viz', () => {
     const event: EventAnnotationLayer = {
-      annotationType: 'EVENT',
+      annotationType: AnnotationType.Event,
       name: 'My Event',
       show: true,
-      sourceType: 'NATIVE',
-      style: 'solid',
+      showLabel: true,
+      sourceType: AnnotationSourceType.Native,
+      style: AnnotationStyle.Solid,
       value: 1,
     };
 
     const interval: IntervalAnnotationLayer = {
-      annotationType: 'INTERVAL',
+      annotationType: AnnotationType.Interval,
       name: 'My Interval',
       show: true,
-      sourceType: 'table',
+      showLabel: true,
+      sourceType: AnnotationSourceType.Table,
       titleColumn: '',
       timeColumn: 'start',
       intervalEndColumn: '',
       descriptionColumns: [],
-      style: 'dashed',
+      style: AnnotationStyle.Dashed,
       value: 2,
     };
 
     const timeseries: TimeseriesAnnotationLayer = {
-      annotationType: 'TIME_SERIES',
+      annotationType: AnnotationType.Timeseries,
       name: 'My Timeseries',
       show: true,
-      sourceType: 'line',
-      style: 'solid',
+      showLabel: true,
+      sourceType: AnnotationSourceType.Line,
+      style: AnnotationStyle.Solid,
       titleColumn: '',
       value: 3,
     };
-    const chartProps = new ChartProps({
+    const chartProps: unknown = new ChartProps({

Review comment:
       Just marking this with the other unknowns, in case this can be described with [one of these prop types](https://github.com/apache/superset/blob/6f8927209b80dc61ba14fc66ff94871b36a83321/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts#L32-L37) instead of `unknown`.




-- 
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: notifications-unsubscribe@superset.apache.org

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] corbinrobb commented on a change in pull request #17758: feat(timeseries-chart): add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx
##########
@@ -136,10 +136,29 @@ const onlyTotalControl = {
   },
 };
 
+const percentageThresholdControl = {
+  name: 'percentage_threshold',
+  config: {
+    type: 'TextControl',
+    label: t('Percentage threshold'),
+    renderTrigger: true,
+    isFloat: true,
+    default: 5,

Review comment:
       Ah yes, that is a bit confusing. What you got here makes a lot of sense. I will change it to this and push it up!




-- 
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: notifications-unsubscribe@superset.apache.org

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] lyndsiWilliams merged pull request #17758: feat(timeseries-chart): add percentage threshold input control

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


   


-- 
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: notifications-unsubscribe@superset.apache.org

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] github-actions[bot] commented on pull request #17758: feat(timeseries-chart): add percentage threshold input control

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17758:
URL: https://github.com/apache/superset/pull/17758#issuecomment-1005950899


   @lyndsiWilliams Ephemeral environment spinning up at http://34.221.152.42:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.


-- 
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: notifications-unsubscribe@superset.apache.org

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] lyndsiWilliams commented on pull request #17758: feat(timeseries-chart): add percentage threshold input control

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on pull request #17758:
URL: https://github.com/apache/superset/pull/17758#issuecomment-1005949482


   /testenv up


-- 
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: notifications-unsubscribe@superset.apache.org

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[bot] edited a comment on pull request #17758: feat(timeseries-chart): add percentage threshold input control

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17758:
URL: https://github.com/apache/superset/pull/17758#issuecomment-999102042


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17758](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (814bff0) into [master](https://codecov.io/gh/apache/superset/commit/63d9693f21786431ba7e2ec11d6658bcd3a1f9e9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (63d9693) will **decrease** coverage by `1.44%`.
   > The diff coverage is `80.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17758/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17758      +/-   ##
   ==========================================
   - Coverage   67.77%   66.32%   -1.45%     
   ==========================================
     Files        1602     1568      -34     
     Lines       64151    61455    -2696     
     Branches     6773     6237     -536     
   ==========================================
   - Hits        43476    40759    -2717     
   - Misses      18822    19099     +277     
   + Partials     1853     1597     -256     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `50.92% <80.00%> (-3.92%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ugins/plugin-chart-echarts/src/Timeseries/types.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90eXBlcy50cw==) | `100.00% <ø> (ø)` | |
   | [...tend/plugins/plugin-chart-echarts/src/controls.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvY29udHJvbHMudHN4) | `71.42% <50.00%> (-2.26%)` | :arrow_down: |
   | [...lugin-chart-echarts/src/Timeseries/transformers.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1lcnMudHM=) | `52.50% <83.33%> (+10.25%)` | :arrow_up: |
   | [...gin-chart-echarts/src/Timeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1Qcm9wcy50cw==) | `60.97% <100.00%> (+7.22%)` | :arrow_up: |
   | [...ntend/src/dashboard/util/replaceUndefinedByNull.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL3JlcGxhY2VVbmRlZmluZWRCeU51bGwudHM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/components/Slider/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2xpZGVyL2luZGV4LnRzeA==) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
   | [.../src/explore/components/controls/HiddenControl.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9IaWRkZW5Db250cm9sLnRzeA==) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
   | [...t-frontend/src/filters/components/GroupBy/index.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9Hcm91cEJ5L2luZGV4LnRz) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
   | [...ntend/src/filters/components/GroupBy/buildQuery.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9Hcm91cEJ5L2J1aWxkUXVlcnkudHM=) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
   | [...end/src/filters/components/TimeGrain/buildQuery.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9UaW1lR3JhaW4vYnVpbGRRdWVyeS50cw==) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
   | ... and [550 more](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [63d9693...814bff0](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@superset.apache.org

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] github-actions[bot] commented on pull request #17758: feat(timeseries-chart): add percentage threshold input control

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17758:
URL: https://github.com/apache/superset/pull/17758#issuecomment-1011437966


   Ephemeral environment shutdown and build artifacts deleted.


-- 
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: notifications-unsubscribe@superset.apache.org

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] AAfghahi commented on a change in pull request #17758: feat(timeseries-chart): [WIP] add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
##########
@@ -244,3 +252,203 @@ describe('EchartsTimeseries tranformProps', () => {
     );
   });
 });
+
+describe('Does transformProps transform series correctly', () => {
+  type seriesDataType = [Date, number];
+  type labelFormatterType = (params: {
+    value: seriesDataType;
+    dataIndex: number;
+    seriesIndex: number;
+  }) => string;
+  type seriesType = {
+    label: { show: boolean; formatter: labelFormatterType };
+    data: seriesDataType[];
+    name: string;
+  };
+
+  const formData = {
+    colorScheme: 'bnbColors',
+    datasource: '3__table',
+    granularity_sqla: 'ds',
+    metric: 'sum__num',
+    groupby: ['foo', 'bar'],
+    showValue: true,
+    stack: true,
+    onlyTotal: false,
+    percentageThreshold: 50,
+  };
+  const queriesData = [

Review comment:
       can we add this to fixtures?




-- 
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: notifications-unsubscribe@superset.apache.org

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] lyndsiWilliams commented on a change in pull request #17758: feat(timeseries-chart): [WIP] add percentage threshold input control

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



##########
File path: superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
##########
@@ -49,8 +53,8 @@ describe('EchartsTimeseries tranformProps', () => {
   };
 
   it('should tranform chart props for viz', () => {
-    const chartProps = new ChartProps(chartPropsConfig);
-    expect(transformProps(chartProps)).toEqual(
+    const chartProps: unknown = new ChartProps(chartPropsConfig);

Review comment:
       There are [a few prop types described in this component](https://github.com/apache/superset/blob/6f8927209b80dc61ba14fc66ff94871b36a83321/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts#L32-L37), would any of those work here instead of `unknown`? I see `unknown` in other spots in your PR and I'm assuming it's because the props change type, but I'm wondering if you might be able to make it work with a `thisTypeOfProps | thatTypeOfProps` type of description.




-- 
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: notifications-unsubscribe@superset.apache.org

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[bot] edited a comment on pull request #17758: feat(timeseries-chart): add percentage threshold input control

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17758:
URL: https://github.com/apache/superset/pull/17758#issuecomment-999102042


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17758](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5bc821f) into [master](https://codecov.io/gh/apache/superset/commit/63d9693f21786431ba7e2ec11d6658bcd3a1f9e9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (63d9693) will **decrease** coverage by `0.60%`.
   > The diff coverage is `80.00%`.
   
   > :exclamation: Current head 5bc821f differs from pull request most recent head 814bff0. Consider uploading reports for the commit 814bff0 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17758/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17758      +/-   ##
   ==========================================
   - Coverage   67.77%   67.16%   -0.61%     
   ==========================================
     Files        1602     1609       +7     
     Lines       64151    64786     +635     
     Branches     6773     6860      +87     
   ==========================================
   + Hits        43476    43514      +38     
   - Misses      18822    19418     +596     
   - Partials     1853     1854       +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `53.95% <80.00%> (-0.89%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ugins/plugin-chart-echarts/src/Timeseries/types.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90eXBlcy50cw==) | `100.00% <ø> (ø)` | |
   | [...tend/plugins/plugin-chart-echarts/src/controls.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvY29udHJvbHMudHN4) | `71.42% <50.00%> (-2.26%)` | :arrow_down: |
   | [...lugin-chart-echarts/src/Timeseries/transformers.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1lcnMudHM=) | `52.50% <83.33%> (+10.25%)` | :arrow_up: |
   | [...gin-chart-echarts/src/Timeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1Qcm9wcy50cw==) | `60.97% <100.00%> (+7.22%)` | :arrow_up: |
   | [superset-frontend/src/setup/setupColors.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLnRz) | `81.81% <0.00%> (-8.19%)` | :arrow_down: |
   | [...src/dashboard/components/PropertiesModal/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC9pbmRleC50c3g=) | `66.03% <0.00%> (-0.85%)` | :arrow_down: |
   | [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <0.00%> (ø)` | |
   | [superset-frontend/src/components/Icons/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvbnMvaW5kZXgudHN4) | `100.00% <0.00%> (ø)` | |
   | [superset-frontend/src/common/components/index.tsx](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL2luZGV4LnRzeA==) | `100.00% <0.00%> (ø)` | |
   | [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `100.00% <0.00%> (ø)` | |
   | ... and [31 more](https://codecov.io/gh/apache/superset/pull/17758/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [63d9693...814bff0](https://codecov.io/gh/apache/superset/pull/17758?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@superset.apache.org

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