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





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.