From 71e3203327c20eb3b412580eed9720ac6c0ca485 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Mon, 18 May 2026 10:27:12 -0500 Subject: [PATCH 1/4] fix(dm/private-rooms): batch UI polish for 5 issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundles five user-impacting UI fixes; all UI-only, no contract or delegate WASM changes, no migration entry required. * #283 — Auto-scroll in DM thread modal didn't fire after PR #278's Codex round-1 fix removed the reactive read from `use_effect`. Mirrored the conversation.rs pattern: the last bubble's onmounted updates a `Signal>>` and the effect reads it as a subscribing call so it re-fires on new bubble mounts. * #284 — Encrypted-message placeholder during the invite-acceptance sync window (new private-room invitee, no secrets yet) was alarming ("[Encrypted message - secret vN not available (have: [])]"). When the secrets map is empty render "Decrypting your invitation — this should only take a moment..."; when SOME secrets are present but not this version, render a neutral diagnostic. * #267 — Same-second / clock-skewed inbound DM didn't revive a hidden thread (filter rule is strict `<=`). Added an explicit `unhide_dm_thread` call from the inbound delta path in `apply_delta_inner`, symmetric with the existing outbound-send unhide in `dm_thread_modal::do_send`. * #279 — In-app invite-accept on a stale invite card re-opened the full nickname-prompt flow for a previously-LeaveRoom'd room. The `PRESENT_INVITATION_REQUEST` bridge effect now gates on `is_invitation_processed`, matching the URL-bar and click- interceptor paths. * #280 — Banned user saw an ENABLED "Accept invitation" button on the invite card because `already_member = can_participate().is_ok()` collapsed Banned and NotMember to the same false. Replaced the boolean with a `InviteCardState::{Joinable, AlreadyMember, Banned}` enum; the Banned variant disables the button with destructive styling and "You're banned from this room". Regression tests added: * `auto_scroll_effect_subscribes_to_last_dm_bubble_signal` + `dm_bubble_exposes_last_bubble_sink_prop` — source-text pins so a future refactor that "cleans up" the unused-looking signal can't silently re-break #283. * `invite_card_state_classifies_*` (5 tests) — pure-function pins for the new `classify_invite_card_state` helper covering each match arm + the "must distinguish banned from not-a-member" invariant. * `decrypt_placeholder_for_empty_secrets_is_friendly` + `decrypt_placeholder_for_missing_version_is_neutral_not_alarming` — pin both #284 placeholder branches. * `apply_delta_inner_revives_hidden_thread_for_inbound_dm_sender` — source-text pin for the #267 wiring in apply_delta_inner. * `dm_accept_bridge_gates_on_is_invitation_processed` — source-text pin for the #279 gate in the bridge effect. Closes #267 #279 #280 #283 #284 [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 --- ui/src/components/app.rs | 60 ++++ .../app/freenet_api/room_synchronizer.rs | 90 +++++ ui/src/components/conversation.rs | 107 +++++- .../direct_messages/dm_thread_modal.rs | 309 +++++++++++++++--- 4 files changed, 511 insertions(+), 55 deletions(-) diff --git a/ui/src/components/app.rs b/ui/src/components/app.rs index 75333a96..1e5194b8 100644 --- a/ui/src/components/app.rs +++ b/ui/src/components/app.rs @@ -242,6 +242,14 @@ pub fn App() -> Element { // interceptor bridge above (AGENTS.md "Never defer signal clears in // use_effect"). // + // Gate on `is_invitation_processed` — without it, a previously- + // accepted-then-LeaveRoom'd invitation re-presented via the DM + // accept-card would pop the nickname-prompt modal again, which is + // alarming for the user and forces them through the whole join + // flow for a room they've already acted on (#279). The URL-bar + // (line 192) and click-interceptor (line 141) paths already gate + // on this; the DM-card bridge was the missing case. + // // `try_read() -> Err` on the FIRST render is benign: the picker / // accept-card paths only ever write via `crate::util::defer()`, which // schedules a setTimeout(0) macrotask — the writer is never holding @@ -258,6 +266,21 @@ pub fn App() -> Element { return; }; *PRESENT_INVITATION_REQUEST.write() = None; + let fingerprint = inv.to_encoded_string(); + if is_invitation_processed(&fingerprint) { + // The user has already accepted or dismissed this + // invitation in this browser. Don't re-open the modal: + // either they're still a member (in which case the rail + // already shows the room) or they've explicitly left and + // re-presenting the join flow would be confusing. Silent + // drop is the same handling the click-interceptor bridge + // uses for the equivalent case. + debug!( + "In-app invitation accept ignored: already processed for room {:?}", + MemberId::from(inv.room) + ); + return; + } info!( "In-app invitation accept: opening modal for room {:?}", MemberId::from(inv.room) @@ -461,3 +484,40 @@ fn get_auth_token_from_window() { } } } + +#[cfg(test)] +mod tests { + // ----------------------------------------------------------------- + // Issue freenet/river#279 regression guard: + // + // The bridge effect that translates `PRESENT_INVITATION_REQUEST` + // (set by the DM thread's "Accept" button on an invite-card DM) + // into the local `receive_invitation` signal MUST gate on + // `is_invitation_processed`. Without that gate, a previously- + // accepted-then-LeaveRoom'd room re-presented via the DM accept + // path re-opens the full nickname-prompt flow — which is alarming + // for the user and forces them to re-run the join flow for a + // room they already acted on. + // + // The URL-bar and click-interceptor paths already gate on this; + // the DM bridge was the missing case. Source-text pin because the + // effect runs inside Dioxus's render loop and isn't unit-testable + // without the runtime. + // ----------------------------------------------------------------- + #[test] + fn dm_accept_bridge_gates_on_is_invitation_processed() { + let src = include_str!("app.rs"); + // Pin the gate by searching the bridge effect for the + // is_invitation_processed call AND the early-return comment. + // A future refactor that removes the gate would need to also + // update this test, which is the intended forcing function. + assert!( + src.contains("is_invitation_processed(&fingerprint)") + && src.contains("In-app invitation accept ignored: already processed"), + "The PRESENT_INVITATION_REQUEST bridge effect must gate on \ + is_invitation_processed before opening the nickname-prompt \ + modal — otherwise a stale invite card re-presents the full \ + join flow for a room the user has already accepted/left (#279)." + ); + } +} diff --git a/ui/src/components/app/freenet_api/room_synchronizer.rs b/ui/src/components/app/freenet_api/room_synchronizer.rs index 32e45aa6..54a9441a 100644 --- a/ui/src/components/app/freenet_api/room_synchronizer.rs +++ b/ui/src/components/app/freenet_api/room_synchronizer.rs @@ -71,6 +71,29 @@ impl RoomSynchronizer { fn apply_delta_inner(owner_vk: VerifyingKey, delta: ChatRoomStateV1Delta) { // Extract new messages for notifications before entering the mutable borrow let new_messages = delta.recent_messages.clone(); + // Extract any inbound DM senders so we can revive hidden threads + // for the (room, peer) pair after the merge lands. Issue + // freenet/river#267: when the user hides a thread and a new + // inbound DM lands in the same unix-second (or with clock skew + // putting its timestamp at `hidden_at_ts`), the filter's strict- + // `<=` rule keeps the thread hidden. Explicit unhide is + // deterministic and idempotent — it pairs with the existing + // outbound-send unhide in `dm_thread_modal::do_send` so both + // directions revive symmetrically. We only filter to inbound + // (to-us) DMs here: outbound (from-us) DMs are already revived + // by `unhide_dm_thread` in the send path. The list is collected + // outside the `with_mut` borrow but actually invoked AFTER the + // borrow drops to keep the signal-write ordering clean. + let inbound_dm_senders: Vec = delta + .direct_messages + .as_ref() + .map(|dm| { + dm.new_messages + .iter() + .map(|m| m.message.sender) + .collect::>() + }) + .unwrap_or_default(); // Will be populated inside with_mut if new messages need notification let mut pending_notification: Option<( @@ -222,6 +245,38 @@ impl RoomSynchronizer { } }); + // Issue freenet/river#267: revive any hidden thread for an + // (owner_vk, sender) pair that just received an inbound DM. + // We post-filter to peers that are NOT ourselves — outbound + // DMs go through their own `unhide_dm_thread` call site in + // `dm_thread_modal::do_send` / `direct_messages::send_structured_dm`. + // Self-id is looked up under a fresh ROOMS borrow because the + // earlier `with_mut` already dropped its guard. + if !inbound_dm_senders.is_empty() { + let self_id_opt: Option = ROOMS.try_read().ok().and_then(|r| { + r.map + .get(&owner_vk) + .map(|rd| rd.self_sk.verifying_key().into()) + }); + if let Some(self_id) = self_id_opt { + // De-duplicate before firing the unhide (multiple + // inbound DMs from the same peer in one batch only need + // one unhide call). `unhide_dm_thread` is idempotent, so + // duplicates are safe, but de-duping avoids redundant + // delegate saves. + let mut seen: std::collections::HashSet = + std::collections::HashSet::new(); + for sender in inbound_dm_senders { + if sender == self_id { + continue; + } + if seen.insert(sender) { + crate::components::app::chat_delegate::unhide_dm_thread(owner_vk, sender); + } + } + } + } + // Update document title after ROOMS.with_mut completes (update_document_title calls ROOMS.read()) update_document_title(); @@ -1328,4 +1383,39 @@ mod tests { full_size ); } + + // ----------------------------------------------------------------- + // Issue freenet/river#267 regression guard: + // + // The DM rail filter uses strict-`<=` against `hidden_at_ts` (see + // `chat_delegate::is_thread_hidden`), so an inbound DM whose + // timestamp falls exactly on the cutoff (same unix-second as the + // hide, or clock skew) leaves the thread hidden. The fix is an + // explicit `unhide_dm_thread(owner_vk, sender)` call from the + // inbound delta path in `apply_delta_inner`, mirroring the + // outbound-send unhide in `dm_thread_modal::do_send` and + // `direct_messages::send_structured_dm`. + // + // We can't unit-test the full delta path without standing up the + // Dioxus runtime + ROOMS signal, so this is a source-text pin: + // the wiring MUST extract inbound senders from the delta and feed + // them into `unhide_dm_thread`. A future refactor that drops the + // call site would otherwise silently re-regress #267. + // ----------------------------------------------------------------- + #[test] + fn apply_delta_inner_revives_hidden_thread_for_inbound_dm_sender() { + let src = include_str!("room_synchronizer.rs"); + assert!( + src.contains("inbound_dm_senders"), + "apply_delta_inner must collect inbound DM senders from the delta \ + so it can call unhide_dm_thread for the (room, peer) pair (#267)" + ); + assert!( + src.contains("chat_delegate::unhide_dm_thread("), + "apply_delta_inner must call unhide_dm_thread on each inbound DM \ + sender so a hidden thread is revived even when the new DM's \ + timestamp matches the hide cutoff exactly (#267). The filter's \ + strict-`<=` rule alone is not sufficient for the same-second case." + ); + } } diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index e85f95c2..9bc988e4 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -322,11 +322,28 @@ fn decrypt_message_content(content: &RoomMessageBody, secrets: &HashMap>() + "[Encrypted message - secret v{} unavailable]", + secret_version ) } } @@ -2745,4 +2762,88 @@ mod tests { "non-http(s) scheme must not be rewritten to same-origin: {html}" ); } + + // ----------------------------------------------------------------- + // Issue freenet/river#284 regression coverage: + // + // A newly-invited member of a private room briefly has NO local + // secrets — the chat-delegate hasn't published the owner's back- + // fill ciphertext yet (or it's mid-flight). The previous + // diagnostic placeholder "[Encrypted message - secret vN not + // available (have: [])]" was alarming and looked like data loss. + // Replace it with a calm, plain-language explanation that this is + // expected and will resolve in a few seconds. Once any secret + // arrives the decryption branch fires and the placeholder + // disappears. + // + // The "we have SOME secrets but not THIS version" case is left as + // a less-alarming neutral diagnostic — that's the rotated-past + // case rather than the sync-window case, and it deserves a + // different message than the joiner's UX. + // ----------------------------------------------------------------- + + fn private_msg_body(secret_version: u32) -> river_core::room_state::message::RoomMessageBody { + // Hand-construct a Private body — we don't actually decrypt in + // these tests, just exercise the placeholder-selection branch. + river_core::room_state::message::RoomMessageBody::Private { + content_type: river_core::room_state::content::CONTENT_TYPE_TEXT, + content_version: river_core::room_state::content::TEXT_CONTENT_VERSION, + ciphertext: vec![0u8; 32], + nonce: [0u8; 12], + secret_version, + } + } + + /// Issue #284: empty-secrets-map case (joiner sync window) renders + /// the friendly "Decrypting your invitation" message, not the + /// diagnostic placeholder that exposes raw `(have: [])` internals. + #[test] + fn decrypt_placeholder_for_empty_secrets_is_friendly() { + let body = private_msg_body(1); + let secrets: HashMap = HashMap::new(); + let rendered = decrypt_message_content(&body, &secrets); + assert!( + rendered.contains("Decrypting your invitation"), + "empty-secrets-map (sync window) must render the friendly \ + explanation, got: {rendered}" + ); + assert!( + !rendered.contains("(have: ["), + "the alarming diagnostic dump must NOT appear in the sync-window \ + placeholder, got: {rendered}" + ); + } + + /// Issue #284: when we have SOME secrets but not the one this + /// message was encrypted under, render a neutral placeholder. + /// This is the rotated-past case, not the sync-window case — the + /// user has decrypted other messages successfully, so the friendly + /// "your invitation is still arriving" copy would be wrong here. + #[test] + fn decrypt_placeholder_for_missing_version_is_neutral_not_alarming() { + let body = private_msg_body(5); + // We have version 1 and 2 but not 5 (the one needed). + let mut secrets: HashMap = HashMap::new(); + secrets.insert(1, [0u8; 32]); + secrets.insert(2, [0u8; 32]); + let rendered = decrypt_message_content(&body, &secrets); + assert!( + !rendered.contains("Decrypting your invitation"), + "joiner-friendly copy must not surface when secrets ARE \ + populated (this is the rotated-past case, not sync-window), \ + got: {rendered}" + ); + assert!( + !rendered.contains("(have: ["), + "the alarming diagnostic dump must not appear in any \ + placeholder branch, got: {rendered}" + ); + // We DO want to surface the version number for diagnostics — it + // helps both the user and the developer triage rotation issues. + assert!( + rendered.contains("v5"), + "the rotated-past placeholder should surface the missing \ + version number, got: {rendered}" + ); + } } diff --git a/ui/src/components/direct_messages/dm_thread_modal.rs b/ui/src/components/direct_messages/dm_thread_modal.rs index 2c7368c6..74f2c611 100644 --- a/ui/src/components/direct_messages/dm_thread_modal.rs +++ b/ui/src/components/direct_messages/dm_thread_modal.rs @@ -16,6 +16,7 @@ use crate::components::direct_messages::{ }; use crate::components::members::Invitation; use crate::components::room_list::receive_invitation_modal::present_invitation; +use crate::room_data::SendMessageError; use crate::util::ecies::unseal_bytes_with_secrets; use dioxus::logger::tracing::{error, info, warn}; use dioxus::prelude::*; @@ -28,6 +29,7 @@ use river_core::room_state::direct_messages::{ use river_core::room_state::dm_body::{decode_body, DirectMessageBody, InvitePayload}; use river_core::room_state::member::MemberId; use river_core::room_state::{ChatRoomParametersV1, ChatRoomStateV1Delta}; +use std::rc::Rc; use std::time::{SystemTime, UNIX_EPOCH}; /// Loose body cap (in bytes) to keep us under `MAX_DM_CIPHERTEXT_BYTES` once @@ -230,8 +232,9 @@ fn DmThreadModalBody(room: VerifyingKey, peer: MemberId) -> Element { // render path itself is purely // declarative. let target_room_data = rooms.map.get(&payload.room_owner_vk); - let already_member = - target_room_data.is_some_and(|rd| rd.can_participate().is_ok()); + let card_state = classify_invite_card_state( + target_room_data.map(|rd| rd.can_participate()), + ); let room_label = target_room_data .map(|rd| { let sealed = @@ -250,7 +253,7 @@ fn DmThreadModalBody(room: VerifyingKey, peer: MemberId) -> Element { BodyKind::Invite(Box::new(InviteCardData { payload: *payload, room_label, - already_member, + card_state, personal_message: personal, })), ) @@ -333,20 +336,25 @@ fn DmThreadModalBody(room: VerifyingKey, peer: MemberId) -> Element { // near the bottom (within ~50px). Don't // yank a reader who's scrolled up. // - // The effect re-fires on each render of the component. It reads - // `view_data.messages.len()` (via the parent memo's ROOMS - // subscription, captured here) as the reactive trigger, and - // `OUTBOUND_SEND_COUNTER` via `.peek()` (NON-reactive — see the - // per-line comment below for why). A `use_hook` Cell remembers - // the previous outbound counter so we can tell "user just sent" - // from "peer sent or initial mount". + // For the effect to re-fire when a new bubble lands we need an + // actual subscribed signal read inside the closure. Dioxus 0.7's + // `use_effect` only re-runs when a signal that was `.read()` + // SUCCESSFULLY inside the closure body changes — capturing a + // plain `usize` `message_count` does not create a subscription, + // and `.peek()` on `OUTBOUND_SEND_COUNTER` is also non-reactive. + // PR #278's Codex round-1 fix replaced the previous `.read()` on + // `OUTBOUND_SEND_COUNTER` with `.peek()` for re-entrancy safety; + // that left the effect with no reactive read at all (issue #283). // - // Why a counter and not just a boolean flag: a boolean would have - // to be cleared, and the cleanup race (clear-vs-next-send) is - // exactly the kind of effect re-entry the rule about "never defer - // signal clears in `use_effect`" was written to prevent. A - // monotonic counter sidesteps the issue. - let message_count = view_data.messages.len(); + // Mirror the conversation.rs pattern: the LAST DM bubble's + // `onmounted` updates `last_dm_bubble`, and the effect reads + // `last_dm_bubble()` (calls the Signal as a function = subscribing + // read). Whenever a new bubble mounts at a different `Rc` identity, + // the effect re-fires. `OUTBOUND_SEND_COUNTER.peek()` stays + // non-reactive — the bubble mount provides the trigger; the + // counter is just consulted to classify the trigger as + // outbound-vs-inbound. + let last_dm_bubble: Signal>> = use_signal(|| None); let first_scroll_done = use_hook(|| std::rc::Rc::new(std::cell::Cell::new(false))); let prev_outbound_bump = use_hook(|| std::rc::Rc::new(std::cell::Cell::new(0u64))); #[cfg(target_arch = "wasm32")] @@ -354,21 +362,19 @@ fn DmThreadModalBody(room: VerifyingKey, peer: MemberId) -> Element { let first_scroll_done = first_scroll_done.clone(); let prev_outbound_bump = prev_outbound_bump.clone(); use_effect(move || { - // Read `message_count` (captured from the parent memo, - // which subscribes to ROOMS) — that's the reactive trigger - // that re-fires the effect whenever a DM (inbound OR - // outbound) lands. We DON'T `.read()` `OUTBOUND_SEND_COUNTER` - // here: per AGENTS.md "Dioxus WASM Signal Safety Rules", - // the write that bumped the counter can notify subscribers - // synchronously during its own write-guard Drop, which - // would panic a plain `.read()` (Codex P2 finding on PR - // #278). `.peek()` is non-reactive and avoids that risk - // entirely. The subscription comes from the parent's - // ROOMS read via `message_count`; both apply_delta (which - // bumps message_count) and the counter write happen in - // the same defer block, so by the time the effect runs - // both values are current. - let _ = message_count; + // Read the last-bubble signal as a SUBSCRIBING read so the + // effect re-runs whenever a fresh bubble mounts (i.e. new + // DM lands or the modal opens with messages already in + // view). Without this, Dioxus has no signal to watch and + // the effect runs exactly once on mount — leaving auto- + // scroll silently broken on subsequent messages (#283). + let trigger = last_dm_bubble(); + if trigger.is_none() { + // No bubble has mounted yet (empty-thread state or + // first render before mounts arrive). Stay subscribed + // and bail until the first mount lands. + return; + } let outbound_bump_now = *OUTBOUND_SEND_COUNTER.peek(); let prev_bump = prev_outbound_bump.get(); let outbound_changed = outbound_bump_now != prev_bump; @@ -404,7 +410,7 @@ fn DmThreadModalBody(room: VerifyingKey, peer: MemberId) -> Element { #[cfg(not(target_arch = "wasm32"))] { // Touch the values so they're not flagged as unused on native. - let _ = (message_count, &first_scroll_done, &prev_outbound_bump); + let _ = (&last_dm_bubble, &first_scroll_done, &prev_outbound_bump); } let peer_label = view_data.peer_nickname.clone(); @@ -820,8 +826,23 @@ fn DmThreadModalBody(room: VerifyingKey, peer: MemberId) -> Element { "No messages yet. Say hello!" } } else { - for (idx, m) in view_data.messages.iter().enumerate() { - DmBubble { key: "{idx}_{m.timestamp}", message: m.clone() } + { + let messages_len = view_data.messages.len(); + view_data.messages.iter().enumerate().map(move |(idx, m)| { + let is_last = idx + 1 == messages_len; + let on_mount = if is_last { + Some(last_dm_bubble) + } else { + None + }; + rsx! { + DmBubble { + key: "{idx}_{m.timestamp}", + message: m.clone(), + last_bubble_sink: on_mount, + } + } + }) } } } @@ -1017,17 +1038,55 @@ struct InviteCardData { /// configured display name if the local user is already a member of /// that room, otherwise a short owner-key prefix. room_label: String, - /// Whether the local user is already a member of `payload.room_owner_vk`. - /// When `true` the Accept button reads "Already a member" and is - /// disabled (no point re-presenting an invitation the user has - /// already accepted). - already_member: bool, + /// State of the local user with respect to the target room. Drives + /// the Accept button label / disabled state on the card. Replaces an + /// earlier `already_member: bool` that conflated "banned" with + /// "joinable" — banned users saw an enabled "Accept invitation" + /// button that would silently fail on submit (#280). + card_state: InviteCardState, /// Optional sender-typed message rendered inside the card above the /// Accept button. `None` collapses the message slot entirely so we /// don't render an empty line. personal_message: Option, } +/// Three-way classification of the local user vs the invitation's +/// target room, computed at memo time from `RoomData::can_participate`. +/// +/// * `Joinable` — room not loaded OR loaded but the user is neither a +/// member nor banned. Accept proceeds normally. +/// * `AlreadyMember` — `can_participate() == Ok(())`. Accept button is +/// disabled and re-labelled to avoid duplicate-join attempts. +/// * `Banned` — `can_participate() == Err(UserBanned)`. Accept button +/// is disabled with destructive styling and a clear message; without +/// this, the prior `already_member = can_participate().is_ok()` +/// collapse left the Accept button enabled for banned users, which +/// would silently fail on submit (#280). +#[derive(Clone, Copy, PartialEq, Debug)] +enum InviteCardState { + Joinable, + AlreadyMember, + Banned, +} + +/// Classify `can_participate()` (or "room not loaded") into the +/// three-way card state used by [`DmInviteCard`]. Extracted as a pure +/// function so the regression test +/// `invite_card_state_distinguishes_banned_from_not_a_member` doesn't +/// have to stand up a full `RoomData`. +fn classify_invite_card_state( + can_participate: Option>, +) -> InviteCardState { + match can_participate { + Some(Ok(())) => InviteCardState::AlreadyMember, + Some(Err(SendMessageError::UserBanned)) => InviteCardState::Banned, + // `UserNotMember` and "room not loaded" both fall through to + // joinable — the Accept flow itself handles "not yet a member" + // via the normal invitation path. + Some(Err(SendMessageError::UserNotMember)) | None => InviteCardState::Joinable, + } +} + #[derive(Clone, PartialEq)] struct RenderedDm { outgoing: bool, @@ -1038,7 +1097,15 @@ struct RenderedDm { } #[component] -fn DmBubble(message: RenderedDm) -> Element { +fn DmBubble( + message: RenderedDm, + /// When `Some`, this bubble is the LAST one in the rendered list + /// and should report its mount via the supplied signal so the + /// auto-scroll effect in [`DmThreadModalBody`] can re-fire (issue + /// freenet/river#283). Only the last bubble carries this; earlier + /// bubbles' mounts are irrelevant to scroll-to-bottom. + last_bubble_sink: Option>>>, +) -> Element { let ts_label = format_local_time(message.timestamp); let align_class = if message.outgoing { "self-end bg-accent/20 text-text" @@ -1087,7 +1154,13 @@ fn DmBubble(message: RenderedDm) -> Element { } }; rsx! { - div { class: "flex flex-col", + div { + class: "flex flex-col", + onmounted: move |cx| { + if let Some(mut sink) = last_bubble_sink { + sink.set(Some(cx.data())); + } + }, {bubble_body} span { class: if message.outgoing { "self-end text-[10px] text-text-muted mt-0.5" } else { "self-start text-[10px] text-text-muted mt-0.5" }, @@ -1112,7 +1185,7 @@ fn DmBubble(message: RenderedDm) -> Element { #[component] fn DmInviteCard(card: InviteCardData) -> Element { let room_label = card.room_label.clone(); - let already_member = card.already_member; + let card_state = card.card_state; let invitation_payload_bytes = card.payload.invitation_payload.clone(); let expected_room = card.payload.room_owner_vk; let personal_message = card.personal_message.clone(); @@ -1122,19 +1195,30 @@ fn DmInviteCard(card: InviteCardData) -> Element { // right above the message that caused the failure). Empty by default. let mut accept_error: Signal> = use_signal(|| None); - let accept_label = if already_member { - "Already a member" - } else { - "Accept invitation" + let accept_label = match card_state { + InviteCardState::AlreadyMember => "Already a member", + InviteCardState::Banned => "You're banned from this room", + InviteCardState::Joinable => "Accept invitation", }; - let button_class = if already_member { - "px-3 py-1.5 text-sm rounded-lg bg-surface text-text-muted cursor-not-allowed" - } else { - "px-3 py-1.5 text-sm rounded-lg bg-accent hover:bg-accent-hover text-white transition-colors" + let button_class = match card_state { + InviteCardState::AlreadyMember => { + "px-3 py-1.5 text-sm rounded-lg bg-surface text-text-muted cursor-not-allowed" + } + InviteCardState::Banned => { + // Destructive-tinted disabled button. `cursor-not-allowed` + // mirrors the AlreadyMember styling so the affordance is + // identical (the button doesn't act); the red tint + // communicates the failure mode. + "px-3 py-1.5 text-sm rounded-lg bg-red-500/20 text-red-300 border border-red-500/40 cursor-not-allowed" + } + InviteCardState::Joinable => { + "px-3 py-1.5 text-sm rounded-lg bg-accent hover:bg-accent-hover text-white transition-colors" + } }; + let is_disabled = !matches!(card_state, InviteCardState::Joinable); let on_accept = move |_| { - if already_member { + if is_disabled { return; } accept_error.set(None); @@ -1177,7 +1261,7 @@ fn DmInviteCard(card: InviteCardData) -> Element { div { class: "flex justify-end pt-1", button { class: "{button_class}", - disabled: already_member, + disabled: is_disabled, onclick: on_accept, "{accept_label}" } @@ -1492,4 +1576,125 @@ mod tests { let prefix = short_vk_prefix(&vk); assert_eq!(prefix.chars().count(), 8); } + + // ----------------------------------------------------------------- + // Issue freenet/river#280 regression coverage: + // `classify_invite_card_state` must distinguish "banned" from + // "not-a-member" / "already-member" so the Accept button is + // appropriately disabled with destructive copy when the local user + // is banned. The previous `already_member = can_participate().is_ok()` + // collapse rendered an ENABLED "Accept invitation" button for + // banned users that would silently fail on submit. + // ----------------------------------------------------------------- + + /// `Some(Ok(()))` — local user IS a member of the target room. + /// Card must read "Already a member". + #[test] + fn invite_card_state_classifies_already_member() { + assert_eq!( + classify_invite_card_state(Some(Ok(()))), + InviteCardState::AlreadyMember, + ); + } + + /// `Some(Err(UserBanned))` — local user is BANNED. Must NOT collapse + /// to "Joinable" (the #280 bug); must distinguish from the + /// not-a-member case so the card can render a clear disabled-with- + /// reason state instead of an enabled-but-doomed-to-fail button. + #[test] + fn invite_card_state_classifies_banned() { + assert_eq!( + classify_invite_card_state(Some(Err(SendMessageError::UserBanned))), + InviteCardState::Banned, + ); + } + + /// `Some(Err(UserNotMember))` — room is loaded (observer-only) but + /// the user can still join via the invite. Card stays joinable. + #[test] + fn invite_card_state_classifies_not_member_as_joinable() { + assert_eq!( + classify_invite_card_state(Some(Err(SendMessageError::UserNotMember))), + InviteCardState::Joinable, + ); + } + + /// `None` — room not loaded at all. Card stays joinable — the Accept + /// flow will resolve the room on demand. + #[test] + fn invite_card_state_classifies_room_not_loaded_as_joinable() { + assert_eq!(classify_invite_card_state(None), InviteCardState::Joinable,); + } + + /// Explicit "banned must not collide with not-a-member" sanity + /// check. Mirrors the issue title verbatim so a future refactor + /// that re-merges the two states is caught by a test name match. + #[test] + fn invite_card_state_distinguishes_banned_from_not_a_member() { + let banned = classify_invite_card_state(Some(Err(SendMessageError::UserBanned))); + let not_member = classify_invite_card_state(Some(Err(SendMessageError::UserNotMember))); + assert_ne!( + banned, not_member, + "Banned and UserNotMember must classify to distinct InviteCardStates (#280)" + ); + } + + // ----------------------------------------------------------------- + // Issue freenet/river#283 regression guard: + // + // PR #278's Codex round-1 fix removed a `.read()` on + // `OUTBOUND_SEND_COUNTER` from inside the auto-scroll + // `use_effect`, but didn't replace it with any other subscribed + // read. The captured `message_count: usize` is a plain value, not + // a signal — Dioxus has nothing to watch, so the effect runs once + // on mount and never re-fires. New messages don't scroll the + // viewport. The fix mirrors `conversation.rs`'s last-bubble + // pattern: the last bubble's `onmounted` updates a `Signal