Repository navigation
UI vendor movebefore - #22
Merged
Merged
Conversation
…e moveBefore, and focus survives a relocation
Upstream relocates keyed rows with `Element.moveBefore` where it exists. That is a
state-preserving atomic move; `insertBefore`, which the DOM defines as remove-plus-insert,
is not. So the FAIL this repo's harness was deliberately built to produce is now a PASS,
and the focus loss we concluded was ours to fix is not ours to fix.
Measured in Chrome 149 — the version matters and was checked FIRST, because `moveBefore`
needs Chrome/Edge 133+ or Firefox 144+ and a FAIL on an older build would have been my
browser, not their code:
order flip focus stayed on button.chip.chip--re in §46 (was: LOST)
filter focus stayed on button.chip.chip--task in §32 rows 20 -> 3
arriving post focus stayed on button.sec__expand in §49
The filter is the path that surprised us and the one the harness could not drive. It is
driven now, without a hand measurement: focus the chip, take the baseline, then activate it
with Enter — a keyboard activation fires the handler without moving focus, which a click
cannot do.
The four numbers that already passed did not move, across flip, filter, load-older, toggle
and an arriving post: rows kept = rows carried, rows rebuilt 0, lazy rendered to pending 0,
bodies re-parsed 0. Held-section shift still 0 px on load-older.
One result worth reading rather than counting. Across the arriving post, §49 — expanded by
hand, so an override — held, while §50 collapsed. §50 had been expanded only because it was
the NEWEST section, and §51 arriving made it ordinary. That is the derived-clamp rule
working, and it is the same test design built by hand in review: a pass that cannot be
"the state happened to match the default", because here the default moved and the override
did not.
Landed on `438b8c8`, not the `b9c633e` that was asked for: `make vendor-ui` clones a BRANCH,
so it takes whatever `main` is at, and upstream moved between the request and the run.
VERSION records what actually arrived.
What came along, since a bump that silently carries unrelated components is one nobody can
review: `reactive.js` (the engine, and CODE this time — 14 changed lines, all in
`Row.moveBefore`), `md.js`, `lazy.js`, and fixes to `combobox`, `popover`, `popconfirm`,
`tooltip`, `upload` — components this app does not use. Also two new files, `_agents.md`
and `_components.jsonl`: the library's own documentation, which the underscore convention
and the `all:`-less embed keep out of the binary.
And a defect this repo reported upstream is fixed in the same bump: `descriptions.js` no
longer carries a literal NUL byte — it is written as the six-character escape now, 5815 to
5820 bytes, and git reads the file as text instead of binary.
`make check` green, 24 JS tests.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…ng that reaches this app
Taken because upstream moves often enough that the alternative is a bigger gap later, not
because anything here needed it. Worth saying plainly, since a vendor bump whose commit
message implies a fix is one someone will later go looking for.
What actually changed, verified by stripping comments from BOTH sides and diffing the whole
file rather than comparing line counts — equal counts survive a swap or a
removal-plus-addition:
reactive.js code IDENTICAL (275 lines each) — comments only
md.js code IDENTICAL (202 lines each) — untouched
upload.js code changed — a component this app never loads
_agents.md the library's own docs
So the engine this transcript depends on is byte-for-byte the same logic, and the one
measurement that could have moved did not:
PASS focus stayed on button.chip.chip--re in §46
PASS rows kept 20/20, rebuilt 0
PASS lazy rendered to pending: 0
PASS bodies re-parsed: 0 (a flip no longer costs a re-parse)
`make check` green, 24 JS tests.
One thing checked because the upstream branch is called `markdown-move-corruption`, which
lands on exactly what this app does — moved rows containing rendered markdown. The commit
already vendored here (438b8c8) documents the limitation: CLONING a rendered
`<puredashboard-markdown>` corrupts it, because `cloneNode()` copies the DOM and not the
instance, so `<h1>Heading</h1><p>Paragraph.</p>` clones to `<p>HeadingParagraph.</p>`.
It cannot reach us, and that was checked rather than assumed: nothing under `pages/`,
`components/`, `lib/` or the harness calls `cloneNode` or declares a `<template>`. The
markdown element is bound into the row as a NODE (`${lazyFor(sec, clamped)}`), so it lives
outside the part of the template the engine clones — it is built fresh and given `.value`,
which is exactly what upstream's own note says to do instead of cloning.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
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.
No description provided.