Sign up
View stargazersSign up

refactor(mobile): the inspector's bar is its pane's AppBar#27205

open1
t3/workspace-split-6
t3/workspace-split-7
opened4d agobyJJulius Marminge
20 files changed
View on GitHubTwo-way sync enabledPR last imported 1h ago

Each inspector pane built its own header. On iOS, the Files and Git panes each mounted a one-screen native stack with a hand-written header config. Android drew separate headers, so feature code branched by platform and passed a header inset down. The iPhone Git sheet had no close button.

Inspector panes now render AppBar like any other screen. The inspector collects the options their AppBar sets, keeping them per pane because hidden panes stay mounted, and draws them:

  • iOS 26: in the native split view's inspector column, using the column's own navigation bar. That bar was already there but hidden. No change to the fork and no new build.
  • iOS JS column (before 26) and the sheet: in a one-screen native stack.
  • Android: AppBar draws inline as before. The scope only keeps the pane's options off the thread's route.

Inside the inspector, AppBar adds a close button that hides the column or dismisses the sheet. FilesHeader, InspectorHeaderInset and the headerInset parameter are gone. The Files and Git panes no longer branch by platform for their bar.

Before (iPhone, from the layer below): the Git sheet's hand-built header has no close button. Before: iPhone Git sheet without a close button

After: After: iPad iOS 27, Files in the native inspector column bar with Close, Refresh and search After: iPad iOS 27, Git in the same column bar; branch title and status subtitle, no leftover search After: Close hides the column After: iPhone Files sheet with its bar from AppBar After: iPhone Git sheet, now with Close

Not checked on a device:

  • Android: the Simlock pool failed to boot an Android tablet twice.
  • iPadOS 18 JS column: the pool lost the iOS 18.6 iPad after it paired. Its sidebar did list threads, which confirms the earlier layer's fix.
  • Review and thread preview inspectors: they host their own native stacks, which are now nested inside the inspector's stack on iOS before 26 and in the sheet.

UIKit draws Close on the inner side of Refresh.

An adversarial review found no option leakage. Its findings are fixed in this layer: the Android Git card, a stale bar when a pane's content goes empty, and the Duo inset for the error fallback.

Part of the workspace split. Stacked on #27107.

Opus 5.5 via Claude Code in T3 Code.

🤖 Generated with Claude Code

Checks

20 successful · 10 skipped · 1 neutral
All checks passed
09d4f6e
MmacroscopeappBot4d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This refactor centralizes inspector headers and changes mobile search and refresh presentation. It adds a lint suppression requiring human review; the supplied Medium finding also identifies Duo search becoming unavailable after header updates.

Needs human approval because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

Ggithub-actionsBot4d ago
Synced from GitHub

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for 5fee9e3.

This comment will update automatically after the next completed run.

CcoderabbitaiBot4d ago
Synced from GitHub

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

  • Last reviewed commit: 169cefa4a1891dbf6f9d6bec8e95953d8bcd0b18
  • Latest commit, not reviewed: 09d4f6ef4575cb8e8e36d7f8fbb758970783c9fd

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between fc5de39356057a25f9f31920e8bdecff19e2b31f and 169cefa4a1891dbf6f9d6bec8e95953d8bcd0b18.

External image Files selected for processing: 3
  • apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx
  • apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx
  • apps/mobile/src/features/threads/ThreadRouteScreen.tsx

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

Walkthrough

The change adds inspector-bar state and close handling across workspace and thread inspector panes. File and Git inspector headers now use shared AppBar layouts. The change removes inspector header-inset parameters and platform-specific header components, and adjusts search placement on supported Duo devices.

Changes: Inspector Bar

Layer / File(s)Summary
Add inspector bar store
apps/mobile/src/features/layout/inspector-bar-store.tsx		The store tracks AppBar options by pane owner, publishes the shown owner’s options, and provides scope and owner adapters.
Connect inspector bars to workspace layouts
apps/mobile/src/features/layout/inspector-bar.android.tsx, apps/mobile/src/features/layout/inspector-bar.tsx, apps/mobile/src/features/layout/workspace-inspector-pane.tsx, apps/mobile/src/features/layout/workspace-inspector-sheet.tsx, apps/mobile/src/features/layout/AdaptiveWorkspaceLayout.tsx, apps/mobile/src/features/layout/createWorkspaceStackNavigator.ios.tsx		Workspace panes and sheets provide inspector-bar scopes with close callbacks. The iOS split layout renders the inspector bar header and connects its close action to hiding the inspector.
Route thread pane options through owners
apps/mobile/src/features/threads/thread-inspector-content-stack.tsx, apps/mobile/src/features/threads/thread-inspector-preview.tsx		Thread inspector panes use their modes as owner IDs and report visibility. The preview content scope provides a null close context.
Use shared AppBar headers
apps/mobile/src/components/ui/AppBar.tsx, apps/mobile/src/components/ui/AppBar.android.tsx, apps/mobile/src/features/files/FilesHeader.tsx, apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx, apps/mobile/src/features/files/thread-file-navigator-pane.tsx, apps/mobile/src/features/threads/ThreadRouteScreen.tsx, apps/mobile/src/features/threads/git/GitOverviewSheet.tsx		AppBar renders a close-inspector action when a handler is available and adjusts search placement on supported Duo devices. File and Git inspector layouts use AppBar and ScreenContent. The change removes FilesHeader and inspector header-inset parameters and wrappers.

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

