Skip to content

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

Closed
kai392 wants to merge 1 commit into
JSONbored:mainfrom
kai392:fix/chat-rail-mobile-state-7792
Closed

fix(miner-ui): keep mobile chat sheet mounted so conversation state survives (#7792)#7879
kai392 wants to merge 1 commit into
JSONbored:mainfrom
kai392:fix/chat-rail-mobile-state-7792

Conversation

@kai392

@kai392 kai392 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Mobile ChatRail passes forceMount so RailBody / chat state survives sheet close/reopen (matches desktop hidden contract).
  • SheetContent forwards forceMount to Radix Portal + Overlay + Content (Content-only was a no-op once Portal unmounted).
  • Regression test: typed draft survives a mobile close/reopen cycle.

Closes #7792

Test plan

  • vitest run src/chat-rail.test.tsx in apps/loopover-miner-ui — 7/7 pass (after ui-kit build)
  • CI validate-code (miner-ui + ui-kit)
  • One-shot PR — will not push follow-up commits

@superagent-security

Copy link
Copy Markdown
Contributor

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

@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

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-21 15:57:05 UTC

3 files · 1 AI reviewer · no blockers · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
Fixes #7792 by forwarding forceMount through SheetPortal/Overlay/Content (the actual mechanism needed since Radix's Portal-level Presence was unmounting the subtree before Content's own forceMount had any effect) and wires it in from the mobile ChatRail sheet, matching the desktop `hidden` pattern. The fix targets the right layer (Portal, not just Content), defaults forceMount to undefined so all other sheet consumers keep prior unmount-on-close behavior, and the new test drives the real code path (renders ChatRail, types into a stubbed stateful conversation component, closes/reopens, asserts state survives) rather than fabricating an unreachable scenario. The validate/validate-code CI failures are undetailed and this branch is one commit behind default, which is a plausible innocuous cause per the given base-branch status.

Nits — 5 non-blocking
  • The `Chat conversation state is lost on mobile every time the chat sheet is closed, contradicting the component's own documented intent #7792` references in comments (chat-rail.tsx:51, sheet.tsx:64) are just issue-number annotations, not magic numbers needing a named constant — the external brief's suggestion here doesn't apply to a comment.
  • chat-rail.test.tsx mocks `./components/chat/conversation` with a stateful stub; consider adding a one-line comment noting this stub intentionally diverges from the real ChatConversation's props/behavior so future readers don't assume parity.
  • No regression test added for the default (non-mobile, non-forceMount) SheetContent usages to confirm forwarding `forceMount={undefined}` doesn't change existing unmount-on-close behavior, though this is low-risk given the explicit default handling.
  • Consider a brief inline comment in sheet.tsx on why Portal-level forceMount is required (Radix Presence unmounts at the Portal boundary) since this is a non-obvious Radix internal detail worth preserving for future maintainers.
  • If other SheetContent call sites exist elsewhere in the app, a quick grep to confirm none of them rely on Content unmounting-on-close for cleanup side effects (e.g., resetting internal state) would be worth a sentence in the PR description.

CI checks failing

  • validate
  • validate-code

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 diff forwards forceMount through SheetPortal/SheetOverlay/SheetContent and passes it from the mobile ChatRail sheet, directly fixing the unmount-on-close root cause identified in the issue, and adds the requested regression test in chat-rail.test.tsx verifying draft state survives a mobile 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 <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> 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

…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 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-code)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 21, 2026
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