[CP Staging] fix: keep a capped column from claiming width it does not need

src/components/Table/calculateDynamicColumnWidths.ts

@@ -5,7 +5,7 @@ type DynamicColumnConstraints = {
     /** Width the column needs to render its widest content and its header label in full, including non-text extras. */
     contentWidth: number;
 
-    /** Smallest width the column may be squeezed to. A column that must never truncate sets this to its content width. */
+    /** Smallest width the column may be squeezed to, capped at `contentWidth`. Set to `contentWidth` to never truncate. */
     minWidth: number;
 
     /** Largest width the column may claim. Content past it truncates instead of widening the column any further. */
@@ -125,10 +125,11 @@ function distributeAvailableWidth(desiredWidths: number[], maxWidths: number[],
  * 1. Every column's content fits inside an equal share of the available width, so the columns stay equal (`1fr`).
  * 2. The content fits overall but unevenly, so a column whose content can't fit an equal share takes exactly the width
  *    it needs, and the remaining columns split what's left equally. A column with long content grows only as far as its
- *    content, rather than also claiming the largest share of the slack.
- * 3. The content does not fit, so the columns are squeezed toward their minimum widths, in proportion to how much room
- *    each has to give up. Free-text columns truncate as they shrink; a column holding a known, short set of values has
- *    its content width as its minimum, so it keeps every value in full.
+ *    content, rather than also claiming the largest share of the slack. Columns that resolve wider than the row fall
+ *    through to behavior 3.
+ * 3. The columns need more room than the row has, so they are squeezed toward their minimum widths, in proportion to
+ *    how much room each has to give up. Free-text columns truncate as they shrink; a column holding a known, short set
+ *    of values has its content width as its minimum, so it keeps every value in full.
  * 4. Even the minimum widths don't fit, so the columns stop there and the table scrolls horizontally. Scrolling is
  *    reserved for a table with genuinely too many columns rather than one long value.
  *
@@ -156,10 +157,13 @@ function calculateDynamicColumnWidths(constraints: DynamicColumnConstraints[], a
     // the columns split what's left equally.
     const totalDesiredWidth = sum(desiredWidths);
     if (totalDesiredWidth <= availableWidth) {
-        return {
-            widths: roundWidths(distributeAvailableWidth(desiredWidths, maxWidths, availableWidth), availableWidth, maxWidths),
-            shouldScrollHorizontally: false,
-        };
+        // A capped column can grow into room a later one needs, so the split is checked before it is used. Rounded
+        // first, so fractions don't fail the check.
+        const distributedWidths = roundWidths(distributeAvailableWidth(desiredWidths, maxWidths, availableWidth), availableWidth, maxWidths);
+
+        if (sum(distributedWidths) <= availableWidth) {
+            return {widths: distributedWidths, shouldScrollHorizontally: false};
+        }
     }
 
     // 4. Even squeezed to their minimums the columns don't fit, so they stop there and the table scrolls. Rounding up
@@ -169,7 +173,8 @@ function calculateDynamicColumnWidths(constraints: DynamicColumnConstraints[], a
     // the dynamic ones to share. Sizing them to their own content is what keeps an empty column narrow in that case,
     // rather than leaving every column an equal share of room the table never had. Whether the table has been measured
     // at all is the caller's question, answered before it works out a budget.
-    const minWidths = constraints.map((constraint, index) => Math.min(constraint.minWidth, maxWidths.at(index) ?? 0));
+    // Capped at the desired width, so the room a column has to give up can't come out negative.
+    const minWidths = constraints.map((constraint, index) => Math.min(constraint.minWidth, desiredWidths.at(index) ?? 0));
     const totalMinWidth = sum(minWidths);
     if (totalMinWidth >= availableWidth) {
         return {
@@ -178,16 +183,26 @@ function calculateDynamicColumnWidths(constraints: DynamicColumnConstraints[], a
         };
     }
 
-    // 3. The content doesn't fit, so every column gives up room in proportion to how much it has to give. A column whose
-    // minimum is its content width has nothing to give and keeps its content in full.
+    // 3. The columns need more room than the row has, so every one of them gives up space in proportion to how much it
+    // has to give. A column whose minimum is its content width has nothing to give and keeps its content in full.
     const totalSqueezableWidth = totalDesiredWidth - totalMinWidth;
+
+    // Nothing to squeeze, so there is no ratio to work out.
+    if (totalSqueezableWidth <= 0) {
+        return {
+            widths: minWidths.map((minWidth) => Math.ceil(minWidth)),
+            shouldScrollHorizontally: false,
+        };
+    }
+
     const squeezeRatio = (availableWidth - totalMinWidth) / totalSqueezableWidth;
 
     return {
         widths: roundWidths(
             desiredWidths.map((desiredWidth, index) => {
                 const minWidth = minWidths.at(index) ?? 0;
-                return minWidth + (desiredWidth - minWidth) * squeezeRatio;
+                // Reached with room to spare, the share exceeds what the column asked for, so a cap still applies.
+                return Math.min(minWidth + (desiredWidth - minWidth) * squeezeRatio, maxWidths.at(index) ?? 0);
             }),
             availableWidth,
             maxWidths,

src/components/Table/types.ts

@@ -67,7 +67,7 @@ type TableColumnDynamicSizing<DataType extends TableData = TableData> = {
     /** Whether this column's values come from a fixed set (a role, a status), so it always fits them in full and never truncates. */
     shouldFitContent?: boolean;
 
-    /** Smallest width this column may be squeezed to. Defaults to a readable width, or the column's content width when that is narrower. */
+    /** Smallest width this column may be squeezed to, capped at its content width. Defaults to a readable width. */
     minWidth?: number;
 
     /**

src/components/Table/useDynamicColumnWidths.ts

@@ -138,8 +138,9 @@ function useDynamicColumnWidths<DataType extends TableData, ColumnKey extends st
     const totalColumnCount = columns.length + (hasSelectionColumn ? 1 : 0);
     const totalGapWidth = Math.max(totalColumnCount - 1, 0) * styles.gap3.gap;
     const rowChromeWidth = (styles.mh5.marginHorizontal + styles.ph3.paddingHorizontal) * 2;
-    const availableWidth = tableWidth - rowChromeWidth - totalGapWidth - fixedColumnsWidth - selectionColumnWidth;
-
+    // Floored because the tracks are whole px. A fractional budget leaves a fraction over once they are rounded, and
+    // handing it to a column would put a sub-pixel track in the row. Rounding down keeps the columns inside the table.
+    const availableWidth = Math.floor(tableWidth - rowChromeWidth - totalGapWidth - fixedColumnsWidth - selectionColumnWidth);
     if (availableWidth <= 0) {
         return noDynamicWidths;
     }
@@ -176,7 +177,6 @@ function useDynamicColumnWidths<DataType extends TableData, ColumnKey extends st
     }
 
     const {widths, shouldScrollHorizontally} = calculateDynamicColumnWidths(constraints, availableWidth);
-
     // The columns fit equally, which is exactly what the static `1fr` tracks already do.
     if (widths.length === 0) {
         return noDynamicWidths;

tests/unit/calculateDynamicColumnWidthsTest.ts

@@ -79,6 +79,66 @@ describe('calculateDynamicColumnWidths', () => {
             expect(result.widths).toEqual([200, 200]);
             expect(result.shouldScrollHorizontally).toBe(false);
         });
+
+        it('keeps the columns inside the row when a capped column needs less than its cap', () => {
+            // Given the Company cards table at 1163px: a member column at 359px, a card column whose content overflows
+            // its 180px cap, a card name column at 353px, an export account column whose 154px of content stops short
+            // of the same cap, and an actions column that must fit its 112px Assign button
+            const constraints = [buildConstraints(359, 160), buildConstraints(331, 331, 180), buildConstraints(353, 120), buildConstraints(154, 120, 180), buildFitContentConstraints(112)];
+
+            // When the widths are resolved
+            const result = calculateDynamicColumnWidths(constraints, 1163);
+
+            // Then the columns are squeezed instead of letting the export account column grow into its cap, so they add
+            // up to the row and the actions cell is not pushed outside the table
+            expect(result.widths).toEqual([362, 180, 355, 154, 112]);
+            expect(sumOf(result.widths)).toBe(1163);
+            expect(result.shouldScrollHorizontally).toBe(false);
+        });
+
+        it('holds a capped column to its maximum while the others take up the slack', () => {
+            // Given a row with more room than the columns' content needs, where resolving it leaves the last column a
+            // share wider than its 50px cap
+            const constraints = [buildFitContentConstraints(300), buildConstraints(30, 30, 60), buildConstraints(200, 10, 50)];
+
+            // When the widths are resolved
+            const result = calculateDynamicColumnWidths(constraints, 400);
+
+            // Then the capped column stops at 50px and the room it cannot use goes to the column that can
+            expect(result.widths).toEqual([320, 30, 50]);
+            expect(result.widths.at(2)).toBeLessThanOrEqual(50);
+            expect(result.shouldScrollHorizontally).toBe(false);
+        });
+
+        it('resolves a row where no column has any room to give up', () => {
+            // Given two columns that both have to fit their content, so neither can be squeezed, in a row a capped
+            // column claims more of than it needs
+            const constraints = [buildFitContentConstraints(70), buildConstraints(30, 30, 40)];
+
+            // When the widths are resolved
+            const result = calculateDynamicColumnWidths(constraints, 105);
+
+            // Then each column takes its own width rather than a ratio worked out by dividing by zero
+            expect(result.widths).toEqual([70, 30]);
+            expect(result.shouldScrollHorizontally).toBe(false);
+        });
+
+        it('holds a column whose minimum is wider than its content to its content', () => {
+            // Given an empty column asking for a minimum wider than the content it has, next to a column that claims
+            // more of the row than it needs
+            const constraints = [
+                {contentWidth: 0, minWidth: 1, maxWidth: 2},
+                {contentWidth: 5, minWidth: 0, maxWidth: 8},
+            ];
+
+            // When the widths are resolved
+            const result = calculateDynamicColumnWidths(constraints, 6);
+
+            // Then it takes its content width, because a minimum it never asked to fill would give it a negative share
+            // of the squeeze and a negative grid track
+            expect(result.widths).toEqual([0, 6]);
+            expect(sumOf(result.widths)).toBeLessThanOrEqual(6);
+        });
     });
 
     describe('behavior 3: the content does not fit, so the columns are squeezed', () => {