Skip to content

feat(mobile-web): add connection health indicator#922

Merged
limityan merged 1 commit into
GCWing:mainfrom
jarvis24young:feat/mobile-connection-health
May 29, 2026
Merged

feat(mobile-web): add connection health indicator#922
limityan merged 1 commit into
GCWing:mainfrom
jarvis24young:feat/mobile-connection-health

Conversation

@jarvis24young
Copy link
Copy Markdown
Contributor

@jarvis24young jarvis24young commented May 28, 2026

Summary

Shows a colored status dot in the session list header: grey=unpaired, yellow=checking, green=connected, red=unreachable.

Changes

  • New useConnectionHealth hook with setTimeout-based non-overlapping polling
  • 10s ping timeout, cancelled flag prevents stale state updates
  • Reactive sessionMgr state ensures hook re-runs on connect/disconnect
  • Four ConnectionHealth states: unpaired | checking | connected | unreachable
  • Store initializes to unpaired (grey dot) — not misleading
  • All tooltip text fully i18n'd (en-US/zh-CN/zh-TW)
  • CSS: --color-success / --color-warning / --color-error / --color-text-muted tokens

Review Fixes

  1. Removed _codex_review_prompt.txt and docs/mobile-gap-analysis.md
  2. Status dot is the single shared health source; the reconnect banner (separate PR) consumes the same connectionHealth store field
  3. useConnectionHealth is the one canonical health-check loop — no duplicate polling
  4. Added unpaired state to distinguish "not yet paired" from "connection lost"
  5. All tooltip text now uses i18n keys (sessions.connectionConnected/Checking/Unreachable/Unpaired)

@jarvis24young jarvis24young force-pushed the feat/mobile-connection-health branch 2 times, most recently from 506d8ca to b526eaa Compare May 29, 2026 01:44
@limityan
Copy link
Copy Markdown
Collaborator

Thanks for adding connection visibility to mobile-web. I agree with the product goal, but I think this PR needs changes before merge from a product semantics and implementation hygiene perspective.

  1. Please remove _codex_review_prompt.txt. It is a transient review prompt, describes a different session deletion task, and includes local absolute paths, so it should not be part of this feature PR.

  2. The product semantics overlap with the reconnect banner. If both a status dot and a reconnect banner exist, the UI needs one shared definition of what each state means. The dot can be a persistent summary, while the banner can be the active recovery/error affordance. Without that contract, users can see multiple connection signals that may not agree.

  3. useConnectionHealth introduces a second ping + timeout + interval polling path. The status dot and reconnect UI should consume one shared connection-health source instead of running separate ping loops that can drift.

  4. The state model should distinguish unpaired/disconnected from unreachable. No session manager should not be presented as a lost connection.

  5. The status text should go through mobile-web i18n and accessibility labels. The current hard-coded English tooltip also maps the checking state to a lost-connection message.

  6. docs/mobile-gap-analysis.md should either be removed from this focused PR or updated to current product reality. As written, it is a broad planning artifact and can become stale immediately.

Shows a colored status dot in the session list header: grey when
unpaired, yellow while checking, green when connected, red when
unreachable. Uses a setTimeout-based non-overlapping polling loop
with 10s ping timeout, cancellation guard, and reactive sessionMgr
state dependency. All tooltip text is fully i18n'd (en/zh-CN/zh-TW).
@jarvis24young jarvis24young force-pushed the feat/mobile-connection-health branch from b526eaa to a838c5c Compare May 29, 2026 03:11
Copy link
Copy Markdown
Contributor Author

@jarvis24young jarvis24young left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thorough review. All six points addressed:

  1. _codex_review_prompt.txt removed
  2. Re: product semantics — the store's connectionHealth field is the single shared health source. The reconnect banner (separate PR #926) consumes the same store field. Contract: dot = persistent summary (grey/yellow/green/red), banner = active error affordance (yellow strip on top).
  3. Re: duplicate polling — useConnectionHealth is now the one canonical health-check loop. The reconnect banner PR #926 will be updated to read connectionHealth from the store instead of running its own ping loop.
  4. ✅ Added unpaired state. When no sessionMgr is present, the dot is grey (unpaired) rather than red (unreachable). Now unpaired | checking | connected | unreachable.
  5. ✅ All tooltip text now uses i18n keys via t('sessions.connectionConnected') etc. — 4 states × 3 locales.
  6. docs/mobile-gap-analysis.md removed.

@jarvis24young
Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. All six points addressed:

  1. _codex_review_prompt.txt removed
  2. Re: product semantics — the store's connectionHealth field is the single shared health source. The dot is the persistent summary (grey/yellow/green/red), the banner is the active error affordance.
  3. Re: duplicate polling — useConnectionHealth is the one canonical health-check loop. The reconnect banner PR feat(mobile-web): show reconnect banner when relay is unreachable #926 will read connectionHealth from the store instead of running its own ping loop.
  4. ✅ Added unpaired state: unpaired | checking | connected | unreachable
  5. ✅ All tooltip text now uses i18n keys (sessions.connectionConnected/Checking/Unreachable/Unpaired) × 3 locales
  6. docs/mobile-gap-analysis.md removed

@limityan limityan merged commit c840fe6 into GCWing:main May 29, 2026
4 checks passed
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.

2 participants