fix(plugin-chart-echarts): improve yAxisBounds parsing #815
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/superset/superset-ui/mzy5te28i |
Codecov Report
@@ Coverage Diff @@
## master #815 +/- ##
==========================================
+ Coverage 25.57% 25.66% +0.08%
==========================================
Files 359 361 +2
Lines 7949 7957 +8
Branches 1055 1058 +3
==========================================
+ Hits 2033 2042 +9
+ Misses 5796 5795 -1
Partials 120 120
Continue to review full report at Codecov.
|
| expect(parseYAxisBound(1)).toEqual(1); | ||
| expect(parseYAxisBound('1')).toEqual(1); | ||
| expect(parseYAxisBound(10.1)).toEqual(10.1); | ||
| expect(parseYAxisBound('10.1')).toEqual(10.1); |
There was a problem hiding this comment.
AFAIK the YAxisBoundsControl doesn't actually return strings, but I decided to parse string values, just in case.
| */ | ||
| // eslint-disable-next-line import/prefer-default-export | ||
| export function parseYAxisBound(bound?: string | number | null): number | undefined { | ||
| if (bound === undefined || bound === null || Number.isNaN(Number(bound))) { |
There was a problem hiding this comment.
Number(undefined) returns NaN, so you don't technically need that, but if it helps for code-reading clarity, so be it :)
There was a problem hiding this comment.
Thanks for the review! Just to make sure I understood correctly: That last bit is to catch unparseable strings and NaNs (`undefined was already checked at the beginning). If this can be written more compactly I'm happy to learn+change!
🐛 Bug Fix
Currently the
YAxisBoundsControlwill initialize withnullvalues despite being passedundefined, causing trouble when setting the y axis bounds. This improves the parsing of said values prior to using them so Y-axis bounds are always either undefined or valid values.Before
After