Keep a row's actions from wandering when its line gets an id
A line this client sends has no msgid until the server echoes it back, and the row swapped a :spacer for the ↩ and ☺ buttons in place when it arrived — one node changing tag under one key. Reconciled by position, that rebuilt the row in the wrong order: the actions ended up after the text, the row was wider than the column, and a row laid out from its right edge put everything else off the left of the screen. The same held for the surface a jump leaves behind, which swapped :vbox for :card under the body's key. So a key each, and the change is a removal and an addition — which is what it is. And glimmer from our fork until the fix in it lands upstream: a component unmounted between a cell firing and its queued render running rendered anyway, into widgets the backend had already freed and handed out again, which was the rest of what took the message list apart in a channel that was moving. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5b5e90e parent: 8a491ba modified
deps.edn +6 -3 | @@ -2,9 +2,12 @@ | ||
| 2 | 2 | |
| 3 | 3 | ;; glimmer owns the reactive half (ratom, components, reconciler); |
| 4 | 4 | ;; glimmer-vidya paints it as Vidya/egui through libvidya's retained-tree ABI. |
| 5 | - :deps {jolt-lang/glimmer {:git/url "https://github.com/jolt-lang/glimmer" | |
| 6 | - :git/tag "v0.1.0" | |
| 7 | - :git/sha "5581c331c51aff989259b9e8e92ec920fe5e6741"} | |
| 5 | + ;; Our fork of glimmer, until the reconciler fix in it lands upstream: a | |
| 6 | + ;; component unmounted between a cell firing and the queued render running | |
| 7 | + ;; used to render anyway, into widgets the backend had already freed and | |
| 8 | + ;; handed out again — which took the message list apart in a busy channel. | |
| 9 | + :deps {jolt-lang/glimmer {:git/url "https://gitlab.com/nandithebull/glimmer" | |
| 10 | + :git/sha "4e733aa26343cef0478d8dfd3f2b76c0113b0969"} | |
| 8 | 11 | nandi/glimmer-vidya {:local/root "../vidya/glimmer"}} |
| 9 | 12 | |
| 10 | 13 | ;; libvidya, the Rust/egui build (the C/raylib one does not export the tree |
| @@ -2,9 +2,12 @@ | |||
| 2 | 2 | ||
| 3 | ;; glimmer owns the reactive half (ratom, components, reconciler); | 3 | ;; glimmer owns the reactive half (ratom, components, reconciler); |
| 4 | ;; glimmer-vidya paints it as Vidya/egui through libvidya's retained-tree ABI. | 4 | ;; glimmer-vidya paints it as Vidya/egui through libvidya's retained-tree ABI. |
| 5 | - :deps {jolt-lang/glimmer {:git/url "https://github.com/jolt-lang/glimmer" | 5 | + ;; Our fork of glimmer, until the reconciler fix in it lands upstream: a |
| 6 | - :git/tag "v0.1.0" | 6 | + ;; component unmounted between a cell firing and the queued render running |
| 7 | - :git/sha "5581c331c51aff989259b9e8e92ec920fe5e6741"} | 7 | + ;; used to render anyway, into widgets the backend had already freed and |
| 8 | + ;; handed out again — which took the message list apart in a busy channel. | ||
| 9 | + :deps {jolt-lang/glimmer {:git/url "https://gitlab.com/nandithebull/glimmer" | ||
| 10 | + :git/sha "4e733aa26343cef0478d8dfd3f2b76c0113b0969"} | ||
| 8 | nandi/glimmer-vidya {:local/root "../vidya/glimmer"}} | 11 | nandi/glimmer-vidya {:local/root "../vidya/glimmer"}} |
| 9 | 12 | ||
| 10 | ;; libvidya, the Rust/egui build (the C/raylib one does not export the tree | 13 | ;; libvidya, the Rust/egui build (the C/raylib one does not export the tree |
modified
src/frq/app.jolt +22 -6 | @@ -377,7 +377,10 @@ | ||
| 377 | 377 | :margin-top (if same-sender? 0 10) |
| 378 | 378 | ;; The jump target is what a "go to message" click scrolls to. |
| 379 | 379 | :scroll-here (boolean (and (:id m) (= (:id m) @s/jump-to)))} |
| 380 | - [(if highlit? :card :vbox) {:key :body :spacing 2 :margin 0} | |
| 380 | + ;; Likewise a key each: the surface a jump leaves behind is a different | |
| 381 | + ;; node from the plain column, not the same node wearing another tag. | |
| 382 | + [(if highlit? :card :vbox) {:key (if highlit? :body-card :body-plain) | |
| 383 | + :spacing 2 :margin 0} | |
| 381 | 384 | ;; A run from one person reads as one block; repeating the nick on every |
| 382 | 385 | ;; line is what made a busy channel look like a list of headers. The time |
| 383 | 386 | ;; rides on that same line, for the same reason: once per run, not once |
| @@ -416,12 +419,19 @@ | ||
| 416 | 419 | ;; it, and one with a width takes the column's whole height. Reacting |
| 417 | 420 | ;; comes first in the source because a row laid out from the right lays |
| 418 | 421 | ;; its first child furthest right: on screen this reads ↩ then ☺. |
| 422 | + ;; A key of its own for each branch, rather than one key whose node | |
| 423 | + ;; changes tag under it: a line this client sent has no msgid until the | |
| 424 | + ;; server echoes it back, and swapping a `:spacer` for a `:button` in | |
| 425 | + ;; place is what left the row rebuilt in the wrong order — the actions | |
| 426 | + ;; ended up after the text, and every row beside it laid out from an | |
| 427 | + ;; edge that was no longer the row's. Two keys make it a removal and an | |
| 428 | + ;; addition, which is what it is. | |
| 419 | 429 | (if (:id m) |
| 420 | - [react-button @s/current m] | |
| 421 | - [:spacer {:key :react :size 0}]) | |
| 430 | + [react-button {:key :react-button} @s/current m] | |
| 431 | + [:spacer {:key :react-gap :size 0}]) | |
| 422 | 432 | (if (:id m) |
| 423 | - [:button {:key :reply :label "↩" :on-click #(s/reply-to! m)}] | |
| 424 | - [:spacer {:key :reply :size 0}]) | |
| 433 | + [:button {:key :reply-button :label "↩" :on-click #(s/reply-to! m)}] | |
| 434 | + [:spacer {:key :reply-gap :size 0}]) | |
| 425 | 435 | ;; The text column carries the pills as well as the words: put them |
| 426 | 436 | ;; outside this row and they sit below it, and the row is as tall as |
| 427 | 437 | ;; the buttons at its right edge — which is half a line of air between |
| @@ -481,7 +491,13 @@ | ||
| 481 | 491 | new-day? (conj (day-separator (str "day-" i) (clock/day-label (:at m)))) |
| 482 | 492 | ;; A new day breaks the run: the first line under a heading |
| 483 | 493 | ;; names its sender and its time, whoever spoke last night. |
| 484 | - true (conj [message-row i (when-not new-day? prev) m])))) | |
| 494 | + ;; Keyed by the line itself, not by where it sits: the list is | |
| 495 | + ;; reconciled by position when its children are not all keyed, | |
| 496 | + ;; and a line that arrives anywhere but the end — a backlog | |
| 497 | + ;; replay, an echo taking the place of what was sent — shifts | |
| 498 | + ;; every row after it onto the widgets of its neighbour. | |
| 499 | + true (conj ^{:key (or (:id m) (str "row-" i))} | |
| 500 | + [message-row i (when-not new-day? prev) m])))) | |
| 485 | 501 | (range (count messages)) |
| 486 | 502 | messages)) |
| 487 | 503 | |
| @@ -377,7 +377,10 @@ | |||
| 377 | :margin-top (if same-sender? 0 10) | 377 | :margin-top (if same-sender? 0 10) |
| 378 | ;; The jump target is what a "go to message" click scrolls to. | 378 | ;; The jump target is what a "go to message" click scrolls to. |
| 379 | :scroll-here (boolean (and (:id m) (= (:id m) @s/jump-to)))} | 379 | :scroll-here (boolean (and (:id m) (= (:id m) @s/jump-to)))} |
| 380 | - [(if highlit? :card :vbox) {:key :body :spacing 2 :margin 0} | 380 | + ;; Likewise a key each: the surface a jump leaves behind is a different |
| 381 | + ;; node from the plain column, not the same node wearing another tag. | ||
| 382 | + [(if highlit? :card :vbox) {:key (if highlit? :body-card :body-plain) | ||
| 383 | + :spacing 2 :margin 0} | ||
| 381 | ;; A run from one person reads as one block; repeating the nick on every | 384 | ;; A run from one person reads as one block; repeating the nick on every |
| 382 | ;; line is what made a busy channel look like a list of headers. The time | 385 | ;; line is what made a busy channel look like a list of headers. The time |
| 383 | ;; rides on that same line, for the same reason: once per run, not once | 386 | ;; rides on that same line, for the same reason: once per run, not once |
| @@ -416,12 +419,19 @@ | |||
| 416 | ;; it, and one with a width takes the column's whole height. Reacting | 419 | ;; it, and one with a width takes the column's whole height. Reacting |
| 417 | ;; comes first in the source because a row laid out from the right lays | 420 | ;; comes first in the source because a row laid out from the right lays |
| 418 | ;; its first child furthest right: on screen this reads ↩ then ☺. | 421 | ;; its first child furthest right: on screen this reads ↩ then ☺. |
| 422 | + ;; A key of its own for each branch, rather than one key whose node | ||
| 423 | + ;; changes tag under it: a line this client sent has no msgid until the | ||
| 424 | + ;; server echoes it back, and swapping a `:spacer` for a `:button` in | ||
| 425 | + ;; place is what left the row rebuilt in the wrong order — the actions | ||
| 426 | + ;; ended up after the text, and every row beside it laid out from an | ||
| 427 | + ;; edge that was no longer the row's. Two keys make it a removal and an | ||
| 428 | + ;; addition, which is what it is. | ||
| 419 | (if (:id m) | 429 | (if (:id m) |
| 420 | - [react-button @s/current m] | 430 | + [react-button {:key :react-button} @s/current m] |
| 421 | - [:spacer {:key :react :size 0}]) | 431 | + [:spacer {:key :react-gap :size 0}]) |
| 422 | (if (:id m) | 432 | (if (:id m) |
| 423 | - [:button {:key :reply :label "↩" :on-click #(s/reply-to! m)}] | 433 | + [:button {:key :reply-button :label "↩" :on-click #(s/reply-to! m)}] |
| 424 | - [:spacer {:key :reply :size 0}]) | 434 | + [:spacer {:key :reply-gap :size 0}]) |
| 425 | ;; The text column carries the pills as well as the words: put them | 435 | ;; The text column carries the pills as well as the words: put them |
| 426 | ;; outside this row and they sit below it, and the row is as tall as | 436 | ;; outside this row and they sit below it, and the row is as tall as |
| 427 | ;; the buttons at its right edge — which is half a line of air between | 437 | ;; the buttons at its right edge — which is half a line of air between |
| @@ -481,7 +491,13 @@ | |||
| 481 | new-day? (conj (day-separator (str "day-" i) (clock/day-label (:at m)))) | 491 | new-day? (conj (day-separator (str "day-" i) (clock/day-label (:at m)))) |
| 482 | ;; A new day breaks the run: the first line under a heading | 492 | ;; A new day breaks the run: the first line under a heading |
| 483 | ;; names its sender and its time, whoever spoke last night. | 493 | ;; names its sender and its time, whoever spoke last night. |
| 484 | - true (conj [message-row i (when-not new-day? prev) m])))) | 494 | + ;; Keyed by the line itself, not by where it sits: the list is |
| 495 | + ;; reconciled by position when its children are not all keyed, | ||
| 496 | + ;; and a line that arrives anywhere but the end — a backlog | ||
| 497 | + ;; replay, an echo taking the place of what was sent — shifts | ||
| 498 | + ;; every row after it onto the widgets of its neighbour. | ||
| 499 | + true (conj ^{:key (or (:id m) (str "row-" i))} | ||
| 500 | + [message-row i (when-not new-day? prev) m])))) | ||
| 485 | (range (count messages)) | 501 | (range (count messages)) |
| 486 | messages)) | 502 | messages)) |
| 487 | 503 | ||