Skip to content

fix(miner-ui): keep mobile chat sheet mounted so conversation state survives (#7792) - #7885

Merged
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
kai392:fix/mobile-chat-preserve-7792
Jul 21, 2026
Merged

fix(miner-ui): keep mobile chat sheet mounted so conversation state survives (#7792)#7885
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
kai392:fix/mobile-chat-preserve-7792

Conversation

@kai392

@kai392 kai392 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Mobile ChatRail uses forceMount so chat state survives sheet close/reopen (matches desktop).
  • SheetContent forwards forceMount to Portal + Overlay + Content (Content-only is insufficient).
  • Regression test: draft text survives mobile close/reopen.

Closes #7792

Test plan

  • vitest run src/chat-rail.test.tsx — 7/7 pass
  • eslint clean on changed miner-ui files (prettier LF)
  • CI validate-code

…urvives (JSONbored#7792)

Pass forceMount through SheetContent to Radix Portal/Overlay/Content so closing
the mobile chat sheet matches desktop's hidden-but-mounted rail behavior.

Closes JSONbored#7792

Co-authored-by: Cursor <cursoragent@cursor.com>
@kai392
kai392 requested a review from JSONbored as a code owner July 21, 2026 15:58
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

…urvives (JSONbored#7792)

Pass forceMount through SheetContent to Radix Portal/Overlay/Content so closing
the mobile chat sheet matches desktop's hidden-but-mounted rail behavior.

Closes JSONbored#7792

Co-authored-by: Cursor <cursoragent@cursor.com>
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-21 16:07:47 UTC

3 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This fixes a real bug: the mobile chat sheet was unmounting `RailBody`/`ChatConversation` on close, losing draft/conversation state, while the desktop docked panel preserved it via `hidden` instead of unmounting. The fix correctly forwards `forceMount` through `SheetPortal`, `SheetOverlay`, and `SheetPrimitive.Content` together (not just Content, which would be a no-op since Radix's Portal Presence gates the whole subtree), and the regression test exercises the actual close/reopen cycle via a stateful mock rather than a fabricated scenario. The change is narrow, well-targeted at #7792, and the reasoning in the code comments matches the underlying Radix Presence behavior.

Nits — 5 non-blocking
  • apps/loopover-miner-ui/src/components/chat-rail.tsx:51 — the `Chat conversation state is lost on mobile every time the chat sheet is closed, contradicting the component's own documented intent #7792` reference in the comment is fine as an issue pointer, not a magic number needing a named constant; no action needed.
  • packages/loopover-ui-kit/src/components/sheet.tsx:78 — since `SheetOverlay` is unconditionally force-mounted whenever any caller passes `forceMount`, verify the overlay's CSS (opacity/pointer-events tied to `data-[state]`) fully hides it when closed so it doesn't intercept clicks or show a stray backdrop while off-screen.
  • The test at chat-rail.test.tsx:106 uses `getByRole('dialog', { hidden: true })` to assert continued presence, which is correct, but consider also asserting `aria-hidden`/`data-state="closed"` on the dialog to guard against the overlay remaining interactive.
  • Consider adding a brief inline test or assertion in sheet.tsx (or a ui-kit level test) that a sheet without `forceMount` still fully unmounts on close, to guard the 'every other sheet' claim in the comment against future regressions.
  • apps/loopover-miner-ui/src/components/chat-rail.tsx docked-panel comment references 'any future in-rail state' — since RailBody now also force-mounts identically for both mobile and desktop, this comment could be tightened to say both paths now share the same never-unmount contract.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7792
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 39 registered-repo PR(s), 24 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor kai392; Gittensor profile; 39 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR forwards forceMount through SheetPortal/SheetOverlay/SheetContent and passes it from chat-rail's mobile Sheet, directly implementing the issue's recommended fix so RailBody stays mounted across close/reopen; it also adds the requested regression test asserting draft state survives a mobile sheet close/reopen cycle.

Review context
  • Author: kai392
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, Kotlin, Perl, TypeScript, Vue
  • Official Gittensor activity: 39 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before /
before /
after /
after /
/ mobile before / (mobile)
before / (mobile)
after / (mobile)
after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

Scroll preview
Route Before (production) After (this PR's preview)
/ before / (scroll)
before / (scroll)
after / (scroll)
after / (scroll)

A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit e7e10e7 into JSONbored:main Jul 21, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chat conversation state is lost on mobile every time the chat sheet is closed, contradicting the component's own documented intent

2 participants