From cc51a69486cac5a99f32df7ae104e117a4d08f32 Mon Sep 17 00:00:00 2001
From: Carson <cpsievert1@gmail.com>
Date: Mon, 20 Nov 2023 11:08:44 -0600
Subject: [PATCH] Close #112. Avoid setting a new default height in order to
 get consistent filling layout behavior

---
 shinywidgets/_shinywidgets.py        | 29 +++++++++++++++-------------
 shinywidgets/static/shinywidgets.css | 12 +++++++-----
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/shinywidgets/_shinywidgets.py b/shinywidgets/_shinywidgets.py
index d0060f0..8878391 100644
--- a/shinywidgets/_shinywidgets.py
+++ b/shinywidgets/_shinywidgets.py
@@ -312,24 +312,27 @@ def set_layout_defaults(widget: Widget) -> Tuple[Widget, bool]:
 
     layout = widget.layout         # type: ignore
 
-    # Give the ipywidget Layout() width/height defaults that are more sensible for
-    # filling layout https://ipywidgets.readthedocs.io/en/stable/examples/Widget%20Layout.html
+    # If the ipywidget Layout() height is set to something other than "auto", then
+    # don't do filling layout https://ipywidgets.readthedocs.io/en/stable/examples/Widget%20Layout.html
     if isinstance(layout, Layout):
-        if layout.width is None:   # type: ignore
-            layout.width = "100%"
-        if layout.height is None:  # type: ignore
-            layout.height = "400px"
-        else:
-            if layout.height != "auto":  # type: ignore
+        if layout.height is not None and layout.height != "auto":  # type: ignore
+            fill = False
+
+    pkg = widget_pkg(widget)
+
+    # Plotly provides it's own layout API (which isn't a subclass of ipywidgets.Layout)
+    if pkg == "plotly":
+        from plotly.graph_objs import Layout as PlotlyLayout
+        if isinstance(layout, PlotlyLayout):
+            if layout.height is not None:
                 fill = False
 
     widget.layout = layout
 
-    # Some packages (e.g., altair) aren't setup to fill their parent container by
-    # default. I can't imagine a situation where you'd actually want it to _not_ fill
-    # the parent container since it'll be contained within the Layout() container, which
-    # has a full-fledged sizing API.
-    pkg = widget_pkg(widget)
+    # altair, confusingly, isn't setup to fill it's Layout() container by default. I
+    # can't imagine a situation where you'd actually want it to _not_ fill the parent
+    # container since it'll be contained within the Layout() container, which has a
+    # full-fledged sizing API.
     if pkg == "altair":
         from altair import JupyterChart
 
diff --git a/shinywidgets/static/shinywidgets.css b/shinywidgets/static/shinywidgets.css
index 7df0f36..29f7238 100644
--- a/shinywidgets/static/shinywidgets.css
+++ b/shinywidgets/static/shinywidgets.css
@@ -7,12 +7,14 @@
 }
 
 /*
-  For most/all ipywidgets, we can set these defaults via the Layout() API,
-  but go.FigureWidget().layout points to plotly.js' layout, and I'm not
-  sure how to get at the ipywidget's layout object from there.
+  .html-fill-item defaults to `flex-basis: auto`, which works well when item's
+  all have consistent height, but we can't assume that (because different
+  widgets want different height defaults). By setting `flex-basis: 400px`,
+  we let widgets keep its natural height (when not filling), but when filling,
+  widgets will all grow/shrink to a consistent height.
 */
-.plotly.plot-container {
-  height: 400px;
+.shiny-ipywidget-output.html-fill-container > .lm-Widget.html-fill-item {
+  flex-basis: 400px;
   width: 100%;
 }