Skip to content

feat(mobile): add opt-in terminal controls - #185

Closed
Lint111 wants to merge 1 commit into
Ark0N:masterfrom
Lint111:agent/split-mobile-terminal-controls
Closed

feat(mobile): add opt-in terminal controls#185
Lint111 wants to merge 1 commit into
Ark0N:masterfrom
Lint111:agent/split-mobile-terminal-controls

Conversation

@Lint111

@Lint111 Lint111 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Add one opt-in mobile terminal control system with responsive surfaces for keyboard-open and keyboard-hidden states.

Controls

  • Keyboard hidden:
    • Esc at the left edge
    • Up, Enter, and Down centered
    • Tab at the right edge
    • vertical swipe on the surrounding band sends Up or Down
    • pressing Up and Down together sends one Enter without leaking arrow keys
  • Keyboard open:
    • compact accessory row with Esc, arrows, Option+Enter, Tab/Shift+Tab, paste, path insertion, clear input, CLI commands, and dismiss
    • fixed-height horizontal layout keeps primary actions inside narrow phone widths
  • Optional feedback:
    • short vibration when navigator.vibrate is available
    • short Web Audio tone when enabled

Interaction Safety

  • Pointer gestures claim the interaction before button activation, so a swipe that begins or ends on a button does not also click it.
  • Canceled pointers clear pressed state without dispatch.
  • Controls hide while Codeman dialogs are open and return afterward.
  • Terminal-control taps do not focus xterm or open the mobile keyboard.
  • Active web views suppress terminal controls.
  • Existing upstream path-picker and clear-input accessory actions are preserved.

Settings and Migration

  • Mobile Terminal Controls, haptics, and sound are available in App Settings on touch devices.
  • Controls and haptics default off.
  • Explicit canonical or legacy true preferences migrate to enabled.
  • Legacy extendedKeyboardBar: false and device detection alone do not enable the new controls.

Volume Keys

Codeman maps browser keydown events reported as volume up/down and applies the same simultaneous-direction Enter chord. This is progressive enhancement only: most mobile browsers reserve physical volume keys at the operating-system level and do not expose those events to web pages. The visible controls are the reliable interface.

Validation

  • npx vitest run --config config/vitest.config.ts test/mobile-navigation-pad.test.ts
    • 21 tests passed
  • npx vitest run --config test/mobile/vitest.config.ts test/mobile/navigation-pad.test.ts
    • 10 Playwright-backed tests passed
  • npx vitest run --config test/mobile/vitest.config.ts test/mobile/settings.test.ts
    • 20 Playwright-backed tests passed
  • npx vitest run --config test/mobile/vitest.config.ts test/mobile/keyboard.test.ts -t "control rows|unified terminal-control|unified controls|accessory bar"
    • 4 focused tests passed
  • npm run check:frontend-syntax
    • 26 frontend files passed
  • npm run build
    • passed

Scope

This PR does not change terminal tap classification, draft/IME/paste handling, PTY viewport ownership, or terminal-history streaming. Those are separate review slices.

Unify keyboard-open and keyboard-hidden terminal navigation behind one opt-in setting. Add swipe, simultaneous Up+Down Enter, exposed volume-key mapping, modal suppression, and optional haptic or sound feedback while preserving upstream path-picker actions.
@Lint111
Lint111 marked this pull request as ready for review August 2, 2026 05:42
Copilot AI review requested due to automatic review settings August 2, 2026 05:42

Copilot AI 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.

Pull request overview

This PR introduces an opt-in “Mobile Terminal Controls” system for touch devices, adding two responsive control surfaces (keyboard-hidden navigation pad and keyboard-open accessory bar) plus user-facing settings and extensive mobile test coverage to validate layout and input behavior.

Changes:

  • Add unified mobile terminal controls (navigation pad + accessory bar) with optional haptic/audio feedback and modal/keyboard-aware visibility.
  • Update mobile keyboard/layout handling and CSS to support handheld (including scaled/foldable) viewport behaviors.
  • Expand/adjust Playwright + unit tests to cover settings defaults/migration, navigation pad interactions (including volume keys), and keyboard geometry.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/mobile/settings.test.ts Updates default expectations and adds assertions for new mobile terminal controls settings.
test/mobile/navigation-pad.test.ts Adds Playwright-backed interaction/layout tests for the new keyboard-hidden navigation pad and unified controls.
test/mobile/keyboard.test.ts Adjusts keyboard/layout expectations and adds coverage for incremental resize behavior and unified accessory actions.
test/mobile/helpers/constants.ts Adds a dedicated test port + selector + Android keyboard height constant for new navigation pad tests.
test/mobile-navigation-pad.test.ts Adds JSDOM unit tests for settings migration, pointer chords/swipes, and volume-key behavior.
src/web/public/terminal-ui.js Adds sendTerminalKey() to send control sequences without focusing xterm via the durable input path.
src/web/public/styles.css Introduces shared CSS vars and handheld keyboard-visible layout rules; updates accessory sizing.
src/web/public/settings-ui.js Adds settings UI wiring, defaults, persistence, and per-device filtering for mobile terminal controls + feedback toggles.
src/web/public/mobile.css Refactors mobile spacing/sizing to use shared vars; adds the navigation pad UI styles.
src/web/public/mobile-handlers.js Updates handheld detection/classes and keyboard baseline/layout logic; syncs mobile controls on resize/keyboard events.
src/web/public/keyboard-accessory.js Implements MobileTerminalControls, MobileNavigationPad, and updates accessory bar to unified action set + feedback.
src/web/public/index.html Adds App Settings toggles for Mobile Terminal Controls, haptics, and sound; updates viewport meta for keyboard resizing.
src/web/public/i18n.js Adds i18n strings for the new settings labels/descriptions/tooltips (including zh-CN).
src/web/public/app.js Initializes MobileTerminalControls from stored settings + defaults during app startup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1099 to +1103
sendKey(action) {
const sequence = TERMINAL_CONTROL_SEQUENCES[action];
if (!sequence || typeof app === 'undefined' || !app.activeSessionId) return;
app.sendTerminalKey(sequence);
this.feedback(action);
Comment on lines +1145 to +1149
/** True when a pointer target will perform its own semantic key claim. */
isKeyControlTarget(target) {
if (!target?.closest) return false;
if (target.closest('.mobile-terminal-nav')) return true;
const accessoryButton = target.closest('.keyboard-accessory-bar [data-action]');
@Ark0N

Ark0N commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Hi Lior, closing this one as part of a cleanup of the seventeen PRs currently open from you. The full explanation is in #173, and I would rather you read that one than this note, because it is the honest version and it is not a dismissal of your work.

The short form: sixteen PRs opened in a single day, roughly 42,000 added lines in total, landing mostly on the same few files (nine touch app.js, eight touch terminal-ui.js), with five already conflicting against master and none carrying CI results. I cannot review that at the standard this codebase needs, and merging it at a lower standard would be worse than not merging it.

Going forward, please keep no more than three open PRs at a time, each one a single behavior change that stands on its own, with a test that fails on master and passes with the fix. #214 and #215, merged today, are good models.

If this particular change fixes something that genuinely annoys you in daily use, it is a good candidate to be the first one you reopen on its own, rebased on current master. I will review it properly.

Thanks for the effort you put in, and sorry to close it this way.

@Ark0N Ark0N closed this Aug 5, 2026
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.

3 participants