Change: Refactor

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant InspectorPane
3 participant InspectorBarOwner
4 participant InspectorBarStore
5 participant InspectorBarHeader
6 participant AppBar
7 participant CloseCallback
8 InspectorPane->>InspectorBarOwner: provide pane options and visibility
9 InspectorBarOwner->>InspectorBarStore: set options and select visible owner
10 InspectorBarStore-->>InspectorBarHeader: publish shown owner options
11 InspectorBarHeader->>AppBar: render header options
12 AppBar->>CloseCallback: invoke close handler

Suggested reviewers: maria-rcks, saphid

Merge Risk: External image Minimal · previous review 169ce

The inspector now uses shared headers and close handling. No verified merge-blocking regression remains.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description follows the template's main sections in substance. It gives the problem (each inspector pane built its own header, with platform branching and a missing iPhone Git close button). It ex…Add a Scope and approval section. Link the triaged bug issue or the discussion with explicit maintainer approval of this direction and scope, including the approval comment. If no prior issue exists, explain why the change qualifies as an e…
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title "refactor(mobile): the inspector's bar is its pane's AppBar" is a conventional commit title that names the main change. The changeset makes inspector panes render AppBar, has the inspector c…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "refactor(mobile): the inspector's bar is its pane's AppBar" is a conventional commit title that names the main change. The changeset makes inspector panes render AppBar, has the inspector collect and display each pane's options, adds a Close button, and removes FilesHeader and headerInset. The title is specific and related to the main change, though it uses some internal phrasing.

Full details: Description check

Explanation

The description follows the template's main sections in substance. It gives the problem (each inspector pane built its own header, with platform branching and a missing iPhone Git close button). It explains the change (AppBar in panes, per-pane option collection, platform-specific rendering paths, and removal of FilesHeader and headerInset). It has a Verification section with screenshots and a list of unchecked areas. Gaps: the Scope and approval section is missing. It does not link a triaged issue or maintainer approval, and it does not explain why the change qualifies for an exemption. The verification is partial: Android and iPadOS 18 were not checked on a device. The author also states that the change is stacked on another PR, which is not a substitute for the approval reference.

Resolution

Add a Scope and approval section. Link the triaged bug issue or the discussion with explicit maintainer approval of this direction and scope, including the approval comment. If no prior issue exists, explain why the change qualifies as an exemption. This is a broad refactor, so it likely needs approval. Also state the verification results for the untested paths (Android, iPadOS 18 JS column, review and thread preview inspectors), or explain why they are out of scope for this PR.

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

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
apps/mobile/src/components/ui/AppBar.tsxline 212Outdated · fc5de39Open thread
……@@ -175,6 +198,19 @@ export function AppBar(props: AppBarProps) {
209+ </Toolbar>
210+ ) : null}
211+ {duoBottomSearch ? (
212+ <Toolbar placement="bottom">
MmacroscopeappBoton line 2124d ago

🟡 Medium ui/AppBar.tsx:212

In Duo noncompact-search mode, updating a header option clears the bottom search toolbar, leaving search unavailable while the pane remains mounted. The main NativeStackScreenOptions writes unstable_headerToolbarItems: () => [] while the bottom Toolbar also writes that option; when the header options are reapplied, the toolbar's unchanged signature prevents it from restoring its value. Keep this option to a single writer by not clearing it from the main options when duoBottomSearch is true.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @apps/mobile/src/components/ui/AppBar.tsx around line 212:
2
3In Duo noncompact-search mode, updating a header option clears the bottom search toolbar, leaving search unavailable while the pane remains mounted. The main `NativeStackScreenOptions` writes `unstable_headerToolbarItems: () => []` while the bottom `Toolbar` also writes that option; when the header options are reapplied, the toolbar's unchanged signature prevents it from restoring its value. Keep this option to a single writer by not clearing it from the main options when `duoBottomSearch` is true.

Added 1 commit

  • refactor(mobile): the inspector's bar is its pane's AppBar09d4f6e