From c3b113b97889d1fdf6adb99665f12ef40e76c48b Mon Sep 17 00:00:00 2001 From: pablofdezr Date: Wed, 15 Jul 2026 18:45:39 +0200 Subject: [PATCH] fix(trees): keep deep flattened paths readable (#941) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every segment of a flattened directory row rendered its own Truncate, so each one shrank and truncated independently. A deep single-child path spent the row on ellipses instead of on the path ("t… / kc… / … / se…"), and a narrow enough row showed nothing else. Split the path where the `leaf-path` split does, at the last separator, and hand the halves to MiddleTruncate with the terminal directory prioritised. The leading run absorbs the shrinking while the directory the row actually opens holds its width: "test / kotlin / com… / server". MiddleTruncate renders each half twice, once hidden to detect overflow, which would have duplicated the per-segment data attributes that drag and drop and the tests query. Only the text metrics of the hidden copy matter, so OverflowText takes a `measureChildren` stand-in for it and MiddleTruncate a `measureContents` pair. Renaming keeps the flex line it had, where the input sizes itself against a flex item. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../trees/src/components/OverflowText.tsx | 43 +++++++-- packages/trees/src/render/FileTreeView.tsx | 95 +++++++++++++++---- packages/trees/src/style.css | 20 +++- .../test/e2e/file-tree-composition.pw.ts | 64 +++++++++++++ .../test/file-tree-keyboard-selection.test.ts | 34 +++++-- 5 files changed, 221 insertions(+), 35 deletions(-) diff --git a/packages/trees/src/components/OverflowText.tsx b/packages/trees/src/components/OverflowText.tsx index a382278c6..c46f2e331 100644 --- a/packages/trees/src/components/OverflowText.tsx +++ b/packages/trees/src/components/OverflowText.tsx @@ -20,9 +20,17 @@ export interface OverflowTextProps extends PropsWithChildren { className?: string; marker?: ComponentChildren | ((props: MarkerProps) => ComponentChildren); variant?: 'default' | 'fade'; + // Stands in for `children` in the hidden copy that detects overflow. Only the + // text metrics of that copy matter, so pass a plain-text equivalent when + // `children` holds things that should exist once in the DOM: data attributes + // that get queried, ids, form controls. + measureChildren?: ComponentChildren; } -export type MiddleTruncateProps = Omit & +export type MiddleTruncateProps = Omit< + OverflowTextProps, + 'mode' | 'children' | 'measureChildren' +> & AllowableContentGroups & { minimumLength?: number; priority?: 'start' | 'end' | 'equal'; @@ -51,10 +59,14 @@ type AllowableContentGroups = | { children?: never; contents: [ComponentChildren, ComponentChildren]; + // Per-segment stand-ins for the hidden copies that detect overflow, in the + // same order as `contents`. See `measureChildren` on OverflowTextProps. + measureContents?: [ComponentChildren, ComponentChildren]; } | { contents?: never; children: string; + measureContents?: never; }; // When a split boundary lands adjacent to whitespace, the trailing/leading @@ -210,7 +222,8 @@ function OverflowMarker({ function OverflowContent(options: OverflowTextProps) { 'use no memo'; - const { mode, children } = options; + const { mode, children, measureChildren } = options; + const overflowChildren = measureChildren ?? children; // The inner span wrapper here is only needed to implement // the right aligned internals for fruncate @@ -220,7 +233,11 @@ function OverflowContent(options: OverflowTextProps) { {mode === 'fruncate' ? {children} : children}
- {mode === 'fruncate' ? {children} : children} + {mode === 'fruncate' ? ( + {overflowChildren} + ) : ( + overflowChildren + )}
); @@ -228,6 +245,7 @@ function OverflowContent(options: OverflowTextProps) { export function OverflowText({ children, + measureChildren, mode = 'truncate', marker = '…', variant = 'default', @@ -235,7 +253,11 @@ export function OverflowText({ }: OverflowTextProps): JSX.Element { 'use no memo'; const contentNode = ( - + {children} ); @@ -291,6 +313,7 @@ export function Fruncate({ export function MiddleTruncate({ children, contents, + measureContents, priority = 'end', split = 'center', minimumLength = 12, @@ -306,8 +329,16 @@ export function MiddleTruncate({ console.error('MiddleTruncate: contents must be an array of two items'); return null; } - firstSegment = {contents[0]}; - secondSegment = {contents[1]}; + firstSegment = ( + + {contents[0]} + + ); + secondSegment = ( + + {contents[1]} + + ); } else { // TODO: figure out how to support ReactNode children in the future if (typeof children !== 'string') { diff --git a/packages/trees/src/render/FileTreeView.tsx b/packages/trees/src/render/FileTreeView.tsx index de219c62b..38eb1d73e 100644 --- a/packages/trees/src/render/FileTreeView.tsx +++ b/packages/trees/src/render/FileTreeView.tsx @@ -1,6 +1,6 @@ /** @jsxImportSource preact */ import { Fragment } from 'preact'; -import type { JSX } from 'preact'; +import type { ComponentChildren, JSX } from 'preact'; import { useCallback, useEffect, @@ -41,6 +41,7 @@ import type { FileTreeItemHandle, FileTreeRowDecoration, FileTreeVisibleRow, + FileTreeVisibleSegment, } from '../model/publicTypes'; import { FILE_TREE_DEFAULT_ITEM_HEIGHT, @@ -86,30 +87,86 @@ function formatFlattenedSegments( return renameInput ?? row.name; } - return ( - - {segments.map((segment, index) => { - const isLast = index === segments.length - 1; - return ( + const isRenaming = renameInput != null; + + const renderSegment = ( + segment: FileTreeVisibleSegment, + content: ComponentChildren + ): JSX.Element => ( + + {content} + + ); + + // Renaming keeps every segment on a flex line of its own, where the terminal + // segment becomes the input. The input sizes itself against a flex item, and + // it has to stay visible rather than be truncated away. + if (isRenaming) { + return ( + + {segments.map((segment, index) => ( - - {isLast && renameInput != null ? ( + {renderSegment( + segment, + index === segments.length - 1 ? ( renameInput ) : ( {segment.name} - )} - + ) + )} {index < segments.length - 1 ? ' / ' : ''} - ); - })} + ))} + + ); + } + + // Otherwise the path splits at its last separator, the same place the + // `leaf-path` split picks. Truncating each segment on its own spent the row on + // ellipses ("t… / kc… / … / se…") instead of on the path; truncating the run + // as a whole would drop the terminal directory, which is the one the row + // actually opens. So the leading run absorbs the shrinking and the terminal + // directory holds its ground: "test / kotlin / com… / server". + const leading = segments.slice(0, -1); + const terminal = segments[segments.length - 1]; + if (terminal == null) { + return row.name; + } + + // This separator opens the terminal segment, so its leading space lands at + // the start of that line box, where the browser collapses it away ("b/ c"). + // A non-breaking space survives. + const terminalSeparator = '\u00a0/ '; + + // The hidden copies that detect overflow measure the same text without + // duplicating the per-segment markup that drag and drop and the tests query. + const leadingText = leading.map((segment) => segment.name).join(' / '); + const terminalText = `${terminalSeparator}${terminal.name}`; + + return ( + + ( + + {renderSegment(segment, segment.name)} + {index < leading.length - 1 ? ' / ' : ''} + + )), + + {terminalSeparator} + {renderSegment(terminal, terminal.name)} + , + ]} + measureContents={[leadingText, terminalText]} + /> ); } diff --git a/packages/trees/src/style.css b/packages/trees/src/style.css index ef8e2d267..8661f6f9f 100644 --- a/packages/trees/src/style.css +++ b/packages/trees/src/style.css @@ -856,8 +856,10 @@ z-index: 2; /* Flattened segment markers sit high enough to cover the row outline unless - * their painted background is inset by the focus ring width. */ - [data-item-flattened-subitems] { + * their painted background is inset by the focus ring width. The marker + * sits outside the segments themselves, so inset from the section that + * contains them. */ + [data-item-section='content']:has([data-item-flattened-subitems]) { --truncate-marker-block-inset: var(--trees-focus-ring-width); } @@ -930,7 +932,21 @@ } /* Flattened Directory Parts */ + /* + Outside renaming the segments are a middle-truncate group, so the leading run + can shrink while the terminal directory holds its width. As flex items each + segment shrank and truncated on its own, which spent a narrow row on ellipses + rather than on the path. + */ [data-item-flattened-subitems] { + display: block; + min-width: 0; + } + /* + Renaming swaps the terminal segment for an input, which sizes itself against a + flex item rather than against the row. + */ + [data-item-flattened-subitems]:has([data-item-rename-input]) { display: inline-flex; align-items: center; gap: 2px; diff --git a/packages/trees/test/e2e/file-tree-composition.pw.ts b/packages/trees/test/e2e/file-tree-composition.pw.ts index ae898aeda..33a724be4 100644 --- a/packages/trees/test/e2e/file-tree-composition.pw.ts +++ b/packages/trees/test/e2e/file-tree-composition.pw.ts @@ -341,6 +341,70 @@ test.describe('file-tree composition surfaces', () => { expect(fileMarkerPadding.bottom).toBe(0); }); + test('a narrow flattened row truncates the leading run and keeps the terminal directory', async ({ + page, + }) => { + await page.goto('/test/e2e/fixtures/file-tree-composition.html'); + await page.waitForFunction( + () => window.__fileTreeCompositionFixtureReady === true + ); + + const measureFlattenedRow = (mountWidth: string) => + page.evaluate((width) => { + const mount = document.querySelector( + '[data-file-tree-composition-mount]' + ); + if (!(mount instanceof HTMLElement)) { + throw new Error('Expected file-tree composition mount.'); + } + mount.style.width = width; + + const host = document.querySelector('file-tree-container'); + const row = Array.from( + host?.shadowRoot?.querySelectorAll('button[data-type="item"]') ?? [] + ).find( + (candidate) => + candidate.querySelector('[data-item-flattened-subitems]') != null + ); + if (!(row instanceof HTMLElement)) { + throw new Error( + 'Expected a flattened row in the composition fixture.' + ); + } + + const segments = Array.from( + row.querySelectorAll('[data-item-flattened-subitem]') + ); + const terminal = segments.at(-1); + + return { + // A segment carrying its own marker is a segment truncating alone. + segmentsWithOwnMarker: segments.filter( + (segment) => segment.querySelector('[data-truncate-marker]') != null + ).length, + segmentCount: segments.length, + terminalName: terminal?.textContent ?? null, + terminalWidth: terminal?.getBoundingClientRect().width ?? 0, + }; + }, mountWidth); + + const wide = await measureFlattenedRow('360px'); + expect(wide.segmentCount).toBeGreaterThan(1); + + const narrow = await measureFlattenedRow('120px'); + + // Narrowing the row spends the shrinking on the leading run. Every segment + // truncating on its own is what left a deep path rendering as + // "t… / kc… / … / se…", with none of it readable. + expect(wide.segmentsWithOwnMarker).toBe(0); + expect(narrow.segmentsWithOwnMarker).toBe(0); + + // The terminal directory is the one the row opens, so it keeps its width + // rather than being truncated away with the rest of the path. + expect(narrow.terminalName).toBe(wide.terminalName); + expect(narrow.terminalWidth).toBeCloseTo(wide.terminalWidth, 1); + }); + test('keyboard navigation retargets the focused row trigger away from a stale hover', async ({ page, }) => { diff --git a/packages/trees/test/file-tree-keyboard-selection.test.ts b/packages/trees/test/file-tree-keyboard-selection.test.ts index aed3c32af..241a777de 100644 --- a/packages/trees/test/file-tree-keyboard-selection.test.ts +++ b/packages/trees/test/file-tree-keyboard-selection.test.ts @@ -499,17 +499,35 @@ describe('file-tree keyboard and selection', () => { '[data-item-flattened-subitems]' ); - expect(flattenedContainer?.innerHTML).toContain(' / '); - expect( + // The rendered path reads as one run. The separator opening the terminal + // segment uses a non-breaking space so it survives at the start of its + // line box. + const visibleText = Array.from( flattenedContainer?.querySelectorAll( - ':scope > [data-item-flattened-subitem]' - ).length - ).toBe(2); + '[data-truncate-content="visible"]' + ) ?? [] + ) + .map((node) => node.textContent) + .join(''); + expect(visibleText).toBe('src\u00a0/ lib'); + + // Each segment is in the DOM once. The copies that measure overflow carry + // plain text, so they never duplicate a segment's identity. + expect( + Array.from( + flattenedContainer?.querySelectorAll( + '[data-item-flattened-subitem]' + ) ?? [] + ).map((node) => node.getAttribute('data-item-flattened-subitem')) + ).toEqual(['src/', 'src/lib/']); + + // No span exists solely to hold a separator, which would make it a hover + // and drop target of its own. expect( - flattenedContainer?.querySelector( - ':scope > span:not([data-item-flattened-subitem])' + Array.from(flattenedContainer?.querySelectorAll('span') ?? []).some( + (node) => node.textContent?.trim() === '/' ) - ).toBeNull(); + ).toBe(false); fileTree.cleanUp(); } finally {