Repository navigation
fix(studio): Layers rows show a kind icon, and only a group row has a caret - #5067
Conversation
… a caret Rows show what an element is (image, video, audio, vector, group, text or shape) instead of the tag's first letters. The caret column sits only on rows with children, a group's children indent under its icon, and rows are inset with rounded corners so the selection reads centred.
Edit accuracy: accurate 2040 (base branch 2040), smooth 1631 of thoseThe gate passes. Quarantined, measured but not gated (0) |
somanshreddy
left a comment
There was a problem hiding this comment.
Review at a0ea4709 (full PR). No blockers.
What holds.
layerKindOfreuses the inline editor's ownhasOnlyFormattingChildren, so the Text icon and in-place editing can't drift apart. ThehasOnlyFormattingChildrenexport is the only behaviour-neutral change indomEditInlineText.ts.isCompositionHostmoving intolayerKind.tskeeps the tint and the group icon on one check.- The 48 px step (
CARET + GAP + ICON + GAP) puts a child's icon where its parent's label starts. It matches the 8/8/56/56/104 test. - The new icons exist in
SystemIcons.tsx. Film, Music, Square and Type were already there.
Should consider:
- A caret on a "shape" row. The caret is drawn on
hasChildren(LayersPanel.tsx:458), andchildCountisgetDirectLayerChildren(el).length(domEditingElement.ts:385): any layer child of any element counts. So a plain wrapper<div>holding boxes, with nodata-hf-group, gets the expand caret and the Square/"Shape" icon. "Only a group's row has the expand caret" holds fordata-hf-groupand composition hosts, not for plain containers. Either returngroupfromlayerKindOfwhen the element has layer children, or narrow the claim. The tests' nested case usesdata-hf-group, so the plain-wrapper row isn't pinned either way.
Lows:
2. The icon's title (LayersPanel.tsx:516) shadows the row's title while hovering the icon. A draggable group's "Double-click to enter group" and a fixed layer's "This layer can't be reordered" don't show there.
3. <canvas> (and <iframe>) now read as "Shape". Before it was "Cn". If a canvas-backed layer (Lottie, three) is common in compositions, it may deserve its own kind or the image icon.
Mutations. I ran 8 mutants on layerKind.test.ts + layersPanelExport.test.tsx + the domEditInlineText tests; 6 were caught. The tree was restored and verified clean after each.
- Caught:
svgandaudiomappings dropped;.trim()removed;data-hf-groupcheck dropped;- old 22 px step;
- leaf spacer restored.
- Survived:
picturedropped from the image rule. No case covers it.- Every row rendering
KIND_ICON.shape. Onlydata-layer-kindis asserted, not the rendered icon, so a brokenKIND_ICONmap passes.
CI at a0ea4709: 68 pass, 20 pending, 6 skipping, 0 failing at review time.
What I ran: a read of the full runtime diff, plus the childCount source; vitest run on the 4 test files (37 passed); the 8 mutants. I didn't view the Before/After sheets.
This is a comment, not an approval.
— Somu
… icon keeps the row's hint
jrusso1020
left a comment
There was a problem hiding this comment.
Approving at e9090e1.
The earlier review points are resolved:
- A row's icon and caret now come from the same
hasChildren, so a plain wrapper with child layers shows as Group with a caret. - The icon's tooltip now includes the row's hint.
<picture>is covered.- The render test checks that four kinds draw four distinct icons.
Reuse: the classifier reuses the inline editor's text rule (hasOnlyFormattingChildren) and moves isCompositionHost rather than copying it. The icons come from SystemIcons, with the same Type/Film/Square set the property panel header already uses for text, media and other. I found no existing row-kind helper this should have called. The old two-letter tag table is gone, so the change removes code overall.
Tests: layerKind, layersPanelExport, LayersPanel and domEditInlineText pass 46/46 three times, and tsc --noEmit is clean. I ran 8 mutations and 7 were caught: dropping hasChildren from the classifier or the panel, dropping the whitespace-only text check, dropping the composition-host check, dropping the hint from the icon's tooltip, putting the leaf placeholder back, and changing the indent.
Non-blocking:
- Swapping the audio icon for the shape icon passes, because no render test has an
<audio>row. One audio row in the render test would pin it. - A paragraph the inline editor won't edit, such as
<p>H<sub>2</sub>O</p>, reads as Shape. The test pins this on purpose and it follows the editor's rule, but someone will see a Square on what looks like text.
CI: 32 pass and 28 still running at this head, with no failures so far.
— Rames
There was a problem hiding this comment.
Approving at a70f121b. The only change since my approval at e9090e18 is layerKindOf, which now checks text before the has-children group rule.
- Order:
data-hf-groupand composition hosts are still groups first. Then text with only formatting children. Then a row with children becomes a group, else a shape. So<p>Hi <span class="x">there</span></p>keeps the Text icon, and a plain<div>wrapper is still a Group. - Tests: the Layers tests pass three runs in a row (35/35), and
tsc --noEmitis clean in studio. - Mutations: restoring the old order (
hasChildren ||back in the group check) fails the new assertion. Dropping the finalhasChildren ? "group"fails the wrapper assertion. Both are pinned.
The note from my last review stands, and nothing here changes it: <p>H<sub>2</sub>O</p> still is not Text, because sub is not a formatting tag. It shows as Shape, or as Group when the sub is its own layer row. This is non-blocking.
— Rames
What a user can do now
In Studio's Layers panel, each row says what the element is with an icon people know: image, video, audio, vector, group, text or shape. Before, it showed the tag's first letters ("DI" for a div, "P", "H1"). Rows line up at the left unless they are nested: only a row with children has the expand caret (leaf rows no longer reserve its space), and each child's icon starts where its parent's label starts, one step per nesting level. Rows are inset on both sides with rounded corners, so the selected row's highlight is centred instead of running into the panel's left edge and stopping at the scrollbar.
What changed
components/editor/layerKind.ts:layerKindOf(element).img/pictureis image;video,audioandsvgare video, audio and vector. Adata-hf-groupor a composition host is a group. Text comes next, then a row with layer children (the panel's existing child count) is a group, so a plain wrapper with a caret reads as a group. Text keeps its icon even when its styled spans give it a caret, and media rows keep theirs. An element with text whose descendants are all rich-text formatting tags is text: it calls the inline text editor's ownhasOnlyFormattingChildren(now exported), so the Text icon means Studio edits that element in place. Anything else is a shape.isCompositionHostmoves fromLayersPanel.tsxintolayerKind.ts, so the badge's composition tint and the group icon read the same check (data-composition-srcordata-composition-file).LayersPanel.tsx: the badge renders that kind's icon (with the kind as its tooltip and adata-layer-kindattribute) in the same 20px tile and the same selected and composition tints. The icon's tooltip carries the row's own hint ("Group · Double-click to enter group"), so hovering the icon no longer hides it. The tag-letter table is gone. A leaf row no longer reserves a caret column. One nesting level is a caret, the icon and their gaps (48px), so a child's icon starts where its parent's label starts. Rows, and the drill-in back row, aremx-1 rounded-md.SystemIcons.tsx: addsImageIcon,VectorandFolder(Phosphor Image, BezierCurve, FolderSimple), beside the existing Film, Music, Type and Square.Tests
layerKind.test.ts(new): 17 cases plus a children case covering every kind,<picture>, a wrapper with layer children (group), text with a classed span and children (still text), both composition-host attributes, rich text, text beside an image, a<sub>(not editable in place, so a shape), whitespace-only and media-in-a-div. It failed first because the module did not exist. Dropping the host check from the group rule fails both host cases; replacing the text check withtruefails the image and<sub>cases.layersPanelExport.test.tsx: rows read their names, not tag letters ("Title", not "DiTitle"). A new case over a group nested in a group checks the kinds (text, group, image, group, image), that only a group's row starts with the caret, that a top-level leaf starts with its icon, the indents 8, 8, 56, 56 and 104px, that a plain wrapper with children is a group with a caret, that four kinds render four different icons, and that each icon's tooltip keeps its row's hint. It failed first on main ("DiTitle"). Restoring the leaf placeholder fails it, and so does the old 22px step.layerKind,layersPanelExport,LayersPanelanddomEditInlineTexttests together: 46 passed, 3 runs in a row. Each of these mutants fails at least one test: ignoring children in the group rule, checking children before text, rendering every row's icon as a shape, droppingpicture, and a tooltip without the row's hint.tsc --noEmitin packages/studio shows no errors besides core's generated runtime file, which a fresh tree does not have. The comment-share ratchet passes against the merge base, and oxfmt and oxlint are clean on the changed files.What I did not exercise
<audio>has no row. The unit test covers its icon mapping.Before
Studio's Layers panel on main (6bb85af), Vite dev server, a fixture with text, rich text, an image, a video, an svg, a long name, a dozen boxes and a two-child group. Shots are at 2x with the list scrolled to the end and Back selected: the window 1440 wide (left) and 1100 wide (right), light then dark.
2x zooms, light then dark: the selected row's highlight runs to the left edge, and the group's children start past the group's badge.
Inside the group (double-click its row), light then dark: the back row runs edge to edge.
After
This branch, same fixture and views (captured at a70f121; every head since a0ea470 was pixel-identical in all ten views).
2x zooms: the selected row (light, dark) is inset on both sides with rounded corners. In the group (1440 light and dark, then 1100 light and dark), each child's icon starts where the group's label starts.
Inside the group, light then dark: the back row is inset like the rows under it.