Skip to content

fix(mobile): center Android bottom sheets across the workspace - #18492

Open
PixPMusic wants to merge 1 commit into
pingdotgg:mainfrom
PixPMusic:pixpmusic/fix-android-workspace-sheet-presentation
Open

PixPMusic wants to merge 1 commit into
pingdotgg:mainfrom
PixPMusic:pixpmusic/fix-android-workspace-sheet-presentation

Conversation

@PixPMusic

@PixPMusic PixPMusic commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Problem

On unfolded Android, routed bottom sheets are presented by the navigator inside the detail pane. They sit to the right, and their backdrop leaves the workspace rail, list, and header interactive. Review, Add comment, Git, and Add environment share this presentation defect.

Change

Put Android's existing native presentation stack around the entire workspace. Keep workspace pages inside its base screen and render the original sheet descriptors above it, preserving native fit-to-content measurement, Material surfaces and corners, detents, keyboard handling, and dismissal controls.

Apply one native width policy to routed sheets and the FormSheet used by AppSheet: full width below 600dp; centered with 16dp side gutters and a 640dp cap in wide windows. Sheets remain bottom-anchored. Settings, new thread, and the environment list retain their Android page presentation. Existing full-screen inline/comment composer exceptions remain in place. The iOS navigator and UIKit presentation are unchanged.

Scope and approval

This is a focused fix for an obvious failure of existing modal presentation: the sheet's pane-sized host allows interaction with the workspace underneath it. The fix lives at the shared navigator/native-sheet boundaries, with no per-PR overlays, new operations, or wire changes. It qualifies for the focused bug-fix exception in CONTRIBUTING.md.

Based on fetched main; merged current main abc80c80a7 to resolve the native screens dependency upgrade conflict. The contrast and selection fixes in #18479 and #18480 merged during verification and are preserved as upstream base changes. The local Usage/Settings work is excluded.

Verification

  • After rebasing: vp test run apps/mobile/src/native/workspace-stack-projection.test.ts apps/mobile/src/lib/android-workspace-navigation.test.ts apps/mobile/src/lib/adaptive-navigation.test.ts apps/mobile/src/features/pull-requests/pullRequestSelection.test.ts: 67 passed. Coverage includes real StackRouter Back/dismiss history and preservation of card routes and nested sheet pages.

  • Native :react-native-screens:testDebugUnitTest --tests '*FormSheetWidthTest': 3 passed; patched Kotlin compiled. Tests cover compact/wide boundaries, density, gutters, and repeated fold-width changes. Mobile typecheck, scoped formatting, and diff checks pass. Scoped lint has no errors and three existing AdaptiveWorkspaceLayout warnings.

  • Android 17 foldable emulator: commit and review sheets remain bottom-anchored and centered across the unfolded screen. Taps over the rail, another list row, and Settings header dismiss the backdrop without triggering those controls. Compact commit sizing and system Back to the underlying thread work. An unsent commit draft survives folding; real keyboard lift and keyboard Back were checked. Add comment, Add environment, and Terminal output (AppSheet) use the shared presentation. New thread and Settings remain pages. No commit, review, comment, or agent turn was submitted.

  • State was seeded with seedShowcaseEnvironment, synthetic environment-label probes, a showcase shell, and synthetic provider-data homes. PR-flow evidence reads public PR data. All uploaded screenshots were reviewed; no real userdata database, private thread content, or actual hostname was used.

  • Earlier follow-up on f31b87e: removed the comparison client, rebuilt/installed from this worktree, and repeated the unfolded commit/review and compact fold/Back checks against the same synthetic environment. Bottom anchoring, full-screen centering, full-workspace dimming, and blocked rail/header taps pass after the rebase. Supplemental reviewed captures: unfolded commit, folded commit, unfolded review. No source-control operation, review/comment submission, or agent turn was triggered.

  • Current-head verification on 5020e62050: carried the unchanged Android patch to upstream screens t3.27 and regenerated the lockfile. The upstream tarball has no Android source changes. Rebuilt/installed the native client; 67 focused tests, mobile typecheck, and all 3 native width tests pass. Repeated unfolded commit/review, blocked rail/header taps, and folded commit/system Back against synthetic state. Reviewed captures: unfolded commit, folded commit, unfolded review. No commit, review, comment, or agent turn was submitted.

Gaps: iOS impacts were inspected in source and existing projection tests; no iOS runtime pass was performed. Inline composers, every remaining routed sheet, landscape, and relay/tunnel connections were not individually exercised. Native Kotlin formatting follows the fork's style; its packaged dependency has no native formatter task. Web, desktop, server, provider behavior, and contracts are unaffected.

The matched pairs below retain the original pinned main c32ade4139 baseline and its sheet fix. The supplementary final-head captures above include the merged contrast and selection changes.

