394 lines
16 KiB
Markdown
394 lines
16 KiB
Markdown
# Repeated and instanced UI nodes — design
|
||
|
||
> **Status: design only. Nothing here is implemented.**
|
||
> The audit below is traced from the code as it stands; the design that follows
|
||
> is a proposal to be reviewed and tested before any of it is built.
|
||
|
||
Every page migrated so far is *flat*: each node in the composition renders
|
||
exactly once, so a node id and a rendering are the same thing. Positions is the
|
||
first surface where that is not true. Six of its eight extension points render
|
||
**inside a record** — once per position — and the card those records are drawn
|
||
in is hand-written JSX that the node system cannot see at all.
|
||
|
||
This document says what is there now, proposes a model for it, and is honest
|
||
about what the model costs.
|
||
|
||
---
|
||
|
||
## 1. What is there now
|
||
|
||
### 1.1 The eight Positions placements, and which are instanced
|
||
|
||
`surfaces.js:47-76` declares eight placements under the `positions` surface and
|
||
`provides` already records the distinction that matters — which of them hand a
|
||
section a record:
|
||
|
||
| Placement | Rendered | Host | `provides` |
|
||
|---|---|---|---|
|
||
| `after-position-list-summary` | once per page | `Positions.jsx` (grid) | `[]` |
|
||
| `after-position-list` | once per page | `Positions.jsx` (grid) | `[]` |
|
||
| `after-position-card` | **once per record** | `PositionCard` | `positionId` |
|
||
| `after-header` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
|
||
| `after-position-summary` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
|
||
| `before-candidates` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
|
||
| `after-candidates` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
|
||
| `before-footer` | once per open record | `PositionDrawer` **and** `PositionDetail` | `positionId` |
|
||
|
||
Only the first two are in the node tree today (`pages/admin/positions/nodes.js`),
|
||
drawn through `UiNodeSlot`. The other six are still literal `<SkillSurface>`
|
||
elements.
|
||
|
||
### 1.2 The render loops
|
||
|
||
**The grid** — `Positions.jsx:1158`:
|
||
|
||
```jsx
|
||
{filtered.map((p) => (
|
||
<PositionCard key={p.id} position={p} onOpen={openPosition} isRecent={recentIds.has(p.id)} />
|
||
))}
|
||
```
|
||
|
||
`filtered` is page state: the search box, the role and location selects and the
|
||
sort, applied in `useMemo`. It is not a data source in the `surfaces.js` sense
|
||
and has no entry in `DATA_SOURCES` — it is the page's own working set.
|
||
|
||
**Inside each card** — `Positions.jsx:626-641`. The card's last element, wrapped
|
||
in a click-stopping `div` so a section inside a card does not open the drawer
|
||
behind it:
|
||
|
||
```jsx
|
||
<SkillSurface page="positions" placement="after-position-card"
|
||
context={{ position: p }} className="mt-3" />
|
||
```
|
||
|
||
**The drawer** — `PositionDrawer` (`Positions.jsx:719, 729, 793, 870, 897`) draws
|
||
five surfaces, each with `context={{ position: p }}`, for the one position
|
||
`openPosition` selected. `PositionDetail.jsx:288, 421, 472, 512, 513` draws the
|
||
same five placements for the position named in the route.
|
||
|
||
### 1.3 How a record reaches a section
|
||
|
||
Unchanged all the way down, and already correct for this design:
|
||
|
||
```
|
||
<SkillSurface context={{ position: p }} />
|
||
→ useSkillDataContext(context) SkillSurface.jsx:76 { ...published, ...context, ...collections }
|
||
→ resolveSkillData(section, ctx) dataResolver.js:1192
|
||
if (section.context === 'positionId' && !context.position) → unavailable
|
||
RESOLVERS['position.pipeline']({ position, applications }) → filters by position.id
|
||
```
|
||
|
||
An explicit `context` prop beats what the page published, which is exactly the
|
||
rule an instanced node needs: *a card knows which position it is.*
|
||
|
||
### 1.4 Section resolution
|
||
|
||
`useSkillSections(page, placement)` (`SkillSurface.jsx:46`) is independent of the
|
||
record. It reads the account's active skills and returns
|
||
`{ skill, section }[]` for the placement. **The same list is used for every
|
||
card** — repetition happens below it, in data resolution, not in which sections
|
||
exist. That is what makes one template node correct.
|
||
|
||
### 1.5 DOM identity today
|
||
|
||
None inside a card. `<SkillSurface>` renders `div.space-y-4 > section[aria-label]`
|
||
with no `data-` attributes, and `SkillSection`'s React key
|
||
(`` `${skill.id}:${section.id}` ``) is not emitted. `PositionCard` renders an
|
||
`<article>` with no id. There is nothing in the document that says *which*
|
||
position a rendering belongs to.
|
||
|
||
### 1.6 The card body
|
||
|
||
`PositionCard` (`Positions.jsx:519-643`) is roughly 120 lines of tuned JSX:
|
||
role glyph and client/title/category block, status pill, a terms `<dl>`, one
|
||
line of candidate criteria, a hairline rule, `Progression`, `WorkforceRow`, a
|
||
health chip with the insight line, and a footer pinned with `mt-auto` so a row
|
||
of cards keeps its footers aligned. Plus a two-minute "just saved" treatment.
|
||
|
||
**None of it is addressable.** "Make all position cards compact" has nothing to
|
||
act on today, in either the node tree or the component: no density prop exists.
|
||
|
||
---
|
||
|
||
## 2. The model
|
||
|
||
### 2.1 One node, many renderings
|
||
|
||
The rule the whole design rests on:
|
||
|
||
> A repeated surface is **one node in the tree**. The tree holds the template
|
||
> once; the DOM holds N renderings of it.
|
||
|
||
An operation targets the node, so one operation changes every card — which is
|
||
the "do not duplicate operations once per record" requirement met in the stored
|
||
bytes, not by a de-duplication pass.
|
||
|
||
### 2.2 Repetition is declared by the type, never by a patch
|
||
|
||
```js
|
||
registerNodeType({
|
||
type: 'position-card',
|
||
label: 'Position card',
|
||
component: PositionCardNode,
|
||
container: true,
|
||
accepts: ['skill-surface', ...],
|
||
repeats: {
|
||
from: 'positions', // a key in the bag the PAGE publishes to UiRenderContext
|
||
as: 'position', // the context key each rendering is given
|
||
key: 'id', // the record field that identifies a rendering
|
||
},
|
||
propSchema: { density: { enum: ['comfortable', 'compact'] } },
|
||
capabilities: ['update', 'move', 'hide', 'reorder'],
|
||
});
|
||
```
|
||
|
||
`repeats` lives on the **registration**, which is code, and never on the node —
|
||
so it is absent from `OP_FIELDS`, cannot be written by a stored patch, and gives
|
||
the validator nothing new to police on user data. A patch can change what a card
|
||
looks like; it can never change what a card iterates over.
|
||
|
||
`from` names a key in the page's own `UiRenderContext` bag
|
||
(`UiTreeRenderer.jsx:39`) rather than a `DATA_SOURCES` entry, because `filtered`
|
||
*is* page state — search, filters and sort applied. This adds no new data
|
||
pathway: the renderer reads `useUiContext()[entry.repeats.from]` and still never
|
||
looks inside the bag on its own account.
|
||
|
||
### 2.3 Node schema — unchanged
|
||
|
||
No new field on `UiNode`. A repeater is an ordinary container node whose *type*
|
||
happens to repeat:
|
||
|
||
```js
|
||
{
|
||
id: 'position-card',
|
||
type: 'position-card',
|
||
props: { density: 'comfortable' },
|
||
layout: { ... },
|
||
children: [ /* the template subtree */ ],
|
||
hidden: false, origin: 'builtin', locked: false,
|
||
}
|
||
```
|
||
|
||
The template's children are ordinary nodes with ordinary ids
|
||
(`position-card-terms`, `position-card-extensions`, …). Each is one node and N
|
||
renderings, by the same rule.
|
||
|
||
### 2.4 DOM identity
|
||
|
||
```html
|
||
<article data-ui-node="position-card" data-ui-instance="pos_123"> … </article>
|
||
<article data-ui-node="position-card" data-ui-instance="pos_456"> … </article>
|
||
```
|
||
|
||
`data-ui-node` keeps meaning **the node** — one value, N elements. `data-ui-instance`
|
||
carries the record key from `repeats.key`. Descendants of a rendering inherit the
|
||
instance from their ancestor rather than repeating it, so the pair
|
||
(`data-ui-node`, nearest ancestor `data-ui-instance`) addresses exactly one
|
||
rendering.
|
||
|
||
This **breaks the current invariant that `data-ui-node` is unique in a
|
||
document**, and everything that assumes it must be found and changed. See §5.1.
|
||
|
||
---
|
||
|
||
## 3. Targeting semantics
|
||
|
||
Three scopes. Only the first is proposed for implementation now.
|
||
|
||
### 3.1 Template scope — the default
|
||
|
||
```json
|
||
{ "op": "update", "target": "position-card", "props": { "density": "compact" } }
|
||
```
|
||
|
||
No new field. Applies to the node, therefore to every rendering. This is
|
||
"make all position cards compact", and it is one operation regardless of how
|
||
many positions exist.
|
||
|
||
Everything already true stays true: the op is validated against the type's
|
||
`propSchema` and `capabilities`, refused if `density` is not a declared enum
|
||
value, and stored in `uiLayouts` like any other.
|
||
|
||
### 3.2 Instance scope — designed, not built
|
||
|
||
An optional `scope` on the **operation**, not on the node:
|
||
|
||
```json
|
||
{ "op": "update", "target": "position-card",
|
||
"scope": { "key": "pos_123" },
|
||
"props": { "density": "comfortable" } }
|
||
```
|
||
|
||
The tree stays one template. `applyPatch` partitions:
|
||
|
||
- **unscoped ops** are applied to the tree as they are today;
|
||
- **scoped ops** are attached to their target node, grouped by key, and applied
|
||
at render time to that one rendering — by `applyOperations`, the same engine,
|
||
with the same validator.
|
||
|
||
There is no second mutation engine and no per-record tree in storage.
|
||
|
||
**Legality is declared per operation, not per page.** Each entry in `OPERATIONS`
|
||
gains `instanceable: true|false`:
|
||
|
||
| Operation | Instanceable | Why |
|
||
|---|---|---|
|
||
| `update` | yes | changes one rendering |
|
||
| `hide` | yes | changes one rendering |
|
||
| `move`, `reorder` | no (first cut) | one card structurally unlike its neighbours |
|
||
| `add`, `remove`, `replace` | no (first cut) | "remove this record's card" is a filter, not a layout change |
|
||
|
||
A `scope` on a node whose type does not declare `repeats` is refused — a registry
|
||
read, not a page branch.
|
||
|
||
### 3.3 Predicate scope — reserved, not designed
|
||
|
||
`scope: { where: [...] }` — "make all *draft* position cards compact". Named here
|
||
only so `scope` is an object from the first day rather than a bare key string
|
||
that would have to be widened later.
|
||
|
||
### 3.4 Natural language
|
||
|
||
Template scope needs nothing new: `resolveTarget` already scores "the position
|
||
card" against node titles, labels and ids, and there is exactly one such node.
|
||
|
||
Instance scope needs a record resolver — "this position's card" is only
|
||
answerable when a position is selected, which `PageContext` publishes for the
|
||
drawer but not for the grid. **Deferred.** Until then Owliver refuses instance
|
||
phrasing explicitly rather than silently widening it to every card, which is the
|
||
failure worth guarding hardest: a person who says "only this one" must never get
|
||
"all of them".
|
||
|
||
---
|
||
|
||
## 4. Persistence
|
||
|
||
### 4.1 Shape
|
||
|
||
No new store, no new key, no new tier. `preferences.uiLayouts[page].ops` gains an
|
||
optional `scope` per op:
|
||
|
||
```json
|
||
{
|
||
"schema": 2,
|
||
"page": "positions",
|
||
"updatedAt": "…",
|
||
"ops": [
|
||
{ "op": "update", "target": "position-card", "props": { "density": "compact" } },
|
||
{ "op": "hide", "target": "position-card-pay", "hidden": true },
|
||
{ "op": "update", "target": "position-card",
|
||
"scope": { "key": "pos_123" }, "props": { "density": "comfortable" } }
|
||
]
|
||
}
|
||
```
|
||
|
||
Size is **O(operations), not O(records)**. One op makes every card compact.
|
||
|
||
### 4.2 The schema bump is not optional
|
||
|
||
`OP_FIELDS` (`patch.js:51`) whitelists the fields each op may carry and
|
||
`normalizeOp` **silently drops anything else**. A build that predates `scope`
|
||
would therefore read the third op above as an unscoped one and make *every* card
|
||
comfortable — the exact "only this one became all of them" failure.
|
||
|
||
So:
|
||
|
||
- a patch containing **no** scoped op keeps `schema: 1` and every existing build
|
||
reads it exactly as it does today;
|
||
- the first scoped op written raises that page's patch to `schema: 2`;
|
||
- a reader that does not understand a schema **skips the whole patch** and
|
||
reports it through the existing `skipped` channel, rather than applying part
|
||
of it.
|
||
|
||
### 4.3 Stale keys
|
||
|
||
A scoped op naming a record that no longer exists is reported as `skipped`, like
|
||
any stale target, and **retained** rather than dropped. Retained deliberately:
|
||
the record may simply be filtered out of `filtered` by the page's own search or
|
||
role filter at that moment, and treating the page's filter state as deletion
|
||
would quietly destroy a person's saved overrides every time they typed in a
|
||
search box. Clearing them is an explicit action in the editor.
|
||
|
||
---
|
||
|
||
## 5. Risks
|
||
|
||
**5.1 `data-ui-node` stops being unique.** Any `querySelector` on it silently
|
||
takes the first rendering — in tests, in the editor, and in anything built later.
|
||
*Mitigation:* replace the uniqueness assertion with the real invariant — ids are
|
||
unique unless the node's type declares `repeats`, and (`data-ui-node`, ancestor
|
||
`data-ui-instance`) is unique always. This is a test to write **before** the
|
||
first repeater exists.
|
||
|
||
**5.2 Render cost.** Instance-scoped ops run the operation engine per rendering.
|
||
Negligible for a handful of overrides over tens of cards; not negligible for
|
||
hundreds of records. *Mitigation:* only records that actually carry a scoped op
|
||
do any work, memoized on (template, key, ops hash).
|
||
|
||
**5.3 Migrating `PositionCard` is a real rewrite, and the riskiest one so far.**
|
||
`mt-auto` footer alignment, truncation, the hover and focus rings, and the
|
||
"just saved" animation are all tuned. Phase 5A already produced one margin
|
||
regression on a far simpler surface. *Mitigation:* the same baseline capture
|
||
used for every other migration, run over the grid with a fixed record set, plus
|
||
the class-signature and word-signature checks.
|
||
|
||
**5.4 "Compact" does not exist yet.** No component honours a density prop. The
|
||
engine cannot invent one: the schema is what makes a change *expressible*, not
|
||
what makes it *possible*. Someone must implement two densities in `PositionCard`
|
||
before "make all position cards compact" can do anything, and until then the
|
||
honest answer to that request is that the card offers no such setting.
|
||
|
||
**5.5 The drawer and the detail page draw the same five placements.** They are
|
||
never on screen together — the drawer overlays `/admin/positions`, the detail
|
||
page is `/admin/positions/:id` — but they are different layouts around identical
|
||
extension points. If both render one composition, a change made in the drawer
|
||
also changes the detail page, which may surprise. *Proposal:* one composition
|
||
(`position-detail`) drawn by both hosts, editable from the detail route only in
|
||
the first cut, because an editing session currently holds a single composition
|
||
keyed to the route. Making a session span two compositions is the extension, and
|
||
`uiLayouts` already stores per page so it needs no storage change.
|
||
|
||
**5.6 Per-record overrides invite chaos.** Twenty individually tweaked cards is
|
||
unreviewable. *Mitigation:* keep instance scope to `update` and `hide`, show the
|
||
count of overrides on the node in the editor, and offer to clear them all.
|
||
|
||
**5.7 `repeats.from` is a page-state key, so a rename fails at run time.** The
|
||
page renames its context key, the repeater finds nothing, the grid renders empty
|
||
— with no build error. *Mitigation:* a check-script assertion that every
|
||
registered `repeats.from` is a key the owning page publishes, in the same shape
|
||
as the surface-route guard.
|
||
|
||
**5.8 Skill sections inside a repeater lose id uniqueness too.** A section under
|
||
`after-position-card` is one node and N renderings, each resolving against a
|
||
different `position`. Consistent with the model, and `SkillSurface`'s contract is
|
||
unchanged — `as: 'position'` feeds exactly the `context={{ position }}` prop it
|
||
already takes — but it is the same uniqueness caveat as §5.1 reaching the skill
|
||
layer.
|
||
|
||
**5.9 The MD schema is untouched.** No frontmatter key, no `normalizeSection`
|
||
change, therefore no Go parser change and no oracle regeneration. Confirmed by
|
||
construction: nothing in this design is authored in Markdown.
|
||
|
||
---
|
||
|
||
## 6. What must be tested before any of it is built
|
||
|
||
Against the *existing* Positions architecture, so the semantics are proven
|
||
before the card is touched:
|
||
|
||
1. One `update` on a repeater node changes every rendering — asserted on
|
||
rendering count, not on one element.
|
||
2. That change is **one** operation in `uiLayouts`, with a record count > 1.
|
||
3. A scoped `update` changes exactly one rendering and leaves the others.
|
||
4. A scoped op on a non-repeating node is refused.
|
||
5. A `move`/`remove`/`add`/`replace` carrying a `scope` is refused.
|
||
6. A schema-2 patch read by a schema-1 reader is skipped whole, never widened.
|
||
7. A scoped op naming an absent record is reported skipped and **retained**.
|
||
8. Filtering the grid does not drop overrides for the filtered-out records.
|
||
9. `data-ui-node` × ancestor `data-ui-instance` is unique; ids repeat only for
|
||
types declaring `repeats`.
|
||
10. Owliver refuses instance phrasing rather than widening it to the template.
|
||
11. A skill section under `after-position-card` resolves against its own card's
|
||
position, in every rendering.
|
||
12. The Positions grid renders identically to its captured baseline.
|