Feature: Sidebar fragments - #117
Merged
Merged
Conversation
fade moves opacity, slide and scale move transform. Between them they cover
every animation that does not touch layout, which left the most ordinary one
in any interface — something opening in place — with no expression at all.
The design system rules geometry out of its automatic surface deliberately
(ANIMATABLE_STATE_PROPS, narrowed from `all` after instrumentation caught
border-widths animating on hover), so the only way to write a disclosure was
a raw CSS string in the `styles` escape hatch.
reveal fills that in. The wrapper becomes a single-track grid going 0fr -> 1fr
and the inner div clips, which eases to the content's real auto size — the
thing a plain height transition cannot do. Both renderers already had the
two-element structure the technique needs; it just had to mean something.
Not a max-height guess: that applies the easing curve to the guess rather than
to the height, so most of the duration crosses space that is not there, and it
fails silently once content outgrows the number. Not `interpolate-size` either,
which will be the right answer once it is cross-browser baseline.
$if reveals and then unmounts; $animate reveals and keeps its child, for a
section holding state that must survive being closed. axis: 'inline' opens
sideways, for a label appearing beside an icon.
Also fixes an exit-timing bug found on the way: the unmount timer read its
duration off the *first* effect, so [{fade,200},{slide,700}] tore the node out
at 200ms and cut the slide at under a third. It now waits for the longest.
The schema renderer wraps every node in a `display: contents` div, so `assignedElements()` handed this primitive the wrappers rather than the nodes an author wrote. Those carry no `data-we-id` and, having no box, return a zero rect — so every id came out empty and every drop index was computed against 0x0 rectangles stacked at the origin. A drag-to-reorder written in a template would have looked implemented and done nothing. Resolving each slotted child to its identified descendant fixes both halves at once, because `display: contents` promotes its children to be the container's real flex items: the element carrying the id is also the element carrying the geometry. A child that identifies itself is returned untouched, so the TSX callers that put the attribute on the slotted element are unaffected. Found while planning the sidebar rebuild, but not specific to it — as it stood, no template could use a primitive we ship.
$localState field names are fixed when the template is written and $local
walks a static path, so "is *this* group collapsed?" had no spelling once the
groups came from a $query or a $store. A template could only pre-declare a flag
per group it already knew about — exactly the set a data-driven list does not
have. Anything wanting per-row expansion had to give up and expand everything.
Inverting it fixes the mismatch: the field holds the ids that are on, so the
varying part moves into the value where an expression can reach it. The read
side needed nothing new — { $in: ['$group.id', { $local: 'collapsed' }] }
already worked, which is a decent sign this is the missing half rather than a
new mechanism.
Adds an 'array' local-state type to declare the field, admitted to the Zod
initial union as a bare array (structurally distinct from a token object, so a
malformed token still cannot slip through as a plain-object initial).
The validator now tracks declared field types, because both ways of getting
this wrong are otherwise silent: a missing `value` toggles undefined in and out
of the array forever, and pointing it at a boolean replaces that boolean with
an array on first click, so every read of it downstream starts answering a
different question.
Grid's `template` maps to grid-template-columns, so the row axis had no prop at all and every use of it reached for the `styles` escape hatch. Nothing exotic was going on at those call sites — the inspector's box-model diagram wants a fixed label band top and bottom, which is ordinary layout, and ordinary layout is what DS props are for. Migrates the inspector's two. The third pre-existing use of grid-template-rows, in the call module's stage style, is not a Grid usage — it builds a whole style record including grid-auto-flow and grid-auto-columns, neither of which this covers — so it stays as it is.
It was the last DS prop that bypassed the token system — a raw passthrough, so `transition: '300'` emitted an unitless value the browser drops, silently and indistinguishably from a typo. Same bug the offset props had, same fix: discriminate by shape, so a token becomes a var and '300ms', '0.2s' and 'ease-in-out' pass through untouched. Consistency is the smaller half of it. --we-transition-* is what a theme's animationSpeed preset overrides — 'instant' sets every one to 0ms — so a duration written as a token honours a reduced-motion choice, and one written as '300ms' quietly ignores it. Until now the prop offered no way to write the former. Applied in both places a transition is emitted: buildLayoutStyles for the Solid layout components, and the primitives' instance var, so we-* elements resolve it the same way.
Inside a Row or Column the animation wrapper is the flex item, so `flex: '1'` declared on the then-node was being read by an element the parent is not sizing. It did nothing, silently — the node just failed to grow, which reads as a styling opinion rather than a dropped instruction. The wrapper already mirrors width/height for exactly this reason (it is meant to be invisible to layout but is a real box); flex belongs in the same list.
CollapsibleSidebar was a layer-5 widget whose entire job was arrangement, reached through an `items` array — data in, but not structure. That is the distinction the project turns on: a prop is a customisation somebody predicted and shipped, a node tree is every customisation including the ones nobody thought of. Asked to put a second line under a space's name, a progress bar in a group header, or a section that is neither an item nor a group, the widget's answer was "wait for a release", and the follow-up had no answer at all. The whole rail was also one opaque node, so the visual editor stopped at it. Nothing in it needed code. Hover is two handlers, the width is a transition, the group collapse is a reveal, and the reorder is we-sortable — a primitive precisely because pointer capture and drag geometry are code. Three fragments rather than one, per the kit's rule that an over-parameterised fragment is worse than the duplication it replaced: railShell owns the box and the two ambient fields, railGroup a heading and its set membership, railItem a row. Both readers of `expanded` read it up the tree, as cardShell does with displayMode. USE_FRAGMENT_RAIL in Sidebar.schema.ts switches between the two so they can be compared in place. The widget is untouched and still registered; it goes once the fragments have been lived with. Two deliberate differences from the widget, both noted at the node that causes them. Items are top-aligned rather than vertically centred — centring a scrollable flex column clips the top of it once the space list is long enough to scroll, which is a bug the widget has. And a group's heading leaves rather than fading in place, which stops each group reserving a strip of nothing above it at collapsed width. The rail also now remembers whether it was left open, which the widget never did. A preference, so it is per device and never travels in a shared link.
The kit's rule: a fragment and its recipe are two renderings of one decision, and a drifted recipe teaches the AI a shape the codebase has stopped using. A template authored in the browser cannot import the kit, so the JSON is what it actually needs. Carries the two traps with it — the label is mounted rather than narrowed to zero (a hidden-but-present label is still in the accessibility tree and still found by find-in-page), and data-we-id belongs on a native element, because a web component's props are assigned as DOM properties and the attribute we-sortable looks for would never exist.
Adds the three rail fragments and their ambient contract to the kit listing and the scope table — the table is the only place a reader can see that railItem outside a railShell fails silently, which is the one way to get these wrong. The listing had also fallen behind on its own account: seven fragments and the input tier were missing from it.
The custom-elements manifest is rebuilt as part of the primitives build, so the sortable comment only reached the generated reference on a full build rather than on generate-context alone.
✅ Deploy Preview for coasys-we ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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
1 participant
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.
Collapsible sidebar as template fragments
Status: implemented on
feat/sidebar-fragments. Ten commits; the widget is still in placebehind a switch, so both versions can be compared in the running app.
Summary
The shell's left rail is a layer-5 widget —
CollapsibleSidebar(~380 lines TSX + ~180 SCSS) — whoseentire job is arrangement.
packages/templates/shell/src/Sidebar.schema.tshands it anitemsarrayand a
slots.header, and everything about how those become UI is compiled code. That contradicts therule the architecture is built on:
The consequence is not theoretical. The sidebar is one opaque node: the visual editor stops at it, AI
can only edit props somebody predicted, and a user cannot make an item two lines, put a
SignalControlin a group header, move a badge, or add a section that is neither an item nor a group.
ModuleRail.schema.tsand the default template's own
SpaceSidebar.tsare both pure schema and prove the shape works.This branch rebuilds the rail as three fragments in
@we/template-kit, and closes the five substrategaps that stand in the way. Four of those five are independently useful and none of them are large.
Scope note. The rail is shell chrome, registered as
core:sidebaron thedock-leftanchor —not part of the default template. The default template's sidebar is already plain nodes and is not
touched.
What was verified before planning
Each behaviour the widget owns, checked against what schema can actually express today:
Column+position/zIndex/width/transition$localState { expanded }+onMouseEnter/onMouseLeave→$setLocalmax-width: 0 → 500px+ opacity)$ifon$localwe-button+we-icon/we-avatar/we-badge,hoverPropsModuleRailalready does$localState … persistgrid-template-rows: 0fr → 1fr)props.stylesraw-CSS escape hatchwe-sortablepointermovefallback collapseSupporting facts confirmed in source rather than assumed:
createLayoutComponentspreads unrecognised props onto itsdiv, soonMouseEnter/onMouseLeavereach the DOM on
Column/Row/Grid/Card.semanticValidation.ts:668waves anyonXxxprop through, so the validator will not reject them.isStaticValuerecurses into object values andpropMemoswrapsdeepUnwrap(resolveProp(...))in amemo, so a token nested inside an object prop (
styles: { … : { $if: … } }) does resolve reactively.ConditionalRendererrenders throughRenderSchema, so$localStatedeclared on an$ifthenbranch works.
RenderSchemainTemplateLayout.tsx, so$localStateworks in chrome.Changes
1.
revealtransition effect —@we/schema-shared+@we/schema-solidWhy. The template system's animation vocabulary has exactly one axis.
transitionUtils.tsmapsfade→opacity,slide/scale→transform,pulse→ a keyframe. There is no size axis, and nonotion of a property easing when its value changes. The DS layer reinforces this deliberately:
ANIMATABLE_STATE_PROPSin3-primitives/src/shared/helpers.tswas narrowed fromallafterinstrumentation caught border-widths animating on hover and causing flicker — "nothing that can move a
box" — and the transition sits on
[part=base]rather than:hostbecause the host owns geometry.Geometry animation was therefore ruled out of the automatic surface on purpose, leaving the raw
transitionDS prop as the stated per-instance escape hatch. That prop is used zero times acrosspackages/templates, which is why the gap has not surfaced before.So the single most common in-place animation there is — reveal something to its natural height — has no
expression, and the widget spells it as raw CSS. Blessing the escape hatch in a fragment would push that
incantation onto every author who copies the expansion.
What. Add
revealtoTransitionEffect, alongsidefade/slide/scale/pulse:Implemented as the
0fr → 1frgrid technique, which is the correct mechanism — it eases to the content'sactual
autosize with no measurement. Themax-heightguessCollapsedContentuses (5000px) iswrong for a reason worth recording: the easing is a lie, most of the duration is spent traversing empty
space, and it breaks silently past the guess.
interpolate-size/calc-size(auto)is the eventualbrowser answer but is not cross-browser baseline yet.
ConditionalRendereralready renders exactly the two-element structure the technique needs — an outerwrapper carrying the animated style, and an inner
divwrapping the content. The outer takesdisplay: grid+grid-template-rows, the inner takesoverflow: hidden; min-height: 0. No newprimitive, no new component, no author-facing CSS.
axis: 'inline'usesgrid-template-columns, whichis what replaces the label reveal's magic
max-width: 500px.Composes with the existing effects the same way
slidedoes:Mirror the same effect into
AnimateRendererfor the stay-mounted case.Decide explicitly during implementation:
$ifunmounts content after the exit duration, so acollapsed group leaves the DOM — a behaviour change from the widget, which keeps collapsed groups
mounted. Better for memory, worse for anything inside holding state.
$animatecovers the other case.Implementation wrinkles to handle, not discover:
wrapperStyleinConditionalRenderercopiesSIZE_PROPS(width/height/max-*) from the contentonto the wrapper. A copied
heightwill fight a grid wrapper — reconcile the two.pointer-events: noneand the innerauto; confirm that still holds mid-reveal.durationoff the first effect in the array. Withrevealfirst this is correct;worth a comment so the ordering is not treated as cosmetic.
Rejected alternative: a
we-disclosureprimitive. It needs no measurement, so by the project's ownrule it does not clear the bar for code, and it would be less composable than an effect that sits in the
vocabulary authors already know.
2.
we-sortableschema compatibility —@we/primitivesWhy. This is the one hard blocker. Every schema node is wrapped in a
display: contentsdiv(
SchemaRenderer.tsx:1090).we-sortablefinds its items withslot.assignedElements()and readsdata-we-idoff them (sortable.ts:69-73). From schema-land it would therefore see N wrappers thatcarry no id and — being
display: contents— have zero bounding rects, breaking both identity anddrop-index maths. There is no way to set an attribute on the wrapper from a schema; only node-level
stylesreaches it, and that takes a style object, not attributes.$eachis fine here — it returns a bare<For>with no wrapper of its own, so the sortable does see Nelements rather than one.
What. In the primitive, when an assigned element carries no
data-we-id, resolve the id from itsfirst matching element descendant, and take the rect from that descendant too. Roughly 15 lines in
_getItemId/_getItems.Worth doing on its own merits: as it stands, a primitive we ship cannot be used by any template.
3. Dynamic
$localStatekeying —@we/schema-sharedWhy.
$localStatefields are named, and$localresolves a static dot path. Per-group collapsestate therefore has to be one declared boolean per group. Today's sidebar has literal groups (Spaces,
Apps) so parity is not blocked — but the whole point of the exercise is that a user can generate groups
from data, and a
$eachover categories cannot give each one its own state. The same limit bites anylist wanting per-row expansion.
What. A set-membership spelling, because the read side already exists:
One new action token.
valueresolves through the prop pipeline, so context refs,$storeand literalsall work. Preferred over a computed-key
$setLocal(which would break the token'sstringtype and thevalidation that depends on it) and over merge-with-computed-key (merge keys are literal by design).
Check
OPERATORS.mdbefore settling the name — the repo's rule is that a new operator needs theworkaround to be genuinely absent, which it is here.
4.
Grid.rowsprop —@we/componentsWhy. Independent of everything above.
Grid.templatemaps only togrid-template-columns(
Grid.solid.tsxfinalizeStyle), so grid rows are unreachable from DS props entirely. Three existingcall sites already reach for the
stylesescape hatch to get them —InspectorPanel.tsx:1664,InspectorPanel.tsx:1797,module-system/call/src/store.ts:596— none of which is animating anything.What. A
rowsprop mirroringtemplate, folded intofinalizeStyle. Migrate the three call sites.5. Tokenise the
transitionDS prop —@we/design-utilsWhy.
buildLayoutStylesline 671 is a barestyle.transition = props.transition— the only DS propthat bypasses the token system. Meanwhile
animation.transitiontokens exist ('300' → 250ms) and areemitted as
--we-transition-*(we-menu-groupalready readsvar(--we-transition-300)). Sotransition: '300'silently emits invalid CSS that the browser drops, indistinguishable from a typo atauthor time. This is exactly the bug class already fixed for the
top/right/bottom/leftoffsets,and the fix is documented in the comment immediately above.
What. Resolve duration tokens by shape the way
tokenVaralready does for offsets, so'300'becomes the token and
'300ms ease-in-out','width 300ms ease'etc. pass through untouched.6. The fragments —
@we/template-kitThree fragments, not one. The kit's conventions are explicit that an over-parameterised fragment is
worse than the duplication it replaces, because it also hides it.
railShell({ side, width, collapsedWidth, hoverExpand, persistKey, header, footer, children })$localState { expanded }, the width transition, fixed positioning$local: 'expanded'railGroup({ id, label, action, reorderable, onReorder, children })revealcollapseexpanded; owns its collapse entryrailItem({ icon | avatar, label, badge, active, onClick })expandedReading
expandedup the tree rather than threading it through every layer mirrors the documenteddisplayModecontract inlists/cards.ts. Per the kit's rules, every read up the tree and every writeinto it is declared in each fragment's doc comment, and each carries a comment saying why it exists.
These sit outside
we/— they name no store. Store references stay at the call site.Estimate: ~200–250 lines of TS replacing ~380 TSX + ~180 SCSS.
7. Cut over
Sidebar.schema.ts, then retire the widgetpackages/templates/shell/src/Sidebar.schema.tsgrows from ~190 to roughly ~260 lines and keeps itscurrent structure — profile / settings / marketplace, the reorderable Spaces group with its create
action, the Apps group, the footer, the WE-logo header.
Once it has no call sites, delete
CollapsibleSidebarfrom5-widgetsand drop it fromcomponentRegistry.tsx,slotRegistry.tsand the indexer's generated context data. Regenerate withpnpm --filter @we/ai-context generate-context.Do this last and do not skimp on it.
docs/architecture/template-fragments.mdis blunt about why:most instances will be untouched copies of the first expansion forever, and a bug in it cannot be fixed
centrally for templates users have already saved. The seed matters more than a default would.
How to compare the two
USE_FRAGMENT_RAILat the top ofpackages/templates/shell/src/Sidebar.schema.ts.trueis thefragments,
falseis the widget; both definitions live in that file and the widget is otherwiseuntouched and still registered.
Worth exercising specifically: hover open and close; the Spaces group collapsing and reopening;
dragging a space to reorder it and reloading to confirm it stuck; the rail remembering it was left
open across a reload; and the visual editor being able to select a node inside the rail, which is
the thing the widget could never allow.
Two changes not in the original plan
Both found while building on top of the reveal, both pre-existing and both silent:
$ifexit timing read the first effect's duration, so[{fade,200},{slide,700}]unmounted thenode at 200ms and cut the slide at under a third of its length. It now waits for the longest effect.
$ifwith a transition swallowed the content'sflex. Inside a Row or Column the animationwrapper is the flex item, so
flex: '1'on the then-node was read by an element the parent is notsizing and did nothing at all. The wrapper already mirrors width/height for exactly this reason.
What was descoped
The plan said the
Grid.rowschange would migrate three call sites. Two migrated (the inspector'sbox-model diagrams). The third — the call module's
stageStyle— is not aGridusage at all: itbuilds a whole style record including
grid-auto-flowandgrid-auto-columns, neither of which arowsprop covers. It stays as it was.The
we-hover-regionprimitive is still deferred, so the widget'spointermovefallback for adropped
mouseleavehas no equivalent. If the rail is seen sticking open after a template switch,that is this; it self-corrects on the next hover.
Known follow-ups
pointermovefallback(
CollapsibleSidebar.solid.tsx:63-76) for browsers droppingmouseleaveduring heavy DOM work — areal bug from template switching. Schema gets bare
onMouseEnter/onMouseLeave, so the rail can stickopen. The symptom is cosmetic and self-corrects on the next hover. The clean fix is a small
we-hover-regionprimitive emitting ahoverchangeevent that a schema wires to$setLocal— note aprimitive can only publish state outward via events, never into
$localdirectly. Held until thesticking is observed rather than predicted.
<Index>→<For>: the widget usesIndexwith per-field memos,$eachusesFor. At sidebarscale (tens of items) this is not expected to matter; worth a glance if the spaces list gets long.
CollapsedContent'smax-height: 5000pxclamp could move to therevealmechanism once it exists.Out of scope here.
Test plan
Verified:
pnpm --filter @we/schema-shared validate— 27 schemas, no issuespnpm -r run test— every package passing (kit 57, schema-shared 563, schema-solid 67,design-utils 48)
pnpm build— full monorepo including all three app targetsnpx eslint . --max-warnings 0— cleantsc --noEmiton every touched package. Three pre-existing errors remain, all confirmedpresent on the branch point: two
BackendConfig.uuidin schema-solid tests, one LeafletMap | nullin the location-picker, onePropsimport path in a schema-shared test.reveal— CSS output per axis, composition withfade, no implied opacity, first-winswhen several are given, and
transitionSpantaking the longest rather than the first$toggleLocalIn— add, remove,valuethrough the prop pipeline (a$eachcontextref), empty field, refusal on a non-array field, no-op on an undeclared one
transitiontoken resolution — bare token, duration slot, delay slot, comma-separatedhalves independently, real CSS durations untouched
groups, holds collapsed groups as a set, and carries
data-we-idon a native elementwe-sortable's wrapper resolution is covered by the last of those (the id must land on adiv), but the drag geometry itself is not —3-primitiveshas no test harness, and addingone for this was out of scope
Not verified, and needing a person in front of the running app:
chance of still being wrong, since it depends on runtime DOM geometry no test here exercises
Grid.rowscall sites look unchanged