Skip to content

fix(ui): show WebSocket connection indicator on no-rooms screen (Bug #5)#273

Merged
sanity merged 3 commits into
mainfrom
fix/connection-indicator-no-rooms-bug-5
May 17, 2026
Merged

fix(ui): show WebSocket connection indicator on no-rooms screen (Bug #5)#273
sanity merged 3 commits into
mainfrom
fix/connection-indicator-no-rooms-bug-5

Conversation

@sanity
Copy link
Copy Markdown
Contributor

@sanity sanity commented May 17, 2026

Problem

A brand-new user accepting their first River invite had zero visual feedback when the WebSocket to their local Freenet node was broken. The connection-status pill lived inside MemberList, which returns empty when no room is selected (CURRENT_ROOM = None). Result: invite-flow and empty-state users couldn't tell their setup was disconnected.

Reported by Ivvor on Matrix (2026-05-17). Ivvor only noticed because Delta surfaces a red indicator; River was silent.

"I had no connection indicator in the bottom right to signify my setup wasn't fully connected to the node. Basically we need [a] websocket connected indicator that works even with no rooms joined."

Approach

Extract the indicator into a small reusable ConnectionStatusIndicator component and render it in RoomList's bottom section — the always-rendered left rail — instead of inside MemberList. One source of truth, always visible regardless of room selection.

  • ui/src/components/members.rs — extracts ConnectionStatusIndicator, removes the inline render from MemberList, adds a data-testid and aria-label so the pill is asserted from tests without depending on Tailwind classes.
  • ui/src/components/room_list.rs — renders ConnectionStatusIndicator between the "Import ID" button and the "Built:" timestamp.

Alternatives considered:

  • Add a parallel indicator to the Welcome screen: more places to keep in sync, two sources of truth.
  • Wrap the entire member panel behind a no-room conditional that still renders the pill: more invasive than necessary; the left rail is structurally the right home for an app-wide status indicator.

Testing

  • New ui/tests/connection-status-indicator.spec.ts pins three invariants for Bug Move rabbit logo to the left, it isn't centered #5:
    • indicator is rendered on initial load,
    • indicator lives inside the Rooms rail, NOT the Members rail (so a future panel-empty regression in MemberList can't reintroduce the bug),
    • indicator remains visible when no room is selected (Welcome screen up).
  • Full cargo make test workspace suite green.
  • Full npx playwright test --project=chromium suite green — 34/34 including the 3 new tests.
  • Manual verification via Playwright MCP on both build-ui-no-sync (empty rooms list) and build-ui-example-no-sync (populated rooms list): pill renders in the bottom of the room rail with the appropriate "Error: WebSocket error" state both before and after a room is selected.

Risk

UI-only change. No delegate or contract WASM touched. ConnectionStatusIndicator reads exactly the same SYNC_STATUS GlobalSignal with the same read() pattern as the previous inline code — visual semantics are byte-identical to the existing indicator.

[AI-assisted - Claude]

sanity and others added 2 commits May 17, 2026 11:42
Problem
-------
A brand-new user accepting their first River invite had zero visual
feedback when the WebSocket to their local Freenet node was broken.
The connection-status pill lived inside `MemberList`, which returns
empty when no room is selected (CURRENT_ROOM = None). Result: invite
flow + empty-state users could not tell their setup was disconnected.

Reported by Ivvor on Matrix (2026-05-17). Ivvor only noticed because
Delta surfaces a red indicator; River was silent.

Approach
--------
Extract the indicator into a small reusable `ConnectionStatusIndicator`
component and render it in `RoomList`'s bottom section (the
always-rendered left rail) instead of in `MemberList`. One source of
truth, always visible, regardless of room selection.

A data-testid and aria-label are added so the indicator can be
asserted from Playwright tests without depending on Tailwind class
shapes.

Testing
-------
- New `ui/tests/connection-status-indicator.spec.ts` pins three
  invariants for Bug #5:
  - indicator is rendered on initial load,
  - indicator lives inside the Rooms rail, not the Members rail,
  - indicator remains visible when no room is selected (Welcome
    screen is up).
- Full `cargo make test` workspace suite green.
- Full `npx playwright test --project=chromium` suite green
  (34/34 including the 3 new tests).
- Manual verification via Playwright MCP on both
  `build-ui-no-sync` (empty rooms list) and
  `build-ui-example-no-sync` (rooms list populated): pill renders in
  the bottom-left of the room rail with the "Error: WebSocket error"
  state both before and after a room is selected.

[AI-assisted - Claude]

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…reen

Round-1 review (skeptical + Codex) flagged that the round-1 fix only
moved the indicator into the always-rendered left rail, but the mobile
viewport hides that rail behind `hidden md:flex` until the user taps
the hamburger to switch `MOBILE_VIEW` to Rooms. The default mobile
view is Chat, so a first-time / invite-flow user with no rooms still
lands on the Welcome screen without any visible WebSocket signal —
exactly Bug #5 on mobile.

Add an inline `md:hidden` copy of `ConnectionStatusIndicator` inside
the Conversation panel's no-room Welcome screen so mobile users see
the same signal at first paint. Desktop is unaffected (the inline copy
is hidden by `md:hidden`; the left-rail copy is the visible one).

Tests
-----
Reorganised `connection-status-indicator.spec.ts` into desktop and
mobile describe blocks:

- Desktop (1280px): the visible pill lives in the Rooms rail, not the
  Members rail, both with a room selected (initial load) and on the
  Welcome screen.
- Mobile (375x812): the visible pill lives inside the Welcome screen
  (not the hidden left rail).

All four tests pass; full chromium suite (35 tests) green.

[AI-assisted - Claude]

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@sanity
Copy link
Copy Markdown
Contributor Author

sanity commented May 17, 2026

Internal four-perspective review + Codex review — round 1

Findings

Source Severity Finding
Skeptical / Codex P2 Round-1 fix only moved the indicator to the left rail, which is hidden on mobile (default `MOBILE_VIEW` is Chat). A mobile user with no rooms lands on the Welcome screen and still doesn't see the indicator until they tap the hamburger.
Code-first none Diff matches stated intent; pure render-side change, no semantic difference vs. pre-fix indicator.
Testing minor Test 3's name said "after deselecting" but no deselection happened — clarified.
Big-picture none No tests removed, no assertions weakened, no anti-patterns.

Resolution (commit c2d9c9f)

  • Added an inline (`md:hidden`) `ConnectionStatusIndicator` to the Conversation panel's no-room Welcome screen so mobile users with no rooms see the signal at first paint. Desktop is unaffected (the inline copy is hidden by `md:hidden`).
  • Reorganised the Playwright tests into desktop (1280px) and mobile (375x812) describe blocks. Both placements now have regression coverage. All 35 chromium tests pass.

[AI-assisted - Claude]

Round-2 Codex review (P2) flagged that the extracted indicator kept
the pre-fix inline code's infallible `SYNC_STATUS.read()` calls. With
the indicator now mounted on the no-room/mobile placements added in
round 1 — i.e. always rendered for first-load users — the synchronizer
writes to SYNC_STATUS can fire subscriber notifications during the
write guard's Drop on Firefox mobile and trigger the documented
`RefCell already borrowed` panic path from AGENTS.md
("Dioxus WASM Signal Safety Rules").

Switch the indicator to `try_read()` and snapshot the status once per
render. If the read fails (another writer holds the RefCell), fall
back to `SynchronizerStatus::Connecting` — the same neutral state the
app boots into — and the next render will pick up the real value.

While here, also collapse the three repeated `SYNC_STATUS.read()`
match arms into a single `let (classes, dot, label) = match ...`
destructure, since they were all reading the same snapshotted value
anyway. Pure refactor of an already-passing visual: the output is
byte-identical to the pre-fix indicator for every status.

Tests
-----
- `cargo make test` (workspace) green.
- `cargo clippy -p river-ui --target wasm32-unknown-unknown --features no-sync`
  clean (only the pre-existing unrelated warnings remain).
- Full chromium Playwright suite (35 tests) green.

[AI-assisted - Claude]

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@sanity sanity merged commit 62699e0 into main May 17, 2026
4 checks passed
@sanity sanity deleted the fix/connection-indicator-no-rooms-bug-5 branch May 17, 2026 17:11
sanity added a commit that referenced this pull request May 17, 2026
…publish

Publishes the merged tip of main containing all 5 bug fixes:

- #263 (#255): legacy delegate cursor overwrite
- #268 (#268, Bug #2): share-invite-via-DM picker hang
- #269 (#269, Bug #1): DM to inactive-but-invited members
- #270 (Bug #3 PR A): room-contract accepts msgs at any known secret version
- #272 (Bug #3 PR B): UI rotate back-fill + PUT join_event + #110 prune exemption
- #273 (#273, Bug #5): WebSocket indicator on no-rooms screen

river-core 0.1.8 published to crates.io (workspace version bump).
riverctl 0.1.58 published to crates.io alongside the UI publish
(addresses Bug #4 — riverctl now matches the deployed room contract WASM).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant