-
Notifications
You must be signed in to change notification settings - Fork 7.2k
add ability to disable input temporarily in the TUI. #8876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
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: 8dcea2833f
ℹ️ 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".
| pub fn handle_key_event(&mut self, key_event: KeyEvent) -> (InputResult, bool) { | ||
| if !self.input_enabled { | ||
| return (InputResult::None, false); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Block paste events when input is disabled
The new input_enabled gate only short-circuits handle_key_event, but paste events still go through BottomPane::handle_paste → ChatComposer::handle_paste without checking input_enabled, so a bracketed paste (or clipboard paste event) will still insert text or attach images while the UI is in a read-only state. This makes the “input disabled” state inconsistent and can mutate the composer during sandbox setup; consider guarding paste handling on the same flag.
Useful? React with 👍 / 👎.
| pub fn handle_key_event(&mut self, key_event: KeyEvent) -> (InputResult, bool) { | ||
| if !self.input_enabled { | ||
| return (InputResult::None, false); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Block paste events when input is disabled
As in the original TUI, input_enabled only affects handle_key_event, but paste events still flow through handle_paste without checking the flag, so clipboard pastes can still change the text or add attachments even though input is supposed to be blocked. This breaks the read-only expectation during sandbox setup; consider gating paste handling on input_enabled as well.
Useful? React with 👍 / 👎.
dylan-hurd-oai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 comment, but otherwise LGTM
| "›".bold() | ||
| } else { | ||
| "›".dim() | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[non-blocking] I wonder if this is visually distinguished enough
We will disable input while the elevated sandbox setup is running.