Skip to content

iOS: reorderable built-in toolbar items (Ctrl/Option/Cmd/Esc/Tab) in toolbar settings - #5579

Merged
lawrencecchen merged 3 commits into
mainfrom
feat-ios-toolbar-reorder-builtins
Jun 8, 2026
Merged

lawrencecchen merged 3 commits into
mainfrom
feat-ios-toolbar-reorder-builtins

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jun 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The terminal accessory bar's modifier keys (⌃ ⌥ ⌘), zoom controls, and paste were structurally pinned outside the configurable region, so only the insertable shortcuts and custom actions could be reordered or hidden in toolbar settings. This folds those built-ins into the same reorderable region, so every bar button can be dragged to any position or hidden, alongside custom actions. (⇧ stays off the bar as before, and the trailing "customize" control is a fixed affordance.)

Liquid Glass styling (#5536) applies to every item regardless of position.

Stacking

Stacks on #5536 (feat-ios-accessory-glass). Review/merge sequence: #5536 → this. The PR base is set to feat-ios-accessory-glass so the diff is only this change. The "Esc right of Tab" default already landed on the base; this PR keeps it (and only adds the leading/trailing built-ins around it).

How the data-driven toolbar + persistence works

PR #5510 made the bar data-driven: a unified ToolbarItemID (.builtin(rawValue) | .custom(uuid)), a pure TerminalAccessoryLayoutReducer (order/enable/move over opaque ids), and TerminalAccessoryConfiguration (@Observable, UserDefaults-backed, posts didChangeNotification so the UIKit bar rebuilds live). The settings list (TerminalShortcutsSettingsView) already lists displayItems and reorders via SwiftUI onMove. The only thing keeping modifiers/zoom/paste out was the isUserConfigurable predicate plus fixed pinnedLeadingActions/pinnedTrailingActions lists in the UIKit builder.

Model + migration

  • Widen isUserConfigurable to include every action except shift. Add defaultLeadingActions (⌃ ⌥ ⌘, paste) and defaultTrailingActions (zoom), and rebuild defaultConfigurableOrder so a fresh install renders modifiers/paste leading, zoom trailing.
  • Bump the persisted schema to v3 and add ToolbarLayoutMigration.widenedToV3 (pure, in CmuxMobileTerminalKit). On a v1/v2 → v3 upgrade it force-enables the now-configurable built-ins and inserts them at their old fixed positions, so an upgrading user's bar looks unchanged. Force-enable is confined to the upgrade boundary: a v3 config's enabled set is authoritative, so hiding a modifier then persists. Init order: v3 (load as-is) → v2 (widen) → v1 (relabel + widen) → fresh.

UIKit bar

  • populateAccessoryActions renders the configurable region inline (no pinned lists). ⌘ stays in the saved order but only renders for a Mac remote; updateModifierLabels repopulates on the flip.
  • After every rebuild, reconcileArmedModifierVisibility disarms any armed/sticky modifier whose button is no longer on the bar, so hiding an active modifier can't leave it invisibly modifying every keystroke.

Tests

  • CmuxMobileTerminalKit (swift test, 77 pass): v3 widening transform — inserts/force-enables forced built-ins, preserves a partially-hidden shortcut set, handles nil enabled, no duplicate when a forced id is already present, a custom action keeps its slot while modifiers fold in around it, and round-trips through the reducer.
  • cmuxFeatureTests (simulator, CI): live TerminalAccessoryConfiguration — fresh-install default order (modifiers front, zoom back, Esc after Tab, all shown), reorder + hide round-trips across reload, v2 → v3 migration with a partial enabled subset, migration of a config carrying a custom action (the custom keeps its slot while modifiers/zoom fold in), and that the upgraded config re-persists under v3 keys so hiding a modifier survives.

Verification

iOS simulator app builds clean (ios/scripts/reload.sh --tag tbarord). Kit tests pass locally; cmuxFeatureTests run in CI (local iOS test runs are disallowed).

Localization

Footer (mobile.shortcuts.footer) updated en+ja. New settings labels for the modifiers use terminal.shortcut.name.control/.alternate/.command (added en+ja); zoom reuses the existing localized terminal.input_accessory.zoom_in/zoom_out; paste reuses terminal.input_accessory.paste. All resolve via Bundle.main (the app catalog), matching the package's existing String(localized:) pattern.

🤖 Generated with Claude Code


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Note

Medium Risk
Touches persisted toolbar layout (v3 migration), live UIKit bar rebuild, and armed-modifier state when users hide keys—user-facing input behavior with migration edge cases covered by tests.

Overview
Makes every iOS terminal keyboard-bar button reorderable and hideable, including ⌃ ⌥ ⌘, paste, and zoom—not only insertable shortcuts and custom actions. The settings list and persisted layout now treat the whole bar as one region (⇧ and the trailing customize control stay fixed).

Persistence moves to UserDefaults v3 with ToolbarLayoutMigration.widenedToV3: upgrading from v1/v2 force-enables and inserts previously pinned leading/trailing built-ins so the on-screen bar matches pre-upgrade, then v3 keys make hide/reorder authoritative.

UIKit bar drops separate pinned leading/trailing stacks; it renders TerminalAccessoryConfiguration.enabledItems in user order (⌘ still omitted unless Mac remote). reconcileArmedModifierVisibility disarms modifiers whose buttons are hidden or not rendered so sticky modifiers cannot affect typing invisibly. Accessory button styling is centralized via UIButton.Configuration (including Liquid Glass on iOS 26).

Tests and copy: v3 migration and configuration behavior tests; settings footer and modifier labels localized (en/ja).

Reviewed by Cursor Bugbot for commit 1ac9735. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Make every iOS terminal accessory bar button reorderable and hideable, including ⌃ ⌥ ⌘, Esc/Tab, Paste, and Zoom. Also adopts real iOS 26 Liquid Glass styling for buttons, with a solid-fill fallback on earlier iOS.

  • New Features

    • Built-ins (⌃ ⌥ ⌘, Paste, Zoom) are configurable alongside shortcuts and custom actions; ⇧ stays off the bar.
    • Default layout: modifiers/paste leading, Esc right of Tab, zoom trailing. ⌘ stays in the saved order but only renders for Mac remotes; titles update on flip.
    • Liquid Glass on iOS 26 via UIButton.Configuration (.glass resting, .prominentGlass armed); flat gray/blue fallback on older iOS. Single styling path and fixed widths for glyph/icon buttons to keep them uniform.
    • Hidden modifiers are auto-disarmed so they can’t stay active without a visible button. The whole region renders inline; the trailing “customize” control is fixed.
  • Migration

    • Schema bumped to v3 in CmuxMobileTerminal/CmuxMobileTerminalKit.
    • Upgrading from v1/v2 force-enables and inserts the now-configurable built-ins at their old leading/trailing spots (no duplicates), so the bar looks the same.
    • v3 keys persist the widened layout so the migration runs once; after that, the enabled set is authoritative and hiding a modifier persists.

Written for commit 1ac9735. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Terminal shortcut buttons for modifier keys (Control, Option, Command), zoom controls, and paste are now customizable—they can be reordered or hidden from the toolbar.
  • Changes

    • Default toolbar layout adjusted; Escape now appears immediately after Tab.
  • Improvements

    • Enhanced handling of modifier key visibility when switching between connection types.

@coderabbitai

coderabbitai Bot commented Jun 7, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR expands toolbar action configurability to include modifiers and zoom, introduces a v3 storage schema with automated v2→v3 migration, refactors accessory bar population to render user-configured items in order, unifies button styling, and adds comprehensive migration and configuration tests.

Changes

Terminal Accessory Toolbar Configurability and v3 Schema Migration

Layer / File(s) Summary
Action configurability and display contracts
Packages/CmuxMobileTerminal/Sources/.../GhosttySurfaceView.swift, Packages/CmuxMobileShellUI/Sources/.../TerminalShortcutsSettingsView.swift, ios/cmux/Resources/Localizable.xcstrings
Only .shift is non-configurable; control, option, command, zoom, and paste actions are now user-configurable with explicit localized display names. Updated UI documentation and localized strings clarify that modifiers, zoom, and paste can be moved or hidden.
Toolbar layout v3 schema and migration
Packages/CmuxMobileTerminal/Sources/.../TerminalAccessoryConfiguration.swift, Packages/CmuxMobileTerminalKit/Sources/.../ToolbarLayoutMigration.swift
Introduces v3 storage keys (cmux.terminal.toolbar.order.v3, .enabled.v3); loads v3 directly when present, otherwise migrates from v2 (widening with forced leading/trailing modifiers/zoom) or v1 (relabel then widen). New WidenedLayout struct and widenedToV3(...) helper deduplicate forced items, prepend/append them at prior positions, and force-enable newly configurable actions. Immediate re-persistence ensures one-time migration.
Accessory bar population and unified button styling
Packages/CmuxMobileTerminal/Sources/.../TerminalInputTextView.swift
Bar now renders enabledItems in user-configured order via populateAccessoryActions(), conditionally omitting .command when not driving Mac remote. Unified styling pipeline—applyAccessoryButtonStyle() and accessoryButtonConfiguration()—centralizes iOS 26 glass/prominent-glass and pre-iOS 26 flat styling for built-in/custom buttons with armed/sticky states. updateModifierLabels() triggers full repopulate; refreshAccessoryButtonStyles() delegates to unified styling.
Migration v3 and configuration tests
Packages/CmuxMobileTerminalKit/Tests/.../ToolbarCustomizationTests.swift, ios/cmuxPackage/Tests/.../TerminalAccessoryConfigurationTests.swift
ToolbarLayoutMigrationV3Tests validates witenedToV3() forced-item insertion, deduplication, and force-enable behavior with hidden-item preservation. TerminalAccessoryConfigurationTests asserts fresh-install default ordering, persistence of reorder and hidden state, v2→v3 widening migration with forced modifiers/zoom, and custom action slot preservation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • manaflow-ai/cmux#5510: The main PR's v3 toolbar layout migration and accessory-bar refactor build directly on this PR's ToolbarItemID/custom-actions architecture, extending the same components' ordering/enabled persistence and button population logic.
  • manaflow-ai/cmux#5532: Both PRs reshape the iOS terminal input-accessory toolbar's default action layout by introducing curated default ordering and updating pinned regions (e.g., modifiers/zoom placement), with the main PR extending this via v3 migration and updated action configurability/display names.

Poem

🐰 The toolbar now bends to your will,
Modifiers once locked are configurable still,
A v3 schema glides from v2 with grace,
While buttons paint themselves in their finest place,
Hide, reorder, rearrange with delight!


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (3 errors, 1 warning)

Check name Status Explanation Resolution
Cmux Algorithmic Complexity ❌ Error TerminalAccessoryConfiguration.enabledItems calls resolve() which does customActions.first() scan for each custom ID, creating O(n²) nested scans on scalable customActions in the hot UI path. Use a dictionary keyed by UUID or cached lookup to replace the O(n) first(where:) scan in resolve() for custom actions.
Cmux Swift Logging ❌ Error GhosttySurfaceView.swift line 9: Logger must be nonisolated private let in MainActor-strict Swift 6 code. Change private let log = to nonisolated private let log = on line 9.
Cmux Architecture Rethink ❌ Error Code drops Shift button from UI then patches symptoms with reconcileArmedModifierVisibility() to silently disarm armed Shift state. Shift should remain fixed-rendered per PR objectives. Render Shift button as fixed control in populateAccessoryActions() independent of configuration, remove symptom-patch reconciliation once rendering is restored.
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (15 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: making built-in toolbar items (Ctrl/Option/Cmd/Esc/Tab) reorderable in the toolbar settings UI.
Description check ✅ Passed The description covers all template sections with substantial detail: comprehensive summary of changes, detailed testing approach, and verification steps. All required checklist items are addressed.
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.
Cmux Swift Actor Isolation ✅ Passed TerminalAccessoryConfiguration is properly @MainActor @Observable; all new data models are Sendable value types; UIView subclasses used appropriately; no implicit MainActor issues.
Cmux Swift Blocking Runtime ✅ Passed NSLock serializes libghostty callbacks; DispatchSemaphore is DEV-only in visibleTerminalSnapshot(). Both have explicit lint:allow comments with documented carve-outs.
Cmux No Hacky Sleeps ✅ Passed Check scope is limited to TypeScript/JavaScript/shell/build scripts; all PR changes are Swift files (.swift) and localization strings, which are explicitly excluded from this check.
Cmux Swift Concurrency ✅ Passed PR introduces no new legacy async patterns; all changes are data model and UI layout updates without DispatchQueue, Combine, fire-and-forget Tasks, or completion handlers.
Cmux Swift @Concurrent ✅ Passed No new async functions, @concurrent annotations, or heavy async operations in PR changes; all new functions are synchronous and UI-safe.
Cmux Swift File And Package Boundaries ✅ Passed GhosttySurfaceView adds +46 lines (under 250-line threshold for files >800); TerminalInputTextView +42 net; ToolbarLayoutMigration appropriately in Kit package.
Cmux User-Facing Error Privacy ✅ Passed PR adds only safe UI text and standard keyboard key names with no exposed vendor names, credentials, tokens, APIs, databases, or sensitive information.
Cmux Full Internationalization ✅ Passed All user-facing text uses localized APIs with complete translations for all locales: control/alternate/command keys, footer, and customize button all have English and Japanese entries.
Cmux Swiftui State Layout ✅ Passed PR uses modern @Observable pattern in TerminalAccessoryConfiguration; no new @Published/@StateObject patterns, no problematic GeometryReader, ForEach receives value types, no render-time mutations.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR modifies iOS terminal toolbar UI elements only; no NSWindow, NSPanel, NSWindowController, or SwiftUI Window/WindowGroup code added or changed.
Cmux Source Artifacts ✅ Passed All 8 changed files are intentional hand-written source code, test code, or localization resources with no generated file markers; no artifacts, build output, caches, or temp directories were added.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-ios-toolbar-reorder-builtins

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@lawrencecchen
lawrencecchen force-pushed the feat-ios-toolbar-reorder-builtins branch from aa39bf7 to 19f5ee7 Compare June 7, 2026 20:56
@vercel

vercel Bot commented Jun 7, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jun 8, 2026 2:54am

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19f5ee766d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +109 to +110
for id in newLeading + newTrailing where seen.insert(id).inserted {
enabledSet.append(id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Force-enable already-present pinned IDs

When a legacy v1/v2 order already contains one of the IDs being widened (the helper explicitly preserves that case), this loop only appends newLeading + newTrailing, so a forced built-in that was present in order but omitted from enabled remains hidden after migration. That violates the v3 upgrade contract that previously pinned controls are force-shown and can make an upgrading toolbar drop a modifier/zoom/paste item for partially migrated or beta defaults; iterate over all leading + trailing IDs when force-enabling, while still avoiding duplicates.

Useful? React with 👍 / 👎.

@greptile-apps

greptile-apps Bot commented Jun 7, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR folds the previously pinned modifier keys (⌃ ⌥ ⌘), paste, and zoom controls into the unified reorderable toolbar region, so every iOS terminal bar button can be dragged, hidden, or reordered in settings alongside shortcuts and custom actions. A v3 UserDefaults schema and ToolbarLayoutMigration.widenedToV3 handle upgrade from v1/v2 by force-enabling those built-ins at their old fixed positions, leaving the bar visually unchanged for upgrading users.

  • isUserConfigurable widened to all actions except .shift; defaultLeadingActions/defaultTrailingActions express the new default layout, and TerminalInputTextView renders enabledItems in saved order (⌘ skipped for non-Mac remotes) with no separate pinned stacks.
  • reconcileArmedModifierVisibility is called after every bar rebuild to disarm any armed modifier whose button is no longer rendered — but accessoryAction(for: .shift) returns .shift instead of nil, causing shift to be silently disarmed on every rebuild while it is active (see inline comment).
  • v3 migration is pure, identifier-agnostic, handles nil enabled sets and already-present forced ids without duplication; en+ja localization updated for all new settings labels.

Confidence Score: 4/5

Safe to merge after fixing one defect in the new reconcile helper: shift armed via hardware keyboard is silently cleared on every bar rebuild.

The migration logic, UIKit builder refactor, and localization changes are all well-structured and well-tested. The one concrete defect is in accessoryAction(for:): returning .shift instead of nil means that whenever shift is armed, every call to populateAccessoryActions() walks through reconcileArmedModifierVisibility, finds no .shift button on the bar (shift is intentionally never a bar button), and calls modifierState.disarmAll() — silently clearing the modifier mid-session. The fix is a one-liner; everything else in the PR is sound.

Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift — specifically the accessoryAction(for:) helper and its .shift return value.

Important Files Changed

Filename Overview
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift Drops pinned leading/trailing stacks; renders configurable region inline. accessoryAction(for: .shift) returns .shift instead of nil, causing reconcileArmedModifierVisibility to disarm shift on every bar rebuild.
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift Bumps schema to v3, adds v2→v3 migration path alongside existing v1 path; migration key constants renamed clearly; init now public for test injection.
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift Widens isUserConfigurable to all actions except .shift; adds defaultLeadingActions/defaultTrailingActions; expands settingsDisplayName for control/alternate/command/zoom cases.
Packages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/ToolbarLayoutMigration.swift Adds widenedToV3 migration transform: pure, identifier-agnostic, handles nil enabled, no-duplicate guard for already-present forced ids.
ios/cmux/Resources/Localizable.xcstrings Adds terminal.shortcut.name.control/alternate/command with en+ja translations; updates mobile.shortcuts.footer for both locales.
Packages/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/ToolbarCustomizationTests.swift New v3 widening test suite: covers force-enable/insert, partial hidden set, nil enabled, no-duplicate, reducer round-trip, and custom action in-slot migration.
ios/cmuxPackage/Tests/cmuxFeatureTests/TerminalAccessoryConfigurationTests.swift New integration test suite for TerminalAccessoryConfiguration: fresh-install defaults, reorder/hide round-trips, v2→v3 migration with custom actions and hidden shortcuts, and migration-once guarantee.
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift Updates doc comment and footer string to reflect that modifiers/zoom/paste are now reorderable alongside shortcuts.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[App Launch / TerminalAccessoryConfiguration.init] --> B{v3 keys present?}
    B -- Yes --> C[Load as-is, v3 enabled set is authoritative]
    B -- No --> D{v2 keys present?}
    D -- Yes --> E[widenedToV3: prepend leading, append trailing, force-enable]
    D -- No --> F{v1 keys present?}
    F -- Yes --> G[migratedOrder/migratedEnabled then widenedToV3]
    F -- No --> H[Fresh install: empty saved → defaultConfigurableOrder]
    C --> I[reducer.load → displayOrder + enabledSet]
    E --> I
    G --> I
    H --> I
    I --> J[persist under v3 keys — migration runs once]
    J --> K[TerminalInputTextView.populateAccessoryActions]
    K --> L[enabledItems in saved order]
    L --> M{action == .command?}
    M -- not isMacRemote --> N[skip — stays in saved order unrendered]
    M -- isMacRemote --> O[addArrangedSubview]
    L --> O
    K --> P[reconcileArmedModifierVisibility]
    P --> Q{armedModifier present AND accessoryAction returns non-nil?}
    Q -- button not in renderedActions --> R[disarmAll + refreshStyles]
    Q -- button rendered or nil --> S[no-op]
Loading

Reviews (2): Last reviewed commit: "iOS: reorderable built-in toolbar items ..." | Re-trigger Greptile

lawrencecchen added a commit that referenced this pull request Jun 8, 2026
…oolbar items) into dog bundle

# Conflicts:
#	Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
#	Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift
lawrencecchen and others added 3 commits June 7, 2026 19:49
Render the input accessory bar buttons (modifiers ⌃⌥⌘⇧, Esc/Tab/arrows/
$/@//, zoom, agent launchers) with real iOS 26 Liquid Glass via
UIButton.Configuration .glass() (resting) and .prominentGlass() (armed/sticky),
keeping the prior flat gray/blue fill as the < iOS 26 fallback. Button styling
and the armed/sticky restyle now flow through one configuration builder.

Also pin single-glyph modifiers (⌃⌥⌘⇧) and icon buttons (zoom) to a fixed
width so they stay uniform: their glyph metrics differ and the glass capsule
under a greater-than-or-equal min-width let Ctrl/Option grow wider than Cmd.
Variable-text buttons (Claude/Codex/etc.) still size to content.

Independent of the iOS composer work (PR #5511); this improves the existing
terminal accessory bar and can land on its own.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The data-driven terminal accessory bar's default arrangement put Esc seven
positions after Tab. Move Esc so the two most common terminal keys sit
adjacent (Tab, Esc, ^C, ...). Only changes the first-launch / Restore Default
order; a device with a saved v2 order keeps its arrangement until reset.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…toolbar settings

The terminal accessory bar's modifier keys (⌃ ⌥ ⌘), zoom controls, and paste
were structurally pinned outside the configurable region, so only the insertable
shortcuts and custom actions could be reordered/hidden. Fold those built-ins into
the same configurable region so every bar button (except ⇧, which is never
surfaced, and the fixed "customize" control) can be dragged to any position or
hidden in toolbar settings.

- Widen `TerminalInputAccessoryAction.isUserConfigurable` to include modifiers
  (minus shift), zoom, and paste; add `defaultLeadingActions`/`defaultTrailingActions`
  and rebuild `defaultConfigurableOrder` so a fresh install shows modifiers/paste
  leading, zoom trailing, and Esc right after Tab.
- Bump the persisted schema to v3 and add `ToolbarLayoutMigration.widenedToV3`
  (pure, in CmuxMobileTerminalKit): v1/v2 → v3 force-enables the now-configurable
  built-ins and inserts them at their old fixed positions so an upgrading user's
  bar looks unchanged. Force-enable is confined to the upgrade boundary; a v3
  config's enabled set is authoritative, so hiding a modifier persists.
- Render the configurable region inline in `populateAccessoryActions` (no more
  pinned leading/trailing lists). ⌘ stays in the saved order but only renders for
  a Mac remote; `updateModifierLabels` repopulates on the flip. Liquid Glass
  styling (#5536) applies to every item regardless of position.
- Settings editor already lists `displayItems` and reorders via `onMove`, so the
  built-ins now appear there too; updated footer + per-action display names.

Tests: v3 widening transform (Kit, swift test) and the live config model
(fresh-install default order, reorder/hide round-trips, v2→v3 migration with a
partial enabled subset; cmuxFeatureTests, simulator).

Stacks on #5536 (feat-ios-accessory-glass): sequence #5536 → this.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@lawrencecchen
lawrencecchen changed the base branch from feat-ios-accessory-glass to main June 8, 2026 02:50
@lawrencecchen
lawrencecchen force-pushed the feat-ios-toolbar-reorder-builtins branch from c37047b to 1ac9735 Compare June 8, 2026 02:50
Comment on lines +249 to 256
private static func accessoryAction(for modifier: TerminalInputModifier) -> TerminalInputAccessoryAction? {
switch modifier {
case .control: return .control
case .alternate: return .alternate
case .command: return .command
case .shift: return .shift
}
}

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.

P1 shift case silently disarms on every bar rebuild

accessoryAction(for: .shift) returns .shift rather than nil, so the early-exit guard let action = Self.accessoryAction(for: armed) else { return } never fires when shift is armed. Because .shift is intentionally not in enabledItems (isUserConfigurable is false), its button is never added to renderedActions — so guard !renderedActions.contains(action) is always true, and every call to populateAccessoryActions() (triggered by hiding any item, toggling an enabled flag, or a Mac-remote switch) calls modifierState.disarmAll() while shift is active, silently clearing it. Returning nil for .shift makes the guard exit without touching modifier state, which is the correct invariant since shift has no bar button to reconcile against.

Suggested change
private static func accessoryAction(for modifier: TerminalInputModifier) -> TerminalInputAccessoryAction? {
switch modifier {
case .control: return .control
case .alternate: return .alternate
case .command: return .command
case .shift: return .shift
}
}
private static func accessoryAction(for modifier: TerminalInputModifier) -> TerminalInputAccessoryAction? {
switch modifier {
case .control: return .control
case .alternate: return .alternate
case .command: return .command
case .shift: return nil
}
}

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@ios/cmuxPackage/Tests/cmuxFeatureTests/TerminalAccessoryConfigurationTests.swift`:
- Around line 29-50: The test freshInstallDefaultOrder currently only checks
displayOrder (which excludes .shift) so add a runtime assertion against the
actual rendered accessory toolbar to catch regressions: after creating let
config = TerminalAccessoryConfiguration(defaults: freshDefaults()), instantiate
or request the real accessory view/controller that uses that config (the same
codepath used to render the input accessory) and collect its visible toolbar
item identifiers, then assert that the visible items do not contain id(.shift);
keep the existing displayOrder checks and use TerminalAccessoryConfiguration,
displayOrder, TerminalInputAccessoryAction.configurableActions and
config.isEnabled references to locate the test and the configuration.

In
`@Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift`:
- Around line 201-223: populateAccessoryActions currently rebuilds the bar only
from TerminalAccessoryConfiguration.shared.enabledItems and the trailing
customize button, which drops the fixed Shift control; update
populateAccessoryActions to always add the fixed Shift accessory (use the
existing maker, e.g. makeShiftAccessoryButton or the Shift-specific view
factory) to the stack (in its fixed position) when rebuilding the
arrangedSubviews, then add the enabledItems (.builtin via makeAccessoryButton
and .custom via makeCustomAccessoryButton) and finally the settings button;
ensure you don't duplicate the Shift view if it's already present and that the
Shift button remains non-configurable and rendered regardless of enabledItems.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 245d4901-b925-4c90-a601-1865e358a53e

📥 Commits

Reviewing files that changed from the base of the PR and between 8d0ed95 and 1ac9735.

📒 Files selected for processing (8)
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift
  • Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
  • Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift
  • Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift
  • Packages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/ToolbarLayoutMigration.swift
  • Packages/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/ToolbarCustomizationTests.swift
  • ios/cmux/Resources/Localizable.xcstrings
  • ios/cmuxPackage/Tests/cmuxFeatureTests/TerminalAccessoryConfigurationTests.swift

Comment on lines +29 to +50
@Test("fresh install puts modifiers at the front and zoom at the back, all shown")
func freshInstallDefaultOrder() throws {
let config = TerminalAccessoryConfiguration(defaults: freshDefaults())
let order = config.displayOrder

// Leading region: ⌃ ⌥ ⌘ then paste, in that order.
#expect(Array(order.prefix(4)) == [
id(.control), id(.alternate), id(.command), id(.paste),
])
// Trailing region: the two zoom controls, in that order.
#expect(Array(order.suffix(2)) == [id(.zoomOut), id(.zoomIn)])
// Esc sits right after Tab in the redesigned default.
let tabIndex = try #require(order.firstIndex(of: id(.tab)))
#expect(order[tabIndex + 1] == id(.escape))
// Everything is shown on a fresh install, including the now-configurable
// modifiers/zoom/paste.
for action in TerminalInputAccessoryAction.configurableActions {
#expect(config.isEnabled(action.itemID))
}
// Shift is never surfaced as a bar button.
#expect(!order.contains(id(.shift)))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add a behavior test for the fixed Shift key.

These assertions stop at displayOrder, which intentionally excludes .shift, so they would still pass if the accessory bar forgot to render the pinned Shift button. Please add a runtime check against the actual toolbar contents so this regression is caught next time.

As per coding guidelines, "When tests miss a bug, add or adjust behavior-level coverage around the exact repro path before claiming the fix is complete."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@ios/cmuxPackage/Tests/cmuxFeatureTests/TerminalAccessoryConfigurationTests.swift`
around lines 29 - 50, The test freshInstallDefaultOrder currently only checks
displayOrder (which excludes .shift) so add a runtime assertion against the
actual rendered accessory toolbar to catch regressions: after creating let
config = TerminalAccessoryConfiguration(defaults: freshDefaults()), instantiate
or request the real accessory view/controller that uses that config (the same
codepath used to render the input accessory) and collect its visible toolbar
item identifiers, then assert that the visible items do not contain id(.shift);
keep the existing displayOrder checks and use TerminalAccessoryConfiguration,
displayOrder, TerminalInputAccessoryAction.configurableActions and
config.isEnabled references to locate the test and the configuration.

Source: Coding guidelines

Comment on lines 201 to 223
private func populateAccessoryActions() {
guard let stack = accessoryStackView else { return }
for view in stack.arrangedSubviews {
stack.removeArrangedSubview(view)
view.removeFromSuperview()
}
commandAccessoryButton?.removeFromSuperview()
commandAccessoryButton = nil

// Pinned leading modifier controls, in fixed order.
for action in Self.pinnedLeadingActions {
let button = makeAccessoryButton(for: action)
// Command is Mac-only; kept out of the stack and inserted by
// applyModifierPresentation() when driving a Mac remote.
if action == .command {
commandAccessoryButton = button
} else {
stack.addArrangedSubview(button)
}
}
// The user-configurable region: built-in shortcuts and custom actions in
// the user's saved order.
// The user-configurable region: built-in shortcuts/modifiers/zoom/paste
// and custom actions, all in the user's saved order.
for item in TerminalAccessoryConfiguration.shared.enabledItems {
switch item {
case let .builtin(action):
// ⌘ only makes sense against a Mac remote; skip it otherwise
// (it stays in the saved order, just unrendered, so flipping the
// remote re-shows it in place).
if action == .command && !isMacRemote { continue }
stack.addArrangedSubview(makeAccessoryButton(for: action))
case let .custom(custom):
stack.addArrangedSubview(makeCustomAccessoryButton(for: custom))
}
}
// Pinned trailing zoom controls, after the configurable shortcuts (the
// redesigned bar moved zoom here from the leading region).
for action in Self.pinnedTrailingActions {
stack.addArrangedSubview(makeAccessoryButton(for: action))
}
// The "customize" button pinned at the very end of the bar.
stack.addArrangedSubview(makeToolbarSettingsButton())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the fixed Shift button in the rebuilt bar.

populateAccessoryActions() now renders only enabledItems plus the trailing customize control. Because .shift is intentionally outside the configurable order, this rebuild drops the Shift key from the accessory bar entirely instead of keeping it as the fixed non-configurable control for this feature.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift`
around lines 201 - 223, populateAccessoryActions currently rebuilds the bar only
from TerminalAccessoryConfiguration.shared.enabledItems and the trailing
customize button, which drops the fixed Shift control; update
populateAccessoryActions to always add the fixed Shift accessory (use the
existing maker, e.g. makeShiftAccessoryButton or the Shift-specific view
factory) to the stack (in its fixed position) when rebuilding the
arrangedSubviews, then add the enabledItems (.builtin via makeAccessoryButton
and .custom via makeCustomAccessoryButton) and finally the settings button;
ensure you don't duplicate the Shift view if it's already present and that the
Shift button remains non-configurable and rendered regardless of enabledItems.

@lawrencecchen
lawrencecchen merged commit 0a93eef into main Jun 8, 2026
28 of 30 checks passed
lawrencecchen added a commit that referenced this pull request Jun 8, 2026
Every terminal accessory-bar button now sizes to its intrinsic content
plus one shared compact horizontal inset, instead of being pinned wide.

Width:
- Tab/Esc/^C/^D and other text buttons were floored at
  accessoryButtonMinWidth (44pt via greaterThanOrEqualToConstant), so the
  short labels could not get narrower than 44pt. Modifier (⌃⌥⌘) and icon
  buttons (zoom/paste) were truly fixed at 44pt (equalToConstant).
- All three creation paths (makeAccessoryButton, makeCustomAccessoryButton,
  makeToolbarSettingsButton) now use a single greaterThanOrEqualToConstant
  floor, lowered 44 -> 34 (tap-target minimum), so every button hugs its
  content and only floors when the glyph is tiny.
- accessoryButtonContentInsets leading/trailing 10 -> 8 so the bar reads
  tight and uniform; this is what shrinks the already-wide "Tab".

Icons: unify all SF Symbols under one shared symbol config. The keyboard
toggle dropped from a private 16pt config to the shared 14pt config, and
the arrow keys (↑↓←→) became arrow.up/down/left/right SF Symbols so they
render at the same size as paste/zoom instead of as oversized Unicode
glyphs. Arrow accessibility labels added (image-only buttons need them).

Liquid Glass styling (#5536) and reorderable built-ins (#5579) are intact.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview – cmux — 1ac97352 Deployed Jun 8, 2026 by vercel[bot]
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.

1 participant