backport #116

"Heuristic: Don't expand an auto column when a cell spans all frac columns"
PgBiel · Jan 11, 2024 · cd23bcc · cd23bcc
1 parent 88a0492
commit cd23bcc
Show file tree

Hide file tree

Showing 2 changed files with 169 additions and 32 deletions.
diff --git a/tablex-test.typ b/tablex-test.typ
@@ -1165,14 +1165,128 @@ Combining em and pt (with a stroke object):
   [lorem_ipsum_dolor_sit_amet], [lorem], [lorem_ipsum_dolor_sit_amet_consectetur_adipisici], [lorem],
 )
 
-*Rowspans with 1fr and auto using 'fit-spans' (Issues \#56 and \#78)*
+*Rowspans spanning 1fr and auto with 'fit-spans'*
 
-#[
 #let unbreakable-tablex(..args) = block(breakable: false, tablex(..args))
 
+- Normal sizes:
+
+    #unbreakable-tablex(
+        columns: (auto, auto, 1fr, 1fr),
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E]
+    )
+
+- With colspan over auto and 1fr (but not all fractional columns):
+
+    #unbreakable-tablex(
+        columns: (auto, auto, 1fr, 1fr),
+        colspanx(3)[Hello world! Hello!], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E]
+    )
+
+- Using `fit-spans`, column sizes should be identical to the first table (in all three below):
+
+    #unbreakable-tablex(
+        columns: (auto, auto, 1fr, 1fr),
+        fit-spans: (x: true),
+        colspanx(3)[Hello world! Hello!], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E]
+    )
+
+    #unbreakable-tablex(
+        columns: (auto, auto, 1fr, 1fr),
+        fit-spans: true,
+        colspanx(3)[Hello world! Hello!], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E]
+    )
+
+    #unbreakable-tablex(
+        columns: (auto, auto, 1fr, 1fr),
+        colspanx(3, fit-spans: (x: true))[Hello world! Hello!], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E],
+        [A], [BC], [D], [E]
+    )
+
+*Other `fit-spans` tests*
+
+1. Columns
+
+    #unbreakable-tablex(
+        columns: 4,
+        [A], [B], [C], [D],
+    )
+
+    #unbreakable-tablex(
+        columns: 4,
+        colspanx(4, lorem(20)),
+        [A], [B], [C], [D],
+    )
+
+    #unbreakable-tablex(
+        columns: 4,
+        fit-spans: (x: true),
+        colspanx(4, lorem(20)),
+        [A], [B], [C], [D],
+    )
+
+    #unbreakable-tablex(
+        columns: 4,
+        fit-spans: true,
+        colspanx(4, lorem(20)),
+        [A], [B], [C], [D],
+    )
+
+2. Rows
+
+    #unbreakable-tablex(
+        columns: (auto, 4em),
+        [A], [B],
+        [C], [B],
+        [D], [E]
+    )
+
+    #unbreakable-tablex(
+        columns: (auto, 4em),
+        [A], rowspanx(2, line(start: (0pt, 0pt), end: (0pt, 6em))),
+        [C], (),
+        [D], [E]
+    )
+
+    #unbreakable-tablex(
+        columns: (auto, 4em),
+        fit-spans: (y: true),
+        [A], rowspanx(2, line(start: (0pt, 0pt), end: (0pt, 6em))),
+        [C], (),
+        [D], [E #v(2em)]
+    )
+
+    #unbreakable-tablex(
+        columns: (auto, 4em),
+        fit-spans: true,
+        [A], rowspanx(2, line(start: (0pt, 0pt), end: (0pt, 6em))),
+        [C], (),
+        [D], [E #v(2em)]
+    )
+
+*Rowspans spanning all fractional columns and auto (Issues \#56 and \#78)*
+
 _For issue \#78_
 
-- Table is normal:
+- Columns should have the same size in all samples below:
 
     #unbreakable-tablex(
         columns: (1fr, 1fr, auto, auto, auto),
@@ -1183,8 +1297,6 @@ _For issue \#78_
         [a], [b], [c], [d], [e],
     )
 
-- Table has overlap:
-
     #unbreakable-tablex(
         columns: (1fr, 1fr, auto, auto, auto),
         [a], [b], [c], [d], [e],
@@ -1195,8 +1307,6 @@ _For issue \#78_
         cellx(colspan: 3)[#lorem(15)], none, none,
     )
 
-- Table no longer has overlap:
-
     #unbreakable-tablex(
         columns: (1fr, 1fr, auto, auto, auto),
         fit-spans: (x: true),
@@ -1210,7 +1320,7 @@ _For issue \#78_
 
 _For issue \#56_
 
-- Table is normal:
+- Columns should have the same size in all samples below:
 
     #unbreakable-tablex(
         columns: (auto, auto, 1fr),