Matched pairs use the same emulator, theme, orientation, and route state. Unfolded: 2076 × 2152; folded: 1080 × 2364. Relative timestamps and the public PR list can update between captures.

Surface Before After
Unfolded commit Commit sheet anchored in the detail pane with undimmed left workspace Bottom-anchored commit sheet centered across the dimmed workspace
Unfolded review Review sheet confined to the right pane Bottom-anchored review sheet centered across the entire workspace
Folded commit Original full-width compact commit bottom sheet Compact commit bottom sheet retaining its full-width presentation

Model: GPT-6.1 Sol | Harness: Codex in T3 Code

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 12, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 12, 2026 — with ChatGPT Codex Connector
@PixPMusic
PixPMusic marked this pull request as ready for review October 12, 2026 04:13
@macroscopeapp

macroscopeapp Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This change alters the default Android presentation and sizing of multiple existing bottom sheets through a new navigator layer and a patched native dependency. Its cross-layer runtime impact is broader than a small isolated UI fix.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1ae23416-dc63-45d6-ab5f-1e39582527dc
📥 Commits

Reviewing files that changed from the base of the PR and between 5020e62 and e206621.

⛔ Files ignored due to path filters: 1
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing: 4
  • apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx
  • apps/mobile/src/native/workspace-stack-projection.test.ts
  • apps/mobile/src/native/workspace-stack-projection.ts
  • pnpm-workspace.yaml

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The Android workspace navigator separates workspace routes from presentation overlays and renders workspace routes through the adaptive column layout. The Android react-native-screens patch adjusts form-sheet widths based on measured window sizes.

Changes: Android presentation

Layer / File(s) Summary
Route projection and behavior tests
apps/mobile/src/native/workspace-stack-projection.ts, apps/mobile/src/native/workspace-stack-projection.test.ts
projectStackPresentation splits routes at the first overlay. Tests cover review-sheet routes, back navigation, grouped review pages, and cold-linked histories.
Workspace rendering and navigator integration
apps/mobile/src/features/layout/workspace-presentation-context.tsx, apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx, apps/mobile/src/features/layout/createWorkspaceStackNavigator.android.tsx
The adaptive layout provides its column renderer when a presentation host is present. The Android navigator renders workspace routes through that renderer and presents overlay routes in an outer stack.
Android form-sheet sizing
patches/react-native-screens@5.0.0-t3.27.patch, pnpm-workspace.yaml
The patch sizes form sheets using measured window width, centers them, and tests compact, wide, and changing widths. The workspace configuration applies the patch.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant AdaptiveWorkspaceLayoutContent
  participant WorkspaceStackNavigator
  participant WorkspaceStackView
  participant projectStackPresentation
  participant WorkspaceColumnsRendererContext
  AdaptiveWorkspaceLayoutContent->>WorkspaceColumnsRendererContext: provide renderColumns when hosted
  WorkspaceStackNavigator->>WorkspaceStackView: render navigation state
  WorkspaceStackView->>projectStackPresentation: split routes at first overlay
  projectStackPresentation-->>WorkspaceStackView: return workspace routes and overlays
  WorkspaceStackView->>WorkspaceColumnsRendererContext: render workspace routes
  WorkspaceStackView->>WorkspaceStackView: render overlays in outer stack
Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to e2066

The Android sheet changes appear mergeable after normal checks; no unresolved issue was established that should block merge.

Security architecture risk: 🔵 Low · up to e2066

The change is confined to Android navigation and sheet presentation. The inspected code preserves existing route identities and dismissal controls, with no demonstrated expansion of access or privileges. Some rapid-transition and nested-dismissal behavior remains unverified on Android.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The supported exposure is shared Android presentation behavior: workspace-wide routed overlays and Android form sheets using either patched native implementation. A regression could therefore affect multiple screens, but the inspected changes do not grant new service, tenant, credential, or data-store authority.

Trust Boundaries and Controls

  • observed — The synthetic workspace route is a rendering container, not a replacement identity for overlay actions. Production dismissal uses the real descriptor route key and current owning router key; Android legacy dismissal emits a count of one.

Resilience and Maintainability Implications

  • observed — Tests cover helper-level identity and ordering, ordinary back navigation, grouped dismissal, and cold-linked history. These tests do not render the new Android presentation envelope, so they do not prove native grouped-dismissal, interruption, or rapid-repeat behavior.
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: centering Android bottom sheets across the workspace.
Description check Passed The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the affected behavior, implementation, approval exception, tests, manual checks, scree…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the affected behavior, implementation, approval exception, tests, manual checks, screenshots, and known gaps.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@PixPMusic
PixPMusic force-pushed the pixpmusic/fix-android-workspace-sheet-presentation branch from 5020e62 to e206621 Compare October 17, 2026 05:33

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants