Skip to content
This repository was archived by the owner on Dec 10, 2021. It is now read-only.

fix(plugin-chart-echarts): default to standard x-axis format #1043

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

villebro
Copy link
Contributor

@villebro villebro commented Apr 7, 2021

🏆 Enhancements
Default to using the default time axis formatter for the ECharts Timeseries chart and remove default 45 degree rotation. Also introduce a new adaptive time formatter for use in the tooltip which shows increasing level of detail based on the timestamp. This is in contrast with the current adaptive time formatter, which e.g. only shows the seconds/milliseconds when a timestamp has second/subsecond granularity.

Last full year (only months show up):
image

Last quarter (months with weekly ticks):
image

Last month (four day ticks):
image

Last week (daily ticks):
image

Two days (6 hour ticks):
image

Tooltip with millisecond granularity data:
image

@villebro villebro requested a review from a team as a code owner April 7, 2021 12:22
@vercel
Copy link

vercel bot commented Apr 7, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/superset/superset-ui/AM4Scg19Vz9gTJkRJFn9JxNgi2Wf
✅ Preview: https://superset-ui-git-fork-preset-io-villebro-echarts-xaxis-superset.vercel.app

['%d-%m-%Y %H:%M:%S', '%Y-%m-%d %H:%M:%S | 14-01-2019 01:32:10'],
['%d-%m-%Y %H:%M:%S', '%d-%m-%Y %H:%M:%S | 14-01-2019 01:32:10'],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bycatch - typo in the description

@codecov
Copy link

codecov bot commented Apr 7, 2021

Codecov Report

Merging #1043 (dcedf7b) into master (93d7fb4) will increase coverage by 0.31%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1043      +/-   ##
==========================================
+ Coverage   27.74%   28.06%   +0.31%     
==========================================
  Files         428      436       +8     
  Lines        8758     8869     +111     
  Branches     1314     1345      +31     
==========================================
+ Hits         2430     2489      +59     
- Misses       6155     6194      +39     
- Partials      173      186      +13     
Impacted Files Coverage Δ
...perset-ui-chart-controls/src/utils/D3Formatting.ts 100.00% <ø> (ø)
...ugin-chart-echarts/src/Timeseries/controlPanel.tsx 100.00% <ø> (ø)
...ugins/plugin-chart-echarts/src/Timeseries/types.ts 100.00% <ø> (ø)
...gin-chart-echarts/src/Timeseries/transformProps.ts 41.81% <25.00%> (-2.19%) ⬇️
...re/src/time-format/formatters/smartDateDetailed.ts 100.00% <100.00%> (ø)
plugins/plugin-chart-echarts/src/utils/controls.ts 75.00% <0.00%> (-25.00%) ⬇️
plugins/plugin-chart-table/src/transformProps.ts 61.62% <0.00%> (-4.63%) ⬇️
plugins/plugin-chart-table/src/TableChart.tsx 46.07% <0.00%> (-3.93%) ⬇️
...lugins/plugin-chart-table/src/utils/formatValue.ts 64.28% <0.00%> (-2.39%) ⬇️
... and 17 more

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 93d7fb4...dcedf7b. Read the comment docs.

@@ -184,7 +193,7 @@ export default function transformProps(chartProps: ChartProps): EchartsProps {
const value: number = !richTooltip ? params.value : params[0].value[0];
const prophetValue = !richTooltip ? [params] : params;

const rows: Array<string> = [`${xAxisFormatter(value)}`];
const rows: Array<string> = [`${tooltipFormatter(value)}`];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested and found that the String constructor did not work.

image

@zhaoyongjie zhaoyongjie self-requested a review April 9, 2021 08:01
Copy link
Contributor

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Ignore my previous reply.

@villebro villebro merged commit 2af1119 into apache-superset:master Apr 9, 2021
@villebro villebro deleted the villebro/echarts-xaxis branch April 9, 2021 08:03
NejcZdovc pushed a commit to blotoutio/superset-ui that referenced this pull request Apr 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants