Vislib refactor nr5 #8950
Vislib refactor nr5 #8950
Conversation
(cherry picked from commit 29c2021
(the others are left for backward compatibility) (cherry picked from commit b1bd840)
(cherry picked from commit f658a41)
(cherry picked from commit 1226445)
(cherry picked from commit fb81bcb)
(cherry picked from commit 6726ef6)
(cherry picked from commit 36f283a)
(cherry picked from commit 7853c86)
(cherry picked from commit c15667e)
(cherry picked from commit e046000)
(cherry picked from commit f9ddcb2)
(cherry picked from commit 801429b)
| @@ -64,7 +64,16 @@ export default function ColumnHandler(Private) { | |||
| config.chart = { | |||
| type: 'point_series', | |||
| series: _.map(series, (seri) => { | |||
| return _.defaults({}, cfg, { | |||
| return _.defaults({ | |||
There was a problem hiding this comment.
This _.defaults() call is unnecessary
| @@ -198,9 +198,9 @@ export default function AxisFactory(Private) { | |||
| } | |||
|
|
|||
| getLength(el, n) { | |||
| const margin = this.visConfig.get('style.margin'); | |||
| const widthSpacing = 43; | |||
There was a problem hiding this comment.
Where does this number come from?
|
While playing around with this I wrote down some of the bugs that I found that seemed to be from outside of this pr, but that need to be fixed before we can merge the vislib-axis-refactor branch into master |
| }); | ||
| }; | ||
|
|
||
| updateCategoryAxisSize() { |
There was a problem hiding this comment.
this function (similar) existed on y_axis_split before ...
- it would pre-render y axis and update its width so that x axis can then render correctly.
- after x axis is rendered it would update y-axis height so that y axis can render correctly
this seemed super weird so my first step was just removing the pre-render part, but now i see we probably need it so
-
i would pre-render category axis (no matter of its position) and update its width/height (depending on positon) and in case its a horizontal one update y-axis-spacer-block as well. . i decided to go with category axis as there is only one. with value axes we would need to pre-render them all. this way value axes can now render correctly
-
first render the value axes ... we dont need to update anything more after that (so updateXaxisHeight function on axis is not needed anymore)
i moved function to the Layout as we can only call it once whole layout has rendered. (y-axis-split wouldnt work)
There was a problem hiding this comment.
wrong sizing of category axis (it was either too long or too short) and it gets rid of that magic number (which worked just in case your value axis was 43 pixels wide/tall :)
|
@spalger i updated based on your comments. would you prefer that i fix the issues you found in a separate PR ? |
|
Yeah, I would prefer to take care of those outside of this pr. |
ppisljar
force-pushed
the
vislib-axis-refactor-5
branch
from
08d0611 to
43d3930
Compare
* moving margin to css * adding point_series type (the others are left for backward compatibility) * adding series default options * fixing alignment of axes * injectZeros should be applied per value axis * fixing error in axis matching * fixing style (indentation) * fixing tooltips * fixing axisformatter * making grouped/overlap the default mode * making charts direction independent * fixing tests
tbragin
added
the
Feature:Visualizations
another PR in vislib refactor series ...
i tried to make commits as clear as possible, each commit should be one change.
also take a look at PoC branch which is based on top of this ...