Repository: jtablesaw/tablesaw
Pull request #713 (2019-11-05 17:45:22)

Adds LayoutBuilder.autosize and only generates width: or height: to Javascript if they have been actually set
Thanks for contributing.

- [x] Tick to sign-off your agreement to the [Developer Certificate of Origin (DCO) 1.1](https://developercertificate.org)

## Description

Generate layout: autosize when set, don't generate width: / height: and let Plot.ly compute them.

Closes issue #712

## Testing

Yes.



Patch:
diff --git a/jsplot/src/main/java/tech/tablesaw/plotly/components/Layout.java b/jsplot/src/main/java/tech/tablesaw/plotly/components/Layout.java
index 5660ec08..eb598c9f 100644
--- a/jsplot/src/main/java/tech/tablesaw/plotly/components/Layout.java
+++ b/jsplot/src/main/java/tech/tablesaw/plotly/components/Layout.java
@@ -119,6 +119,9 @@ public class Layout {
    */
   private final boolean autoSize;
 
+  private final boolean heightSet;
+  private final boolean widthSet;
+
   /** The width of the plot in pixels */
   private final int width;
 
@@ -178,6 +181,8 @@ public class Layout {
   private Layout(LayoutBuilder builder) {
     this.title = builder.title;
     this.autoSize = builder.autoSize;
+    this.widthSet = builder.widthSet;
+    this.heightSet = builder.heightSet;
     this.decimalSeparator = builder.decimalSeparator;
     this.thousandsSeparator = builder.thousandsSeparator;
     this.dragMode = builder.dragMode;
@@ -222,10 +227,21 @@ public class Layout {
     Map<String, Object> context = new HashMap<>();
     if (!title.equals(DEFAULT_TITLE)) context.put("title", title);
     if (!titleFont.equals(DEFAULT_TITLE_FONT)) context.put("titlefont", titleFont);
-    context.put("width", width);
-    context.put("height", height);
     if (!font.equals(DEFAULT_FONT)) context.put("font", font);
-    if (!autoSize == DEFAULT_AUTO_SIZE) context.put("autosize", autoSize);
+    if (autoSize != DEFAULT_AUTO_SIZE) {
+      context.put("autosize", autoSize);
+      // since autosize is true, we assume the default width / height values are not wanted, not
+      // serialize them, and let Plotly compute them
+      if (widthSet) {
+        context.put("width", width);
+      }
+      if (heightSet) {
+        context.put("height", height);
+      }
+    } else {
+      context.put("width", width);
+      context.put("height", height);
+    }
     if (hoverDistance != DEFAULT_HOVER_DISTANCE) context.put("hoverdistance", hoverDistance);
     if (!hoverMode.equals(DEFAULT_HOVER_MODE)) context.put("hoverMode", hoverMode);
     if (margin != null) {
@@ -300,7 +316,10 @@ public class Layout {
      * is initialized on each relayout. Note that, regardless of this attribute, an undefined layout
      * width or height is always initialized on the first call to plot.
      */
-    private final boolean autoSize = false;
+    private boolean autoSize = false;
+
+    private boolean widthSet = false;
+    private boolean heightSet = false;
 
     /** The width of the plot in pixels */
     private int width = 700;
@@ -409,11 +428,18 @@ public class Layout {
 
     public LayoutBuilder height(int height) {
       this.height = height;
+      this.heightSet = true;
       return this;
     }
 
     public LayoutBuilder width(int width) {
       this.width = width;
+      this.widthSet = true;
+      return this;
+    }
+
+    public LayoutBuilder autosize(boolean autosize) {
+      this.autoSize = autosize;
       return this;
     }
 
diff --git a/jsplot/src/main/resources/layout_template.html b/jsplot/src/main/resources/layout_template.html
index 8f9daa21..526b164d 100644
--- a/jsplot/src/main/resources/layout_template.html
+++ b/jsplot/src/main/resources/layout_template.html
@@ -5,8 +5,12 @@ var layout = {
 {% if titlefont is not null %}
     titlefont: {{titlefont | raw}},
 {% endif %}
+{% if height is not null %}
     height: {{height}},
+{% endif %}
+{% if width is not null %}
     width: {{width}},
+{% endif %}
 {% if showlegend is not null %}
     showlegend: {{showlegend}},
 {% endif %}


Test patch:
diff --git a/jsplot/src/test/java/tech/tablesaw/components/LayoutTest.java b/jsplot/src/test/java/tech/tablesaw/components/LayoutTest.java
index c92f856d..29a72bff 100644
--- a/jsplot/src/test/java/tech/tablesaw/components/LayoutTest.java
+++ b/jsplot/src/test/java/tech/tablesaw/components/LayoutTest.java
@@ -1,18 +1,17 @@
 package tech.tablesaw.components;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import tech.tablesaw.plotly.components.Axis;
 import tech.tablesaw.plotly.components.Grid;
 import tech.tablesaw.plotly.components.Layout;
 import tech.tablesaw.plotly.components.Margin;
 
-@Disabled
 public class LayoutTest {
 
-  @Test
+  // @Test
   public void asJavascript() {
 
     Axis x = Axis.builder().title("x axis").build();
@@ -29,7 +28,7 @@ public class LayoutTest {
     System.out.println(layout.asJavascript());
   }
 
-  @Test
+  // @Test
   public void asJavascriptForGrid() {
 
     Axis x = Axis.builder().title("x axis").build();
@@ -50,4 +49,65 @@ public class LayoutTest {
     assertTrue(asJavascript.contains("rows"));
     assertTrue(asJavascript.contains("xAxis"));
   }
+
+  @Test
+  public void testAutosize() {
+    {
+      Layout layout = Layout.builder().autosize(true).build();
+      assertEquals(
+          "var layout = {\n"
+              + //
+              "    autosize: true,\n"
+              + //
+              "\n\n"
+              + //
+              "};\n",
+          layout.asJavascript());
+    }
+    {
+      Layout layout = Layout.builder().autosize(true).width(800).build();
+      assertEquals(
+          "var layout = {\n"
+              + //
+              "    width: 800,\n"
+              + //
+              "    autosize: true,\n"
+              + //
+              "\n\n"
+              + //
+              "};\n",
+          layout.asJavascript());
+    }
+    {
+      Layout layout = Layout.builder().autosize(true).height(600).width(800).build();
+      assertEquals(
+          "var layout = {\n"
+              + //
+              "    height: 600,\n"
+              + //
+              "    width: 800,\n"
+              + //
+              "    autosize: true,\n"
+              + //
+              "\n\n"
+              + //
+              "};\n",
+          layout.asJavascript());
+    }
+    {
+      // see if 700x450
+      Layout layout = Layout.builder().autosize(false).height(600).build();
+      assertEquals(
+          "var layout = {\n"
+              + //
+              "    height: 600,\n"
+              + //
+              "    width: 700,\n"
+              + //
+              "\n\n"
+              + //
+              "};\n",
+          layout.asJavascript());
+    }
+  }
 }