Reactions on thread replies, in their own id namespace (LC-595) #580
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/lc595-thread-reactions"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes LC-595.
What was broken
The thread panel had no reaction bar at all: a reply could not be reacted to, and its existing reactions were invisible. And because the live
ReactionAdded/ReactionRemovedOOB swap only ever named#reactions-{id}, a reaction made anywhere was a silent no-op for any reply shown in an open panel.Why it needed a namespace, not just a bar
The panel renders the thread ROOT through the same partial as its replies, and the root also appears in the timeline. Reusing the timeline's ids would have recreated exactly the LC-553 bug: a duplicate
#reactions-{id}made the firstouterHTMLtoggle replace the wrong node.So
ReactionSurfaceis now the single source of the namespace.Timelinekeeps the historical unprefixed ids (nothing that already targets them changed);Threadprefixesthread-. Templates call its accessors instead of hardcoding a prefix.The surface has to survive the round trip too: a chip clicked in the panel posts with
?surface=threadso the server answers in the caller's namespace. Without that the response carries the timeline's id and the swap injects the very duplicate the namespace exists to prevent. The picker takes the same parameter, since it is opened from a surface and reacts back into it. Absent, it deserializes toTimeline, so every pre-existing URL is unchanged byte-for-byte (there's a test pinning that).The live update now emits both surfaces unconditionally. The server doesn't know whether a given viewer has a panel open, and an OOB fragment whose target is absent is a no-op in htmx, so sending both is cheaper than tracking panel state and correct in every combination.
The latent duplicate the ticket predicted, confirmed
thread_reply_inner.htmlcarried a comment claiming it avoided emittingid="msg-{id}"and then emitted it anyway. For replies that was harmless by accident (the timeline query filters them out viaparent_id IS NULL), but the thread root was genuinely duplicated for as long as a panel was open. The row is nowthreadmsg-{id}.Two things that fell out rather than being sought
ws/reaction_update.htmlwas a byte-for-byte copy ofpartials/reaction_bar.htmldiffering only byhx-swap-oob, with a comment demanding the two be kept aligned by hand. Every change here would have had to be made twice, so it's folded into one template behind anoobflag, the same patternRoomSeenBaralready uses.get_thread_panelbuilt itsMessageViews withreactions: Vec::new()hardcoded, which is the deeper reason reactions were invisible in the panel. It now loads them through a helper shared by the root and every reply.Testing
just checkclean; suite green at 186 test binaries.New test binary covers: react on a reply, the surface surviving both the chip URL and its target and the picker, the live payload naming both surfaces, and the id invariant, asserted as "the room page and an open panel share exactly one id, the
#thread-panelswap target itself" (which models the outerHTML swap rather than approximating it). Each of the three parts was verified non-vacuous by reverting it and confirming the tests fail.One thing worth a look: I regenerated the four LC-77 render fixtures. The change is 12 blank lines from the new template bindings and no DOM change at all. I verified that rather than asserting it, by filtering the fixture diff for non-blank content and confirming it is empty. Worth a glance since fixtures don't run in CI.
🤖 Generated with Claude Code
https://claude.ai/code/session_019xQaQQJyeJX7ZS9aVECXwF
Reactions rendered only in the main timeline. The thread panel had no reaction bar at all, so a thread reply could not be reacted to and its existing reactions were invisible; and because the live `ReactionAdded` / `ReactionRemoved` OOB swap only ever named `#reactions-{id}`, a reaction made anywhere was a silent no-op for any reply shown in an open panel. The panel could not simply reuse the timeline's ids. It renders the thread ROOT through the same partial that renders replies, and the root also appears in the timeline, so sharing ids would recreate exactly the LC-553 bug where a duplicate `#reactions-{id}` made the first `outerHTML` toggle replace the wrong node. A `ReactionSurface` enum is now the single source of that namespace: `Timeline` keeps the historical unprefixed ids so nothing that already targets them had to change, and `Thread` prefixes `thread-`. Templates call its two accessors rather than hardcoding a prefix. The surface also has to survive the round trip. A chip clicked inside the panel posts with `?surface=thread` so the server answers in the caller's namespace; without that the response would carry the timeline's id and the swap would inject the very duplicate the namespace exists to prevent. The picker takes the same parameter, since it is opened FROM a surface and reacts back INTO it. Absent, the parameter deserializes to `Timeline`, so every pre-existing URL keeps working byte-for-byte. The live update now emits both surfaces unconditionally. The server does not track whether a given viewer has a panel open, and an OOB fragment whose target is absent is a no-op in htmx, so sending both is cheaper than tracking panel state and correct in every combination. This also fixes a latent duplicate the ticket turned up: `thread_reply_inner.html` carried a comment claiming it avoided emitting `id="msg-{id}"`, and then emitted it anyway. For replies that was harmless by accident, since the timeline query filters them out with `parent_id IS NULL`, but the thread root was genuinely duplicated for as long as a panel was open. The row is now `threadmsg-{id}`. Two pieces of tidying fell out rather than being sought. `ws/reaction_update.html` was a byte-for-byte copy of `partials/reaction_bar.html` differing only by `hx-swap-oob`, carrying a comment demanding the two be kept aligned by hand; every change here would have had to be applied twice, so it is folded into the one template behind an `oob` flag (the same pattern `RoomSeenBar` already uses). And `get_thread_panel` built its `MessageView`s with `reactions: Vec::new()` hardcoded, which is why reactions were invisible in the panel even before rendering was possible; it now loads them through a helper shared by the root and every reply. Tests: a new binary covering react-on-a-reply, the surface surviving both the chip URL and its target and the picker, the live payload naming both surfaces, and the id invariant - the room page and an open panel share exactly one id, the `#thread-panel` swap target itself. Each of the three parts was verified non-vacuous by reverting it and confirming the tests fail. The four LC-77 render fixtures were regenerated. The diff is 12 blank lines from the new template bindings and no DOM change whatsoever; verified by filtering the fixture diff for non-blank content, which is empty. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019xQaQQJyeJX7ZS9aVECXwF