[WAR-7321] Fix Cmd+W targeting in Settings - #16018
Draft
warp-agent-staging[bot] wants to merge 2 commits into
Draft
warp-agent-staging[bot] wants to merge 2 commits into
warp-agent-staging[bot] wants to merge 2 commits into
Conversation
Parent the reused Settings backing view to its pane wrapper during attachment so responder-chain actions target the active Settings tab before render. Extend the integration test to cover the Classic input-position flow and verify the parent relationship. Co-Authored-By: Warp <agent@warp.dev>
Contributor
Author
|
This PR was generated with Warp. Comment |
Co-Authored-By: Warp <agent@warp.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Description
Cmd+W resolves through the focused view's responder chain. The Settings backing view is created once under the workspace and later reused inside a pane, so before its first pane render its responder ancestry could still point at the workspace and route Close Current Session away from the active Settings tab.
This change records the Settings backing view under its
PaneViewwhenever the pane attaches and conditionally clears that mapping when the pane detaches. The compare-and-remove cleanup prevents an old wrapper's deferred teardown from erasing a newer wrapper's parentage. The action now reaches the active SettingsPaneGroupwithout changing global Cmd+W semantics, multi-pane close behavior, or tools-panel bindings.The integration regressions cover the reported Appearance → Input position → Classic flow, a Settings and terminal split, and close/reopen during deferred cleanup. They confirm Close Current Session removes only the focused Settings pane and that the reopened Settings responder ancestry survives cleanup of the old wrapper.
Linked Issue
WAR-7321
ready-to-specorready-to-implement. The work is tracked in Linear, where current policy does not use the legacy pipeline labels; implementation was explicitly authorized.Testing
Pre-fix regression proof:
cargo test --manifest-path /Users/runner/workspace/warp/Cargo.toml -p integration --test integration test_open_and_close_settings -- --nocapturefailed atSettings view must be parented to its pane before rendering.Post-fix regression: the same command passed, 1 test passed.
cargo test --manifest-path /Users/runner/workspace/warp/Cargo.toml -p integration --test integration test_close_settings_split_and_reopen_during_cleanup -- --nocapture— passed, 1 test passed.cargo test --manifest-path /Users/runner/workspace/warp/Cargo.toml -p integration --test integration test_pane_group_state_close_pane -- --nocapture— passed, 1 test passed../script/format— passed.cargo build --manifest-path /Users/runner/workspace/warp/Cargo.toml -p warp— passed on macOS arm64.cargo clippy --manifest-path /Users/runner/workspace/warp/Cargo.toml -p warp --all-targets --tests -- -D warnings— passed.cargo clippy --manifest-path /Users/runner/workspace/warp/Cargo.toml -p integration --all-targets --tests -- -D warnings— passed.cargo clippy --manifest-path /Users/runner/workspace/warp/Cargo.toml --workspace --all-targets --all-features --tests -- -D warnings— blocked by existing all-features cfg conflicts inapp/src/ai/execution_profiles/profiles_tests.rsand unrelated dead-code diagnostics; no reported failure was in this diff.I have manually tested my changes locally with
./script/run. The built macOS app was operated end-to-end: with two terminal tabs open, Settings → Appearance → Input position was changed to Classic, then one Cmd+W closed Settings and retained both terminal tabs, including theTAB_MARKER_TWOtab.Screenshots / Videos
Computer-use video recordings
Warp: create tab, open Settings, change Input position to Classic, Cmd+W: Shows creating a second terminal tab in Warp, opening Settings (Cmd+,), navigating to Appearance, changing the Input position setting to "Start at the top (Classic mode)", then pressing Cmd+W and observing which tab closes.
Computer-use screenshots (3)
Warp Settings > Appearance page after selecting the "Start at the top (Classic mode)" option for "Input position"; the left sidebar shows the 3 tabs (New session, echo TAB_MARKER_TWO, Settings-active) and the Input position dropdown now reads "Start at the top (Classic mode)".
Warp Settings > Appearance page showing the tab bar (New session, echo TAB_MARKER_TWO, Settings-active) and the "Input position" row now set to "Start at the top (Classic mode)" after selecting that option from the dropdown.
Warp tab bar and content area immediately after pressing Cmd+W once: the Settings tab has closed, leaving "New session" and "echo TAB_MARKER_TWO" (active) tabs; the active tab shows the previous "echo TAB_MARKER_TWO" command and its "TAB_MARKER_TWO" output inline, consistent with the newly selected Classic input position.
Agent Mode
CHANGELOG-BUG-FIX: Fixed Cmd+W closing another tab instead of the active Settings tab after changing the input position.