@@ -1220,8 +1330,6 @@ _For issue \#56_
         [A], [BC], [D]
     )
 
-- Second column too large:
-
     #unbreakable-tablex(
         columns: (auto, auto, 1fr),
         colspanx(3)[Hello world! Hello!],
@@ -1231,8 +1339,6 @@ _For issue \#56_
         [A], [BC], [D]
     )
 
-- Second column is now normal:
-
     #unbreakable-tablex(
         columns: (auto, auto, 1fr),
         fit-spans: (x: true),
@@ -1243,22 +1349,35 @@ _For issue \#56_
         [A], [BC], [D]
     )
 
-    #unbreakable-tablex(
-        columns: (auto, auto, 1fr),
-        fit-spans: true,
-        colspanx(3)[Hello world! Hello!],
-        [A], [BC], [D],
-        [A], [BC], [D],
-        [A], [BC], [D],
-        [A], [BC], [D]
-    )
 
-    #unbreakable-tablex(
-        columns: (auto, auto, 1fr),
-        colspanx(3, fit-spans: (x: true))[Hello world! Hello!],
-        [A], [BC], [D],
-        [A], [BC], [D],
-        [A], [BC], [D],
-        [A], [BC], [D]
-    )
-]
+#set page(height: auto, width: auto)
+
+Ensure the heuristic doesn't apply on auto width pages.
+Second table should have a longer second column.
+
+#tablex(
+    columns: (auto, auto, 1fr),
+    [A], [BC], [D],
+    [A], [BC], [D],
+    [A], [BC], [D],
+    [A], [BC], [D]
+)
+
+#tablex(
+    columns: (auto, auto, 1fr),
+    colspanx(3)[Hello world! Hello!],
+    [A], [BC], [D],
+    [A], [BC], [D],
+    [A], [BC], [D],
+    [A], [BC], [D]
+)
+
+#tablex(
+    columns: (auto, auto, 1fr),
+    fit-spans: (x: true),
+    colspanx(3)[Hello world! Hello!],
+    [A], [BC], [D],
+    [A], [BC], [D],
+    [A], [BC], [D],
+    [A], [BC], [D]
+)
diff --git a/tablex.typ b/tablex.typ
@@ -1257,12 +1257,13 @@
 }
 
 // calculate the size of auto columns (based on the max width of their cells)
-#let determine-auto-columns(grid: (), styles: none, columns: none, inset: none, align: auto, fit-spans: none) = {
+#let determine-auto-columns(grid: (), styles: none, columns: none, inset: none, align: auto, fit-spans: none, page-width: 0pt) = {
     assert(styles != none, message: "Cannot measure auto columns without styles")
     let total_auto_size = 0pt
     let auto_sizes = ()
     let new_columns = columns
 
+    let all-frac-columns = columns.enumerate().filter(i-col => type(i-col.at(1)) == _fraction-type).map(i-col => i-col.at(0))
     for (i, col) in columns.enumerate() {
         if col == auto {
             // max cell width
@@ -1289,6 +1290,23 @@
                     // `fit-spans.x == true` (it requests to not expand
                     // columns).
                     if last-auto-col == i and this-cell-can-expand-columns {
+                        let cell-spans-all-frac-columns = pcell.colspan > 1 and all-frac-columns.len() > 0 and all-frac-columns.all(i => pcell.x <= i and i < (pcell.x + pcell.colspan))
+                        if cell-spans-all-frac-columns and page-width != 0pt and not is-infinite-len(page-width) {
+                            // HEURISTIC (only effective when the page width isn't 'auto' / infinite):
+                            // If this cell can expand auto cols, but it already
+                            // spans all fractional columns, then don't expand
+                            // this auto column, as the cell would already have
+                            // all remaining available space for itself anyway
+                            // through the fractional columns spanned.
+                            // Effectively, ignore this colspan - it will already
+                            // have the max space possible, since, eventually,
+                            // auto columns will be reduced to fit in the available
+                            // size.
+                            // For 'auto'-width pages, fractional columns will
+                            // always have 0pt width, so this doesn't apply.
+                            return max
+                        }
+
                         // take extra inset as extra width or height on 'auto'
                         let cell_inset = default-if-auto(pcell.inset, inset)
 
@@ -1425,7 +1443,7 @@
     // page_width == 0pt => page width is 'auto'
     // so we don't have to restrict our table's size
     if available_size >= 0pt or page_width == 0pt {
-        let auto_cols_result = determine-auto-columns(grid: grid, styles: styles, columns: columns, inset: inset, align: align, fit-spans: fit-spans)
+        let auto_cols_result = determine-auto-columns(grid: grid, styles: styles, columns: columns, inset: inset, align: align, fit-spans: fit-spans, page-width: page_width)
         let total_auto_size = auto_cols_result.total
         let auto_sizes = auto_cols_result.sizes
         columns = auto_cols_result.columns