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

fix(plugin-chart-echarts): include label length in distance calculation #1056

Merged
merged 3 commits into from
May 3, 2021

Conversation

krsnik93
Copy link
Contributor

🐛 Bug Fix
Fixes labels overlapping axis as reported here: apache/superset#14072 (comment)

Label length (in chars) is now taken into account when calculating distance from axis.

Screenshots:
image
image
image
image

@krsnik93 krsnik93 requested a review from a team as a code owner April 14, 2021 10:56
@vercel
Copy link

vercel bot commented Apr 14, 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/8WMUXU2bbHmPkodUVaAHBKLPFHTn
✅ Preview: https://superset-ui-git-fork-krsnik93-fix-gauge-label-distance-superset.vercel.app

@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #1056 (d99ca34) into master (6825e87) will increase coverage by 0.02%.
The diff coverage is 80.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1056      +/-   ##
==========================================
+ Coverage   27.80%   27.82%   +0.02%     
==========================================
  Files         453      453              
  Lines        9104     9108       +4     
  Branches     1416     1417       +1     
==========================================
+ Hits         2531     2534       +3     
  Misses       6381     6381              
- Partials      192      193       +1     
Impacted Files Coverage Δ
...lugins/plugin-chart-echarts/src/Gauge/constants.ts 100.00% <ø> (ø)
...s/plugin-chart-echarts/src/Gauge/transformProps.ts 84.31% <80.00%> (-0.80%) ⬇️

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 6825e87...d99ca34. Read the comment docs.

@villebro
Copy link
Contributor

@krsnik93 this needs a rebase

@krsnik93
Copy link
Contributor Author

@villebro thanks, done

@krsnik93
Copy link
Contributor Author

@villebro hello, would you have some time to take a look at this?

Copy link
Contributor

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM - when there is some spare time let's try to add native support for improved text alignment to the gauge series type in ECharts.

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.

None yet

2 participants