feat(plugin-chart-echarts): add support for generic axis to mixed chart #20097

villebro · 2022-05-17T12:21:28Z

SUMMARY

Add support for generic x-axis to Mixed Time-Series chart. In addition do the following changes:

Make the chart name and description dependent on the feature flag
Implement temporal x-axis scaling that was introduced in fix(plugin-chart-echarts): xAxis scale is not correct when time grain is quarter #19686
Remove the "Predictive" tag from the Mixed chart (it doesn't support the predictive analytics functionality)
Explicitly support null values on the control panel section config and make the check explicit with a type guard
Minor refactoring/cleanup

AFTER

When the feature flag is enabled, the x-axis control appears, making it possible to use a non-temporal x-axis:

Now the chart is called "Mixed chart" instead of "Mixed Time-series Chart":

TESTING INSTRUCTIONS

Enable GENERIC_CHART_AXES feature flag
Go to chart picker and notice that Mixed time-series chart has been replaced with Mixed chart with a description that doesn't mention the words "timeseries" or "temporal"

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

stephenLYZ · 2022-05-17T12:56:22Z

/testenv up FEATURE_GENERIC_CHART_AXES=true

github-actions · 2022-05-17T13:10:21Z

@stephenLYZ Container image not yet published for this PR. Please try again when build is complete.

github-actions · 2022-05-17T13:10:21Z

@stephenLYZ Ephemeral environment creation failed. Please check the Actions logs for details.

villebro · 2022-05-18T07:27:00Z

/testenv up FEATURE_GENERIC_CHART_AXES=true

github-actions · 2022-05-18T07:30:31Z

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

codecov · 2022-05-18T07:41:16Z

Codecov Report

Merging #20097 (11a1d13) into master (1fe30f1) will decrease coverage by 0.00%.
The diff coverage is 28.57%.

@@            Coverage Diff             @@
##           master   #20097      +/-   ##
==========================================
- Coverage   66.46%   66.45%   -0.01%     
==========================================
  Files        1721     1721              
  Lines       64467    64475       +8     
  Branches     6795     6804       +9     
==========================================
+ Hits        42847    42848       +1     
- Misses      19892    19895       +3     
- Partials     1728     1732       +4

Flag	Coverage Δ
javascript	`51.35% <28.57%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...chart-echarts/src/MixedTimeseries/controlPanel.tsx	`86.66% <0.00%> (-6.20%)`	⬇️
.../plugin-chart-echarts/src/MixedTimeseries/index.ts	`25.00% <0.00%> (-25.00%)`	⬇️
...hart-echarts/src/MixedTimeseries/transformProps.ts	`0.00% <0.00%> (ø)`
.../plugin-chart-echarts/src/Timeseries/buildQuery.ts	`66.66% <ø> (-4.77%)`	⬇️
...nd/src/explore/controlUtils/getSectionsToRender.ts	`80.00% <ø> (-0.96%)`	⬇️
...d/plugins/plugin-chart-echarts/src/utils/series.ts	`87.50% <20.00%> (-4.50%)`	⬇️
...d/packages/superset-ui-chart-controls/src/types.ts	`100.00% <100.00%> (ø)`
...in-chart-echarts/src/MixedTimeseries/buildQuery.ts	`100.00% <100.00%> (ø)`
...gin-chart-echarts/src/Timeseries/transformProps.ts	`58.94% <100.00%> (+2.08%)`	⬆️
...tend/plugins/plugin-chart-echarts/src/constants.ts	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fe30f1...11a1d13. Read the comment docs.

villebro · 2022-05-18T11:07:01Z

An example chart on the eph env: http://34.215.190.191:8080/superset/explore/p/gG3OYRVY0wj/

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts

kgabryje · 2022-05-18T14:20:13Z

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx

@@ -447,4 +447,12 @@ const config: ControlPanelConfig = {
  ],
 };

+if (isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)) {


Can we just use ternary operator when declaring config? Like

isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)) ? { label: t('Shared query fields'), expanded: true, controlSetRows: [[xAxisControl]], } : sections.legacyTimeseriesTime

We still need the time section (I hope we can get rid of it soon, but it'll require some more work that's beyond the scope of this PR)

Sorry, I meant to write

isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)) ? { label: t('Shared query fields'), expanded: true, controlSetRows: [[xAxisControl]], } : null

🤦
I think that's a pattern that we use in several control panels

oh right, I forgot about that pattern 👍

Actually this currently only applies to controls, not control sections. But I added the null option to sections, too (tested that it works exactly like for controls): 60e7ba0 and 11a1d13

kgabryje

Tested, LGTM

github-actions · 2022-05-19T10:54:32Z

Ephemeral environment shutdown and build artifacts deleted.

…rt (apache#20097) * feat(plugin-chart-echarts): add support for generic axis to mixed chart * fix tests + add new tests * address review comments * simplify control panel * fix types and tests

superset-github-bot bot added the preset-io label May 17, 2022

pull-request-size bot added the size/L label May 17, 2022

villebro requested review from zhaoyongjie and stephenLYZ May 17, 2022 12:23

villebro force-pushed the villebro/mixed-generic branch from 4fa6505 to b6e13af Compare May 17, 2022 12:46

villebro force-pushed the villebro/mixed-generic branch from 7894a2f to 5dd1151 Compare May 18, 2022 09:28

kgabryje reviewed May 18, 2022

View reviewed changes

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts Outdated Show resolved Hide resolved

kgabryje reviewed May 18, 2022

View reviewed changes

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts Outdated Show resolved Hide resolved

kgabryje reviewed May 18, 2022

View reviewed changes

villebro added 3 commits May 18, 2022 18:29

feat(plugin-chart-echarts): add support for generic axis to mixed chart

7ebee42

fix tests + add new tests

239ac0a

address review comments

00755eb

villebro force-pushed the villebro/mixed-generic branch from 5dd1151 to 00755eb Compare May 18, 2022 15:31

villebro added 2 commits May 18, 2022 19:47

simplify control panel

60e7ba0

fix types and tests

11a1d13

kgabryje approved these changes May 19, 2022

View reviewed changes

villebro merged commit d5c5e58 into apache:master May 19, 2022

villebro deleted the villebro/mixed-generic branch May 19, 2022 10:52

villebro mentioned this pull request May 20, 2022

fix(explore): handle null control sections #20142

Merged

9 tasks

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(plugin-chart-echarts): add support for generic axis to mixed chart #20097

feat(plugin-chart-echarts): add support for generic axis to mixed chart #20097

villebro commented May 17, 2022 •

edited

Loading

stephenLYZ commented May 17, 2022

github-actions bot commented May 17, 2022

github-actions bot commented May 17, 2022

villebro commented May 18, 2022

github-actions bot commented May 18, 2022

codecov bot commented May 18, 2022 •

edited

Loading

villebro commented May 18, 2022

kgabryje May 18, 2022

villebro May 18, 2022

kgabryje May 18, 2022 •

edited

Loading

villebro May 18, 2022

villebro May 18, 2022 •

edited

Loading

kgabryje left a comment

github-actions bot commented May 19, 2022

feat(plugin-chart-echarts): add support for generic axis to mixed chart #20097

feat(plugin-chart-echarts): add support for generic axis to mixed chart #20097

Conversation

villebro commented May 17, 2022 • edited Loading

SUMMARY

AFTER

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

stephenLYZ commented May 17, 2022

github-actions bot commented May 17, 2022

github-actions bot commented May 17, 2022

villebro commented May 18, 2022

github-actions bot commented May 18, 2022

codecov bot commented May 18, 2022 • edited Loading

Codecov Report

villebro commented May 18, 2022

kgabryje May 18, 2022

Choose a reason for hiding this comment

villebro May 18, 2022

Choose a reason for hiding this comment

kgabryje May 18, 2022 • edited Loading

Choose a reason for hiding this comment

villebro May 18, 2022

Choose a reason for hiding this comment

villebro May 18, 2022 • edited Loading

Choose a reason for hiding this comment

kgabryje left a comment

Choose a reason for hiding this comment

github-actions bot commented May 19, 2022

villebro commented May 17, 2022 •

edited

Loading

codecov bot commented May 18, 2022 •

edited

Loading

kgabryje May 18, 2022 •

edited

Loading

villebro May 18, 2022 •

edited

Loading