[GitHub] williaster commented on a change in pull request #6282: [nvd3] refactor, thinner margins

[GitBox <gi...@apache.org>]

williaster commented on a change in pull request #6282: [nvd3] refactor, thinner margins
URL: https://github.com/apache/incubator-superset/pull/6282#discussion_r232372854
 
 

 ##########
 File path: superset/assets/src/visualizations/nvd3/NVD3Vis.js
 ##########
 @@ -589,19 +583,26 @@ function nvd3Vis(element, props) {
       const marginPad = Math.ceil(
         Math.min(maxWidth * (isExplore ? 0.01 : 0.03), MAX_MARGIN_PAD),
       );
-      const maxYAxisLabelWidth = getMaxLabelSize(svg, chart.yAxis2 ? 'nv-y1' : 'nv-y');
-      const maxXAxisLabelHeight = getMaxLabelSize(svg, 'nv-x');
-      chart.margin({ left: maxYAxisLabelWidth + marginPad });
-      if (yAxisLabel && yAxisLabel !== '') {
-        chart.margin({ left: maxYAxisLabelWidth + marginPad + 25 });
-      }
       // Hack to adjust margins to accommodate long axis tick labels.
       // - has to be done only after the chart has been rendered once
       // - measure the width or height of the labels
       // ---- (x axis labels are rotated 45 degrees so we use height),
       // - adjust margins based on these measures and render again
       const margins = chart.margin();
-      margins.bottom = 28;
+      if (chart.xAxis) {
+        margins.bottom = 28;
 
 Review comment:
   just to understand -- we are modifying the reference to the chart margin object directly? should we instead copy it and call `chart.margin(newMargin)`? if it works I guess that's fine.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org