refactor(layout_column_wrap): Use helper for wrapping children (#910)

shiny/api-examples/layout_column_wrap/app.py

@@ -7,7 +7,7 @@
     ui.layout_column_wrap(y, y, y, width=1 / 2),
     ui.hr(),
     # Has three columns when viewport is wider than 750px
-    ui.layout_column_wrap(y, y, y, width="250%"),
+    ui.layout_column_wrap(y, y, y, width="250px"),
 )
 
 

shiny/ui/_layout.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from typing import Literal, Optional, cast
+from typing import Iterable, Literal, Optional, cast
 
 from htmltools import Tag, TagAttrs, TagAttrValue, TagChild, css, div
 
@@ -128,14 +128,6 @@ def layout_column_wrap(
             else:
                 colspec = f"repeat(auto-fit, minmax(min({width_css_unit}, 100%), 1fr))"
 
-    # Use a new dict so that we don't mutate the original `children` dict
-    upgraded_children: list[TagChild] = []
-    for child_value in children:
-        child = div({"class": "bslib-gap-spacing"}, child_value)
-        if fillable:
-            child = as_fillable_container(child)
-        upgraded_children.append(child)
-
     tag_style_css = {
         "grid-template-columns": colspec,
         "grid-auto-rows": "1fr" if (heights_equal == "all") else None,
@@ -155,7 +147,7 @@ def layout_column_wrap(
             "style": css(**tag_style_css),
         },
         attrs,
-        *upgraded_children,
+        *wrap_all_in_gap_spaced_container(children, fillable),
         components_dependency(),
     )
     if fill:
@@ -170,3 +162,23 @@ def is_probably_a_css_unit(x: TagChild) -> bool:
     if isinstance_cssunit(x):
         return True
     return False
+
+
+def wrap_all_in_gap_spaced_container(
+    children: Iterable[TagChild],
+    fillable: bool = True,
+    class_: Optional[str] = None,
+) -> list[TagChild]:
+    item_class = "bslib-gap-spacing"
+    if class_ is not None:
+        item_class = f"{item_class} {class_}"
+
+    # Use a new list so that we don't mutate the original `children`
+    wrapped_children: list[TagChild] = []
+    for child_value in children:
+        child = div({"class": item_class}, child_value)
+        if fillable:
+            child = as_fillable_container(child)
+        wrapped_children.append(child)
+
+    return wrapped_children

shiny/ui/_layout_columns.py

@@ -4,9 +4,10 @@
 from typing import Dict, Iterable, Optional, TypeVar, Union, cast
 from warnings import warn
 
-from htmltools import Tag, TagAttrs, TagAttrValue, TagChild, css, div
+from htmltools import Tag, TagAttrs, TagAttrValue, TagChild, css
 
 from ._html_deps_shinyverse import web_component_dependency
+from ._layout import wrap_all_in_gap_spaced_container
 from ._tag import consolidate_attrs
 from .css import CssUnit, as_css_unit
 from .fill import as_fill_item, as_fillable_container
@@ -135,7 +136,7 @@ def layout_columns(
         col_widths_attrs(col_widths_spec),
         attrs,
         row_heights_attrs(row_heights),
-        *wrap_all_in_grid_item_container(children, fillable),
+        *wrap_all_in_gap_spaced_container(children, fillable, "bslib-grid-item"),
         web_component_dependency(),
     )
 
@@ -146,26 +147,6 @@ def layout_columns(
     return tag
 
 
-def wrap_all_in_grid_item_container(
-    children: Iterable[TagChild],
-    fillable: bool = True,
-    class_: Optional[str] = None,
-) -> list[TagChild]:
-    item_class = "bslib-grid-item bslib-gap-spacing"
-    if class_ is not None:
-        item_class = f"{item_class} {class_}"
-
-    # Use a new list so that we don't mutate the original `children`
-    wrapped_children: list[TagChild] = []
-    for child_value in children:
-        child = div({"class": item_class}, child_value)
-        if fillable:
-            child = as_fillable_container(child)
-        wrapped_children.append(child)
-
-    return wrapped_children
-
-
 def as_col_spec(
     col_widths: BreakpointsUser[int],
     n_kids: int,