Skip to content

Update banner UI for exiting shell mode in shell prefix#11510

Merged
evelyn-with-warp merged 5 commits into
masterfrom
evelyn/fix-shell-prefix-ui
May 22, 2026
Merged

Update banner UI for exiting shell mode in shell prefix#11510
evelyn-with-warp merged 5 commits into
masterfrom
evelyn/fix-shell-prefix-ui

Conversation

@evelyn-with-warp
Copy link
Copy Markdown
Contributor

@evelyn-with-warp evelyn-with-warp commented May 21, 2026

Description

slack context: https://warpdev.slack.com/archives/C08QY6ZBK2A/p1779384636368719

Linked Issue

  • The linked issue is labeled ready-to-spec or ready-to-implement.
  • Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes).

Testing

  • I have manually tested my changes locally with ./script/run

Screenshots / Videos

https://www.loom.com/share/579775abfd344b23a32eae4cbf216c5c

Agent Mode

  • Warp Agent Mode - This PR was created via Warp's AI Agent Mode

@cla-bot cla-bot Bot added the cla-signed label May 21, 2026
@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 21, 2026

@evelyn-with-warp

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR updates the Agent View shell-mode exit banner to show Cmd+I instead of Backspace.

Concerns

  • The new shortcut display is hardcoded to Cmd+I, but the registered binding is Cmd+I on macOS, Ctrl+I on Linux/Windows, and is user-editable. The banner can therefore show the wrong shortcut.
  • This is a user-facing UI change, but the PR description does not include screenshots or a screen recording demonstrating the updated banner end to end.

Verdict

Found: 0 critical, 2 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

keystroke: Keystroke {
key: "backspace".to_owned(),
key: "i".to_owned(),
cmd: true,
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.

⚠️ [IMPORTANT] This hardcodes Cmd+I for every platform/user, but the registered Set Input Mode to Agent Mode binding is Cmd+I on macOS, Ctrl+I on Linux/Windows, and editable; use the existing keybinding lookup so the banner matches the active binding.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@evelyn-with-warp evelyn-with-warp requested a review from szgupta May 21, 2026 22:27
@evelyn-with-warp
Copy link
Copy Markdown
Contributor Author

/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 21, 2026

@evelyn-with-warp

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR updates the agent view shell-prefix exit banner to display the configured Agent Mode keybinding instead of always showing backspace, with visual evidence provided in the PR description.

Concerns

  • The new dynamic keybinding lookup drops the entire shell-mode exit hint when the Agent Mode binding is unset, even though backspace still exits shell prefix mode.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Comment thread app/src/ai/blocklist/agent_view/agent_message_bar.rs
Co-authored-by: oz-for-oss[bot] <277970191+oz-for-oss[bot]@users.noreply.github.com>
@evelyn-with-warp
Copy link
Copy Markdown
Contributor Author

/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 21, 2026

@evelyn-with-warp

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR updates the Agent View shell-prefix exit hint so it displays the configured input:set_mode_agent shortcut, with a Backspace fallback when no binding is available. The PR description includes visual evidence, and I found no blocking correctness, security, or spec-alignment issues.

Concerns

  • The configured shortcut hint still inherits Backspace-specific disabled styling when the buffer is non-empty, which can make an available shortcut appear unavailable.

Verdict

Found: 0 critical, 0 important, 1 suggestion

Approve with nits

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

key: "backspace".to_owned(),
..Default::default()
},
keystroke: set_input_mode_agent_keystroke,
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.

💡 [SUGGESTION] This now displays the configured mode-toggle shortcut, which still works with a non-empty buffer, but the color overrides below keep rendering it as disabled whenever text is present. Keep the disabled styling only for the Backspace fallback, or compute it based on whether this shortcut is actually unavailable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

https://www.loom.com/share/d7c3e41433bf49f993c1ecba2adb5a00
this is the experience of completely removing color opacity regardless of buffer empty or not

Co-Authored-By: Oz <oz-agent@warp.dev>
@evelyn-with-warp evelyn-with-warp force-pushed the evelyn/fix-shell-prefix-ui branch from e0c921c to 4848c83 Compare May 22, 2026 00:18
@evelyn-with-warp
Copy link
Copy Markdown
Contributor Author

/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 22, 2026

@evelyn-with-warp

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR updates the agent message bar hint for exiting locked shell mode so it reflects the configured Set Input Mode to Agent Mode keybinding, falling back to Backspace when no binding is available. The attached Loom link satisfies the visual-evidence requirement for this user-facing UI change.

Concerns

  • No blocking correctness or security concerns found in the reviewed diff.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Comment on lines +961 to +963
|| Keystroke {
key: "backspace".to_owned(),
..Default::default()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hrm, what's the point of the fallback? I would just get rid of this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@szgupta it was called out by Oz #11510 (review) to handle keybinding unset scenario. Is this a false alarm then? e.g users should have it by os default?

@evelyn-with-warp evelyn-with-warp enabled auto-merge (squash) May 22, 2026 17:14
@evelyn-with-warp evelyn-with-warp merged commit d5c71b4 into master May 22, 2026
25 checks passed
@evelyn-with-warp evelyn-with-warp deleted the evelyn/fix-shell-prefix-ui branch May 22, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants