fix(dm): Enter-to-send, move DMs to left rail, monotonic publish version#258
Merged
Conversation
…n to monotonic counter Three fixes after the #244 production release surfaced UX bugs in real use: ## Enter-to-send in the DM composer Pressing Enter in the DM textarea did nothing — only the Send button worked. The room-message composer already handles `Enter sends, Shift+Enter newlines`; mirror that in `dm_thread_modal.rs`. Refactored the existing `send` closure into a no-arg `do_send` callable so both `onclick` and the new `onkeydown` invoke the same path. Reported by zorolin in the official River room 2026-05-16. ## "Direct Messages" section in the left rail (under Rooms) The previous DM entry point was a button buried in the Members panel, which surfaced only DMs for the currently-selected room. zorolin reported it was confusing; both Ian and zorolin agreed the right shape is a section in the left rail, parallel to Rooms, listing every DM thread across every room the local user is in. - New `components/room_list/dm_rail_section.rs` renders a flat list of `(room, peer)` entries with peer nickname, room name, and unread badge. Sort: unread first, then most-recent. - Hidden when empty so first-load doesn't show a stray section. - Click a row → opens the existing `DmThreadModal`. Cross-room navigation works because the modal takes `(room, peer)` props. - Removed the now-redundant "Direct Messages" button from `members.rs` and dropped the no-longer-reachable `DmInboxModal` + `DM_INBOX_OPEN` signal + `open_dm_inbox` helper. ## Monotonic counter for the web-container publish version The previous scheme used `seconds / 60` to derive the metadata version. This failed on 2026-05-16 production publish: the on-network version was 30000208 (likely a past clock-skew bump that stuck), the timestamp gave 29649402, and fdev rejected the publish. Manual recovery worked but required guessing a number ahead of all past values. Replaced with a committed counter at `published-contract/contract-version.txt`. `cargo make sign-webapp` reads it, increments, writes back, and signs with the new value. Mirror change in `sign-webapp-test` against `test-contract/contract-version.txt`. Updated AGENTS.md, the local river-publish rule, and the global river-publish skill (pushed to ~/.claude in 5f22880). ## What's not in this PR - Outbound DM plaintext persistence — filed as #256. Needs a chat- delegate WASM bump + `legacy_delegates.toml` entry, which is its own work cycle. - Test-publish sed-replace WASM corruption (43-vs-44-char contract IDs) — filed as #257. Manual visual verification against `publish-river-test` was flaky for this PR; the Enter fix mirrors the proven room-message handler and the rail section is a `use_memo`-driven read-only view, so the risk surface is small. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Multi-model review of #258 caught three issues: # Skeptical High — version-counter drift on publish failure Bumping `published-contract/contract-version.txt` BEFORE the actual `fdev publish` means a failed publish leaves the local counter ahead of the on-network value. Re-introduces the same drift class the PR claimed to fix. Fix: `sign-webapp` now copies the counter to `.prev` before bumping; `publish-river` (and `publish-river-debug`, `publish-river-test`) rolls the counter back from the `.prev` snapshot if `fdev publish` fails. Successful publish removes the snapshot. The `.prev` file is gitignored so it can never sneak into a commit. Concurrent publishes on the same workstation are documented as unsupported; wrap in `flock` if you need to. Code-first review pass made the same call. # Skeptical Medium — DM_LAST_SEEN unread-flash window on hydration The seed effect deferred BOTH the DM_LAST_SEEN write AND the DM_LAST_SEEN_SEEDED flag latch. Between the first non-empty ROOMS observation and the deferred tick, consumers (rail section, document title) read DM_LAST_SEEN as empty -> every historical DM looks unread for one render frame. Fix: latch the flag synchronously INSIDE the defer body before the DM_LAST_SEEN write. Consumers now see "seeded, write in flight" rather than "not seeded" during the window. # Code-first / Skeptical Medium — stale module doc `ui/src/components/direct_messages.rs` module docstring still described the removed `DmInboxModal`. Updated to describe the rail section and to point at the new seed function. # Codex P1 — false positive Codex flagged the `do_send` closure being captured into two `move` event handlers as a use-after-move compile error. The Dioxus rsx! macro wraps event handlers in `EventHandler<T>` (Rc-boxed), so each closure captures the inner state independently. `cargo check --target wasm32-unknown-unknown` confirms — the same pattern works in conversation.rs::message_input for room-message Enter-to-send. # Skeptical Low / nit findings — not addressed in this commit - L1 (O(rooms × dms) per render in rail section): bounded by MAX_DM_MESSAGES_PER_PAIR, accept. - L2 (try_read subscription edge case): existing pattern, not a regression. - M1 (peer_still_member stale snapshot): `do_send`'s own resolve_peer_vk re-read is the source of truth; the handler-side gate is just a fast path that matches the visible disabled state. Acceptable as-is. - L4 (room_name.clone() per peer): cosmetic, < 1ms. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Pass 1 review consolidated (code-first + skeptical + Codex): Fixed in dcd4c06:
Codex P1 false positive: Deferred (acceptable):
[AI-assisted - Claude] |
sanity
added a commit
that referenced
this pull request
May 16, 2026
[AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 16, 2026
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
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.
Three fixes after the #244 production release surfaced real UX issues with first users (zorolin tried DMs in the official room and surfaced both Enter-to-send and the buried entry point).
1. Enter-to-send in the DM composer
The DM textarea ignored Enter; only the Send button worked. The room-message composer already had Enter-to-send / Shift-Enter-newline; this PR mirrors it.
Refactored the existing send closure into a no-arg
do_sendcallable so bothonclick(button) and the newonkeydown(Enter) invoke the same path.2. "Direct Messages" section in the left rail under Rooms
zorolin's feedback: the previous DM entry point — a button in the Members panel — was confusing, and per-room-only made cross-room DMs invisible. Both Ian and zorolin agreed DMs belong in the left rail next to Rooms.
components/room_list/dm_rail_section.rsrenders a flat list across ALL rooms. Each row: peer nickname + room name + unread badge. Sorted unread-first, then most-recent.DmThreadModalfor that(room, peer).members.rsand dropped the unusedDmInboxModal+DM_INBOX_OPENsignal +open_dm_inboxhelper.3. Monotonic counter for the web-container publish version
The
seconds / 60scheme failed on 2026-05-16: on-network version had drifted ahead to 30000208 (likely a past clock-skew bump that stuck), timestamp gave 29649402, fdev rejected the publish. Recovery required guessing a number ahead of all past values.Replaced with
published-contract/contract-version.txt— committed counter, incremented bycargo make sign-webappeach publish and committed alongside. Same pattern intest-contract/contract-version.txtfor the test path.Updated AGENTS.md,
.claude/rules/river-publish.md, and the global~/.claude/skills/river-publish/SKILL.md(pushed to ~/.claude in commit 5f22880).What's deferred (not in this PR)
legacy_delegates.tomlmigration entry). Another agent is taking that one.Testing
cargo check -p river-ui --target wasm32-unknown-unknown --no-default-features✓cargo test -p river-ui --bins --no-default-features components::direct_messages::tests✓ (3 tests pass)use_memo-driven read-only view, so the risk surface is small. Will visually verify against production after merge.Closes (none — three separate concerns bundled because they're all small UX iterations from the same release).
[AI-assisted - Claude]