Improved mini-table resource name truncation - #3182
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Okay this is problematic if the other columns are long also, might need another approach. |
|
This could be even more clever by using absolute pixel widths and being container aware. That way if its still got space but a another column is truncated adjusting to fit. But I'm not sure it's worth the extra complexity and DOM measurements. |
| cell: (item: T, index: number) => React.ReactNode | ||
| } | ||
| } & ( | ||
| | { cell: (item: T, index: number) => React.ReactNode } |
There was a problem hiding this comment.
I might be missing something, but I don't believe we're using this index, so it can be dropped here and at the callsite (column.cell(item, index) on line 274).
fakemonster
left a comment
There was a problem hiding this comment.
i'm quite pleased with how well the offscreen measuring works, even if my yagni senses are tingling!
| const cache = new Map<string, number>() | ||
|
|
||
| /** | ||
| * Measure the rendered pixel width of `text` using Canvas `measureText`. |
There was a problem hiding this comment.
this is relatively quick, though you could certainly give up a meaningful chunk of the render cycle calculating a sufficiently large table (i mean on the order of, say, thousands of unique cells). realistically i'm not sure that's a reachable scale, but given the constraints on names (which are most of these columns), i'd wager that text.length is a good-enough estimation that also eliminates the need for a cache
There was a problem hiding this comment.
In this specific use case ... perhaps. Though it depends very much on the character choice and combinations. For columns as narrow as these get that effect can be quite pronounced.
I mention it briefly here: #3182 (comment)
Would also say there's value in consistency beyond this use case. That's to say, rather than adding an exemption here and using string length, it's probably clearer to use text measurement everywhere (which is my recommendation for the Truncate function).
Mini-tables are mini, so we can safely say we're not going to be calculating significant numbers of cells.
| const floor = equalShare / spread | ||
| const ceiling = equalShare * spread | ||
| const clamped = maxWidths.map((w) => | ||
| w > 0 ? Math.min(Math.max(w, floor), ceiling) : 0 |
There was a problem hiding this comment.
there are some convenience functions from remeda you can use here, R.clamp for the positive conditional on this line, and R.sum for L225 and L210 above. nbd
| * distribute remaining table width proportionally. Returns a per-column | ||
| * style object (undefined for fit-to-content columns). | ||
| */ | ||
| function useColumnWidths<T>(columns: Column<T>[], items: T[]) { |
There was a problem hiding this comment.
it prevents having to reason about the kinds of things coming out of this function if you just uniformly return "an object full of props". it's kind of easier to express the idea as a diff:
diff --git a/app/ui/lib/MiniTable.tsx b/app/ui/lib/MiniTable.tsx
index 56dd76aa..0ff54366 100644
--- a/app/ui/lib/MiniTable.tsx
+++ b/app/ui/lib/MiniTable.tsx
@@ -184,12 +184,15 @@ function isTextColumn<T>(
* distribute remaining table width proportionally. Returns a per-column
* style object (undefined for fit-to-content columns).
*/
-function useColumnWidths<T>(columns: Column<T>[], items: T[]) {
+function useColumnWidths<T>(
+ columns: Column<T>[],
+ items: T[]
+): Pick<React.ComponentProps<'td'>, 'className' | 'style'>[] {
return useMemo(() => {
const hasTextCols = columns.some(isTextColumn)
if (!hasTextCols || items.length === 0) {
// Fall back to the old behavior: first column gets w-full
- return columns.map((_, i) => (i === 0 ? 'w-full' : undefined))
+ return columns.map((_, i) => (i === 0 ? { className: 'w-full' } : {}))
}
// Measure max natural text width per text column.
@@ -209,7 +212,7 @@ function useColumnWidths<T>(columns: Column<T>[], items: T[]) {
const textColCount = maxWidths.filter((w) => w > 0).length
const totalTextWidth = maxWidths.reduce((sum, w) => sum + w, 0)
if (totalTextWidth === 0 || textColCount === 0) {
- return columns.map((_, i) => (i === 0 ? 'w-full' : undefined))
+ return columns.map((_, i) => (i === 0 ? { className: 'w-full' } : {}))
}
// Max ratio between widest and narrowest text column.
@@ -226,9 +229,9 @@ function useColumnWidths<T>(columns: Column<T>[], items: T[]) {
// Text columns share available space proportionally; others fit content
return columns.map((col, i) => {
- if (!isTextColumn(col)) return undefined
+ if (!isTextColumn(col)) return {}
const pct = (clamped[i] / clampedTotal) * 100
- return { width: `${pct.toFixed(1)}%` } as const
+ return { style: { width: `${pct.toFixed(1)}%` } }
})
}, [columns, items])
}
@@ -263,11 +266,8 @@ export function MiniTable<T>({
items.map((item, index) => (
<Row tabIndex={0} aria-rowindex={index + 1} key={rowKey(item, index)}>
{columns.map((column, colIndex) => {
- const w = colWidths[colIndex]
- const className = typeof w === 'string' ? w : undefined
- const style = typeof w === 'object' ? w : undefined
return (
- <Cell key={colIndex} className={className} style={style}>
+ <Cell key={colIndex} {...colWidths[colIndex]}>
{isTextColumn(column) ? (
<TruncateCell text={column.text(item)} />
) : (There was a problem hiding this comment.
I like this
|
|
||
| const textColCount = maxWidths.filter((w) => w > 0).length | ||
| const totalTextWidth = maxWidths.reduce((sum, w) => sum + w, 0) | ||
| if (totalTextWidth === 0 || textColCount === 0) { |
There was a problem hiding this comment.
trivial, but since you're not gonna find a negative width, if one of these is true the other is true, so only one needs checking
Pulling a refactor out to keep noise out of #3182. Hoisting these column arrays means the `useMemo` in that PR will actually do something.
|
Let’s do the grid one, #3293 |
Alternative to #3182 proposed by Fable. Closes #2999. <details> <summary>🤖 Fable comparison to #3182</summary> Everything needed is in hand: both implementations, both PR discussions, the PR screenshots, and the notes from the previous session (`.claude/notes/2026-07-17-minitable-grid-rewrite.md`). No browser needed — the April screenshots on [#3182](#3182) demonstrate the key differentiating scenario directly, and the clamp logic they illustrate is unchanged after the rebase. ## Elegance The two branches converge on an identical public API (`text` vs `cell` columns, truncate-with-tooltip via the same scrollWidth check). The difference is entirely in how column widths get decided. **Measurement branch** ([#3182](#3182), +185/−20): canvas `measureText` per cell → clamp ratios to 2.5:1 via `sqrt(5/2)` → normalize to percentage widths. Its main structural weakness is that it re-derives layout outside the layout engine, and has to defeat the layout engine to do it: every text cell is absolutely positioned (`absolute inset-x-3`) specifically so content can't influence native table sizing, then JS reconstructs what the sizes should be from a canvas measurement. Supporting apparatus follows from that choice: `text-width.ts`, a hand-copied `'400 14px SuisseIntl'` / `0.03rem` font spec that must shadow `text-sans-md`, the spread/floor/ceiling heuristic, and a memoization contract that leaks into the API ("keep this array referentially stable"). On the plus side, it keeps genuine table display semantics and reuses `Table.Header`/`Table.HeadCell` from the big table. **Grid branch** ([#3293](#3293), +178/−175, net ~0 against main): the whole sizing policy is one declarative expression — `minmax(0, auto)` for text columns, `max-content` for cell columns — and the browser computes widths from the actual rendered glyphs. It deletes `text-width.ts`, the font constants, the clamp, the memo contract, and all 85 lines of `mini-table.css` nth-child tricks (styles now co-located). The cost is an ARIA tax: `display: grid`/`display: contents` strip implicit table semantics, so every element carries an explicit role and the file needs two jsx-a11y eslint disables. It also re-implements header-cell chrome inline rather than reusing `Table.HeadCell` — though main's css file was already a parallel implementation, so that's not a regression, just a missed reuse the other branch gets. On elegance the grid version wins on the axis that matters most here: the measurement branch simulates the layout engine; the grid branch instructs it. Benjamin explicitly identified container-aware sizing as the ideal in the PR thread ("This could be even more clever by using absolute pixel widths and being container aware… But I'm not sure it's worth the extra complexity") — the grid gets exactly that behavior for free, because it's the layout engine's native job. ## Scenarios with clearly different results **1. Long name next to short columns, with slack in the table — grid clearly better.** This is the case users actually hit ([#2999](#2999)). The measurement floor reserves ~22% for each short column and the ceiling caps the long one at ~56%, so a long name truncates while its neighbors sit half empty. The NIC screenshot in the #3182 thread shows exactly this: `asdasdasd-asdasdas…` cut off while `mock-vpc` and `mock-subnet` each float in ~2× the width they need. Concretely: a name measuring 300px next to two trivial columns in a 512px table gets 55.6% ≈ 285px under measurement (truncated, ~200px dead space nearby); under grid the row's max-contents sum to well under 512, so the full name renders. A grid track never truncates while slack exists anywhere in the row. **2. First paint before the webfont loads, or future font changes — grid correct by construction.** `measureText` against `SuisseIntl` before the font loads measures the fallback font, and nothing re-measures on font load; likewise, any future change to `text-sans-md` or letter-spacing silently desyncs the hardcoded constants. The grid version has no measurement to get wrong. Low frequency in practice (MiniTables appear after user interaction), but it's an entire bug class one approach can have and the other can't. **3. Short content everywhere — grid modestly better.** Compare the two "abc" disk screenshots on #3293: grid lets badge columns hug content and gives leftover to the name; measurement stretches every text column to its clamped percentage, so dividers land disconnected from content (visible in the `asda` NIC screenshot on #3182). Cosmetic, but consistently in grid's favor. **4. Two long text columns under real constraint — different, neither clearly better.** This is the one behavioral fork that's genuinely a matter of taste. Grid's track sizing is max-min water-filling: width distributes equally, short tracks freeze at content size, so the longest column absorbs all of the shortage beyond its equal share. Measurement shrinks proportionally (clamped), spreading the pain. Example: contents of 350px and 220px sharing 400px — grid yields 200/200 (shorter column 91% visible, longer 57%); measurement yields ~245/155 (both ~70% visible). Reasonable people could prefer either. **5. Legacy assistive tech — measurement clearly better, in principle.** It's the only scenario found where measurement wins: it keeps real table display semantics, while grid depends on explicit ARIA roles to patch over `display: contents` semantics-stripping, which was historically buggy in browsers (fixed in current engines, and the role-based Playwright locators pass per the prior session's e2e runs). If the console had to support an old AT/browser combination, that would favor #3182; given the console's modern-browser baseline, it's a theoretical edge. ## Verdict Grid dominates on rendered results in the scenarios users encounter (1–3); its costs are confined to the code (ARIA boilerplate, subgrid idiom, duplicated header chrome) rather than the output. The measurement branch's advantages are semantic purity that its own absolute-positioning hack partially undermines, and a taste-based difference in how two simultaneously-truncating columns split the shortage. If presenting this on the PRs, scenario 1 is the strongest exhibit — the #3182 thread's own screenshots demonstrate the truncate-despite-slack behavior, and Benjamin's "container aware… not worth the extra complexity" comment frames the grid version as delivering the behavior he wanted at negative net complexity. </details> # main <img width="553" height="226" alt="Screenshot 2026-07-17 at 6 24 32 PM" src="https://github.com/user-attachments/assets/affef6ef-26d4-4f82-bb48-cd0d1d48bc45" /> <img width="596" height="270" alt="Screenshot 2026-07-17 at 6 24 59 PM" src="https://github.com/user-attachments/assets/8b97d74b-ff28-41b2-ae61-2a6fb2176026" /> <img width="559" height="359" alt="Screenshot 2026-07-17 at 6 40 56 PM" src="https://github.com/user-attachments/assets/5ce26863-0bf0-40e5-822e-de5f9c8eb9ba" /> # This PR <img width="559" height="226" alt="Screenshot 2026-07-17 at 6 26 10 PM" src="https://github.com/user-attachments/assets/08b2d1fd-e70f-4cdb-b676-1ec8ddcafb2e" /> <img width="559" height="262" alt="Screenshot 2026-07-17 at 6 25 12 PM" src="https://github.com/user-attachments/assets/f99fdda0-a5a0-4c13-b6fb-f84b2fc0615c" /> <img width="300" height="123" alt="image" src="https://github.com/user-attachments/assets/6ff53c8b-434f-4366-8218-ffec69f574c5" /> <img width="561" height="353" alt="Screenshot 2026-07-17 at 6 40 15 PM" src="https://github.com/user-attachments/assets/b00fe1de-cd60-42b1-bf8b-467442098199" />





Fixes #2999
The problem with the
<Truncate>component is that it isn't aware of the size of the container, so you need a very conservative number and even then, due to the variable width of characters, it is not a reliable solution.Let's assume we always want the first cell to fill the width (needs some investigation in case there are outliers here). We can absolutely position the text within it, fill the size of the container and CSS truncate the text. Also prevents either from wrapping.
I still think we might want a pattern to show more properties that are not represented here, e.g. source. Or we allow the user to re-open the modal to see the properties and edit their choices.
Draft, needs further testing.