feat(studio): element groups#1736
Closed
miguel-heygen wants to merge 19 commits into
Closed
Conversation
Wrap selected elements in a <div data-hf-group> in the composition source, with coordinate rebasing so the members' on-screen positions are unchanged. Supports create (Cmd+G) / ungroup (Cmd+Shift+G or an inspector button), select-as-unit, canvas + layer-tree drill-in with a back breadcrumb, move/scale by reusing the single-element drag/resize intercepts, grouping inside sub-compositions, and member-union bounds shared across the selection, hover, and off-canvas overlays. Core: wrapElementsInHtml / unwrapElementsFromHtml mirror splitElementInHtml (round-trip identity, handles left/top declared in a CSS rule), plus wrap-elements / unwrap-elements file-mutation routes with finite-number coordinate validation. Also polishes the 3D transform widget (tangential to groups): depth via scroll-over-cube instead of a redundant perspective slider, a depth-scaled cube for realistic Z feedback, and a screen-Y-down orientation fix so the gizmo mirrors the element's rotation.
miguel-heygen
force-pushed
the
feat/element-groups
branch
from
fa8a1b5 to
74b421c
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Model the timeline as a structural TimelineRef (identifier OR member expression) instead of a string variable name, so the acorn read parser can detect and read the inline window.__timelines[id] = gsap.timeline() form — matching tweens by AST structure rather than identifier name. Static string/dot keys (single/double quote) supported; computed keys stay flagged unsupported; canonical const form unchanged.
…frames) Proves the acorn write path round-trips inline window.__timelines[id] = gsap.timeline() edits — the read-path member ref makes the writer emit window.__timelines[id].to(...) with no writer changes. Covers edit-in-place, add (incl. empty timeline), delete, keyframe add, remove-all-keyframes, and single-quote preservation.
…er path) Mirror the TimelineRef model into the recast parser/writer (gsapParser.ts) — the default studio-api write path — so window.__timelines[id] = gsap.timeline() reads AND round-trips edits (the emitter roots tweens at the member source). Static string/dot keys supported; computed keys stay unsupported; canonical const form unchanged (194 recast tests green). 9 inline read+write tests added.
The unsupported-pattern banner now clears for static window.__timelines["id"] = gsap.timeline() (the parser reports it editable), and the banner copy is retargeted to the genuinely-unsupported case: computed/dynamic keys (window.__timelines[var]).
Sub-comp internal elements (group wrappers + their children) carry no data-start, so the clip tree/manifest never enumerate them and the Scene row had nothing to expand into. Descend into each sub-comp host's DOM studio-side (useTimelineSyncCallbacks), collect groups + children as domClipChildren with parent links, and synthesize child rows spanning the host's bounds at expand time (useExpandedTimelineElements). Manifest stays lean (timed clips only). Verified: selecting a pill shows its siblings under the Scene row; selecting the group shows the group.
Two group selection bugs with animated members: 1) Empty space inside a group's overlay didn't hover/select the group. Members animated outside the wrapper's static box (110px box vs 340px member union), so elementsFromPoint hit only the full-bleed background there. Add a member-union hit-test fallback: a point inside a group's live member bounds resolves to that group (innermost wins). 2) After drilling into a group and selecting a child, nothing else was selectable — out-of-scope resolved to null. Make drill-in non-sticky: interacting outside the drilled group re-resolves normally and exits the drill-in, so a later click on the group selects it as a unit again.
Setting depth fired two un-awaited gsap mutations (transformPerspective, then z) to the same script. They raced read-modify-write: the second read the base before the first landed and wrote back without the other prop, so the lens or z reverted after a seek (depth 'didn't stick' after scrolling). The concurrent writes could also collide on the file and 404 the save. Batch both into one keyframe commit, exactly as commitPose/recenter already do for the rotation axes.
… keyframe promoteSetToKeyframes returned early when the playhead was at or before the set's start (t <= setStart) — so clicking Enable keyframes on a gsap.set element while seeking at 0 (set start 0) did nothing. It only knew how to promote a set into a FORWARD range (held@0% → live@100%). Now, when there's no forward range, replace the set with a single keyframe at the playhead holding its value (matching the no-animation branch), so a 0% keyframe is actually created.
…ne clips Two gaps for elements inside a sub-composition: 1) Clip keyframes rendered off-clip. The keyframe cache computes clip-relative percentages from the element's start/duration, but sub-comp internals aren't in the timeline elements list, so duration defaulted to 1s and percentages blew past 100%. Resolve the timing basis from the sub-comp HOST's bounds (via domClipChildren, since the host's data-composition-src is stripped in the rendered DOM). Shared resolveClipTimingBasis used by both cache populators, which now re-run when the sub-comp children appear. 2) Only GROUPED sub-comp children expanded. Generalize the DOM-children collector to gather id'd children of the sub-comp inner-root (grouped OR ungrouped), descending through id-less structural wrappers; one level into groups for drill-in. Ungrouped pills now expand into timeline rows too.
Dragging timeline keyframe diamonds was unreliable — clip<->tween percentage remapping, an optimistic-hold workaround, and an intermittent no-op/revert when the GSAP session lagged the drag (its own comments document the flakiness). Remove the drag interaction entirely: diamonds still display, click-to-seek, and offer the context menu (add/remove/ease) — keyframe timing is edited via the playhead + panel, which are deterministic. Deletes the keyframe-move plan module + its wiring through TimelineClipDiamonds -> TimelineCanvas -> Timeline -> TimelineEditContext.
Ungrouping removed the wrapper element but left its gsap.set("#group-1") behind,
targeting a now-deleted element. GSAP then threw "target not found" on every
preview run, which drove a selection re-render storm that made canvas context
menus (e.g. Delete All Keyframes) unclickable. unwrapElementsFromHtml now returns
the unwrapped wrapper's id, and the unwrap route strips any GSAP animation
targeting it (reusing the parser + removeAnimationFromScript).
- show the 3D Transform panel for any selected element, not only ones already animated; the first edit creates the gsap.set - scrolling depth on a flat element drops a gentle tilt so depth reads in place - clamp the depth scroll in front of the perspective lens (no runaway z) - register a no-op for the internal _auto keyframe marker so GSAP stops warning
…orms A z/perspective transform foreshortens an element by 1/m44; the drag offset and motion-path geometry ignored it, so the selection overlay and path drifted off depth elements. Fold m44 into the drag offset and the motion-path points, with the inverse applied where a pointer maps back to a stored offset.
- pickBestAnimation is group-aware: a rotation/3D edit no longer merges into a position tween — a fresh same-group tween with a 0% baseline is created instead - editing at a playhead past the tween extends it and keyframes there (matches drag) - update-keyframe MERGES into the existing keyframe instead of overwriting, so editing one property no longer drops z/transformPerspective (the lens then animated from 0 and the element popped) - dragging a keyframed element with a constant position tween keyframes rather than writing a static set
Ungroup only baked the wrapper's layout (left/top), not its GSAP transform — so a moved group's members snapped back to their creation-time positions. Distribute the group's static transform onto each member before stripping it: translation is an exact per-axis add; rotation/scale are composed about the group center so off-center members don't drift. Animated group transforms are left to be stripped, not baked.
Label the keyframe toolbar button 'Add keyframe (K)' (and the at-playhead / extend variants) so its action and shortcut are discoverable.
Fallow audit reportFound 72 findings. Dead code (1)
Duplication (37)
Health (34)
Generated by fallow. |
Collaborator
Author
|
Superseded by a Graphite stack of 8 smaller PRs (each <700 LOC), split along the thematic boundaries:
Same changes, rebased onto current main. Review bottom-up. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Element groups (P1)
A group is a real
<div data-hf-group="Group N">wrapping its children in the composition source. The DOM gives us containment, parent→child transform composition, bounds, z-order, and persistence for free; GSAP selectors survive wrapping; groups nest.Core (
packages/core)wrapElementsInHtml/unwrapElementsFromHtmlmirroringsplitElementInHtml: insert the wrapper at the first member's slot, move members in DOM order (= z-order), and rebase each member's inlineleft/topso its absolute position is unchanged. Members must share one parent (P1). Round-trip is identity.left/toplives in a CSS rule rather than inline — the rebase writes inline on wrap and reverses on unwrap. Covered by a round-trip test over plain, GSAP-animated, and--hf-studio-offsetelements.wrap-elements/unwrap-elementsfile-mutation routes mirroringpatch-element, with bbox/rebase coordinates validated as finite numbers (blocks style-string injection).Studio (
packages/studio)buildStableSelectoremits a[data-hf-group="…"]selector so a wrapper is selectable, patchable, and addressable; clicking a member selects the whole group.3D transform widget polish
Tangential to groups (came out of the same dogfooding pass) — easy to split into a separate PR if preferred:
P/(P−z)).Testing
All packages typecheck, lint, and pass the file-size + audit gates.