Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/src/code/editor/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,9 @@ impl CodeEditorView {
comments.pending_comment = PendingComment::Closed;
});
});
// Reclaim focus so the now-hidden CommentEditor's
// RichTextEditorView doesn't hold stale focus.
ctx.focus_self();
ctx.notify();
}
CommentEditorEvent::DeleteComment { id } => {
Expand Down
46 changes: 12 additions & 34 deletions app/src/code_review/code_review_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ pub enum CodeReviewAction {
OpenCreatePrDialog,
ViewPr(String),
PublishBranch,
SubmitComments,
}

pub struct FileState {
Expand Down Expand Up @@ -1272,7 +1273,6 @@ impl CodeReviewView {
Menu::new()
.prevent_interaction_with_other_elements()
.with_drop_shadow()
.with_width(140.)
});
ctx.subscribe_to_view(&git_operations_menu, |me, _, event, ctx| match event {
MenuEvent::ItemSelected | MenuEvent::Close { .. } => {
Expand Down Expand Up @@ -6719,9 +6719,8 @@ impl CodeReviewView {
// `has_upstream` controls the label/icon on the push-chained
// intent (Commit and push vs Commit and publish).
let diff_state = self.diff_state_model.as_ref(ctx);
let allow_create_pr = diff_state.pr_info().is_none()
&& !diff_state.is_pr_info_refreshing()
&& !diff_state.is_on_main_branch();
let allow_create_pr =
diff_state.pr_info().is_none() && !diff_state.is_on_main_branch();
let has_upstream = diff_state.upstream_ref().is_some();
ctx.add_typed_action_view(|ctx| {
GitDialog::new_for_commit(
Expand Down Expand Up @@ -6774,7 +6773,6 @@ impl CodeReviewView {
let has_uncommitted_changes = self.has_uncommitted_changes(app);
let has_upstream = diff_state.upstream_ref().is_some();
let has_local_commits = !diff_state.unpushed_commits().is_empty();
let is_pr_info_refreshing = diff_state.is_pr_info_refreshing();
// False when upstream == main (e.g. after `git checkout -b feature origin/master`),
// which means the branch hasn't been pushed to its own remote ref yet.
let upstream_differs_from_main = diff_state.upstream_differs_from_main();
Expand All @@ -6787,11 +6785,7 @@ impl CodeReviewView {
PrimaryGitActionMode::Push
} else if diff_state.pr_info().is_some() {
PrimaryGitActionMode::ViewPr
} else if !is_pr_info_refreshing
&& has_upstream
&& !diff_state.is_on_main_branch()
&& upstream_differs_from_main
{
} else if has_upstream && !diff_state.is_on_main_branch() && upstream_differs_from_main {
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] Dropping is_pr_info_refreshing lets the primary button show Create PR while PR discovery is still in flight, so a branch with an existing PR can open the create dialog before refresh completes. Keep the refresh guard here and in the matching Create PR paths.

PrimaryGitActionMode::CreatePr
} else {
// Nothing actionable — show Commit disabled.
Expand Down Expand Up @@ -6828,7 +6822,6 @@ impl CodeReviewView {
button.set_label("Push", ctx);
button.set_icon(Some(Icon::ArrowUp), ctx);
button.set_disabled(false, ctx);
button.clear_tooltip(ctx);
button.set_on_click(
|ctx| ctx.dispatch_typed_action(CodeReviewAction::OpenPushDialog),
ctx,
Expand All @@ -6844,7 +6837,6 @@ impl CodeReviewView {
button.set_label("Create PR", ctx);
button.set_icon(Some(Icon::Github), ctx);
button.set_disabled(false, ctx);
button.clear_tooltip(ctx);
button.set_on_click(
|ctx| ctx.dispatch_typed_action(CodeReviewAction::OpenCreatePrDialog),
ctx,
Expand All @@ -6853,21 +6845,15 @@ impl CodeReviewView {
});
}
PrimaryGitActionMode::ViewPr => {
let diff_state = self.diff_state_model.as_ref(ctx);
let pr_info = diff_state.pr_info().cloned();
let is_pr_info_refreshing = diff_state.is_pr_info_refreshing();
let pr_info = self.diff_state_model.as_ref(ctx).pr_info().cloned();
if let Some(pr_info) = pr_info {
let url = pr_info.url.clone();
let number = pr_info.number;
let label = format!("PR #{number}");
self.git_primary_action_button.update(ctx, |button, ctx| {
button.set_label(label, ctx);
button.set_icon(Some(Icon::Github), ctx);
button.set_disabled(is_pr_info_refreshing, ctx);
button.set_tooltip(
is_pr_info_refreshing.then_some("Refreshing PR info"),
ctx,
);
button.set_disabled(false, ctx);
button.set_on_click(
move |ctx| {
ctx.dispatch_typed_action(CodeReviewAction::ViewPr(url.clone()))
Expand All @@ -6883,7 +6869,6 @@ impl CodeReviewView {
button.set_label("Publish", ctx);
button.set_icon(Some(Icon::UploadCloud), ctx);
button.set_disabled(false, ctx);
button.clear_tooltip(ctx);
button.set_on_click(
|ctx| ctx.dispatch_typed_action(CodeReviewAction::PublishBranch),
ctx,
Expand Down Expand Up @@ -6932,12 +6917,10 @@ impl CodeReviewView {
/// (e.g. a worktree branch whose tracking was auto-set to origin/master).
fn pr_menu_item(&self, app: &AppContext) -> MenuItem<CodeReviewAction> {
let diff_state = self.diff_state_model.as_ref(app);
let is_pr_info_refreshing = diff_state.is_pr_info_refreshing();
if let Some(pr_info) = diff_state.pr_info().cloned() {
MenuItemFields::new(format!("PR #{}", pr_info.number))
.with_icon(Icon::Github)
.with_on_select_action(CodeReviewAction::ViewPr(pr_info.url))
.with_disabled(is_pr_info_refreshing)
.into_item()
} else {
let is_on_main = diff_state.is_on_main_branch();
Expand All @@ -6946,12 +6929,7 @@ impl CodeReviewView {
MenuItemFields::new("Create PR")
.with_icon(Icon::Github)
.with_on_select_action(CodeReviewAction::OpenCreatePrDialog)
.with_disabled(
is_pr_info_refreshing
|| is_on_main
|| !has_upstream
|| !upstream_differs_from_main,
)
.with_disabled(is_on_main || !has_upstream || !upstream_differs_from_main)
.into_item()
}
}
Expand Down Expand Up @@ -7748,6 +7726,10 @@ impl TypedActionView for CodeReviewView {
});
ctx.notify();
}
CodeReviewAction::SubmitComments => {
self.handle_submit_review_with_comments(ctx);
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] The shortcut path calls submit directly, bypassing the disabled state computed for send_button; Cmd+Enter can still try to send when there is no destination or no Warp AI availability. Gate this action with the same sendability predicate as the button.

ctx.focus_self();
}
}
}
}
Expand Down Expand Up @@ -7819,11 +7801,7 @@ impl BackingView for CodeReviewView {
AppContext::show_native_platform_modal(ctx, dialog);
} else if cfg!(all(
not(target_family = "wasm"),
any(
target_os = "linux",
target_os = "freebsd",
target_os = "windows"
)
any(target_os = "linux", target_os = "windows")
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 drops FreeBSD from the unsaved-changes modal branch; on FreeBSD the close handler enters the unsaved-changes block but shows no dialog and does not close. Restore the platform in this cfg.

Suggested change
any(target_os = "linux", target_os = "windows")
any(
target_os = "linux",
target_os = "freebsd",
target_os = "windows"
)

)) {
// Find the workspace to show the Warp-native modal
if let Some(workspace) = ctx
Expand Down
84 changes: 35 additions & 49 deletions app/src/code_review/comment_list_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ use crate::notebooks::editor::view::{EditorViewEvent, RichTextEditorView};
use crate::send_telemetry_from_ctx;
use crate::settings::AISettings;
use crate::view_components::action_button::{
ActionButton, ActionButtonTheme, ButtonSize, NakedTheme, SecondaryTheme,
ActionButton, ActionButtonTheme, ButtonSize, KeystrokeSource, NakedTheme, PrimaryTheme,
SecondaryTheme,
};
use crate::{
appearance::Appearance, code_review::code_review_view::CodeReviewView,
Expand Down Expand Up @@ -49,8 +50,8 @@ use warpui::{
},
platform::Cursor,
ui_components::{
button::{ButtonTooltipPosition, ButtonVariant},
components::{UiComponent, UiComponentStyles},
button::ButtonVariant,
components::UiComponent,
},
units::Pixels,
AppContext, Entity, EntityId, ModelHandle, SingletonEntity, TypedActionView, View, ViewContext,
Expand Down Expand Up @@ -131,7 +132,6 @@ struct ViewState {
chevron_mouse_state: MouseStateHandle,
outdated_chevron_mouse_state: MouseStateHandle,
cancel_button_mouse_state: MouseStateHandle,
submit_button_mouse_state: MouseStateHandle,
resizable_state: ResizableStateHandle,
}

Expand All @@ -142,7 +142,6 @@ impl Default for ViewState {
chevron_mouse_state: Default::default(),
outdated_chevron_mouse_state: Default::default(),
cancel_button_mouse_state: Default::default(),
submit_button_mouse_state: Default::default(),
resizable_state: resizable_state_handle(300.0),
}
}
Expand Down Expand Up @@ -192,6 +191,7 @@ pub struct CommentListView {
active_overflow_comment_id: Option<CommentId>,
pending_scroll_to_comment: Option<CommentId>,
comments_button: ViewHandle<ActionButton>,
send_button: ViewHandle<ActionButton>,
}

impl CommentListView {
Expand All @@ -218,6 +218,22 @@ impl CommentListView {
Event::ItemHovered => {}
});

let send_button = ctx.add_typed_action_view(|ctx| {
ActionButton::new("Send to Agent", PrimaryTheme)
.with_keybinding(
KeystrokeSource::Binding("code_review:submit_comments"),
ctx,
)
.on_click(|ctx| {
ctx.dispatch_typed_action(CommentListAction::Submit);
})
.with_size(ButtonSize::Small)
});

ctx.subscribe_to_model(&AIRequestUsageModel::handle(ctx), |me, _, _, ctx| {
me.update_send_button_state(ctx);
});

Self {
parent,
comment_model: None,
Expand All @@ -231,6 +247,7 @@ impl CommentListView {
active_overflow_comment_id: None,
pending_scroll_to_comment: None,
comments_button,
send_button,
}
}

Expand Down Expand Up @@ -276,6 +293,7 @@ impl CommentListView {
) {
if self.review_destination != destination {
self.review_destination = destination;
self.update_send_button_state(ctx);
ctx.notify();
}
}
Expand Down Expand Up @@ -436,6 +454,7 @@ impl CommentListView {
}

self.recompute_comment_button_label(ctx);
self.update_send_button_state(ctx);
ctx.notify();
}

Expand Down Expand Up @@ -550,6 +569,7 @@ impl CommentListView {
self.comments_by_id.clear();
self.is_collapsed = true;
self.pending_scroll_to_comment = None;
self.update_send_button_state(ctx);
ctx.notify();
}

Expand Down Expand Up @@ -929,16 +949,19 @@ impl CommentListView {
}
}

fn render_send_button(&self, appearance: &Appearance, ctx: &AppContext) -> Box<dyn Element> {
fn render_send_button(&self, _appearance: &Appearance, _ctx: &AppContext) -> Box<dyn Element> {
ChildView::new(&self.send_button).finish()
}

fn update_send_button_state(&mut self, ctx: &mut ViewContext<Self>) {
let ai_available = AIRequestUsageModel::as_ref(ctx).has_any_ai_remaining(ctx);
let ai_enabled = AISettings::as_ref(ctx).is_any_ai_enabled(ctx);
let has_sendable_comments = self.has_non_outdated_comments();

// CLI agents don't consume AI credits, so bypass the ai_available check.
let enable_send = match &self.review_destination {
ReviewDestination::None => false,
ReviewDestination::Cli(_) => has_sendable_comments,
ReviewDestination::Warp => ai_available && has_sendable_comments,
ReviewDestination::Warp => ai_available && ai_enabled && has_sendable_comments,
};

let tooltip_text = Self::send_button_tooltip_text(
Expand All @@ -948,47 +971,10 @@ impl CommentListView {
ai_enabled,
);

let tooltip = appearance
.ui_builder()
.tool_tip(tooltip_text.into_owned())
.build()
.finish();

let button = appearance
.ui_builder()
.button(
ButtonVariant::Accent,
self.view_state.submit_button_mouse_state.clone(),
)
.with_text_label("Send to Agent".to_string())
.with_tooltip(|| tooltip)
.with_tooltip_position(ButtonTooltipPosition::AboveLeft);

if enable_send {
EventHandler::new(button.build().finish())
.on_left_mouse_down(move |ctx, _, _| {
ctx.dispatch_typed_action(CommentListAction::Submit);
DispatchEventResult::StopPropagation
})
.finish()
} else {
// Custom disabled button appearance because setting the `disabled` property
// on the button itself prevents all hoverable interaction (including tooltips).
let background_fill = appearance.theme().surface_3();
let foreground_color = appearance
.theme()
.disabled_text_color(background_fill)
.into_solid();
button
.with_style(UiComponentStyles {
background: Some(background_fill.into_solid().into()),
border_color: Some(foreground_color.into()),
font_color: Some(foreground_color),
..Default::default()
})
.build()
.finish()
}
self.send_button.update(ctx, |button, ctx| {
button.set_disabled(!enable_send, ctx);
button.set_tooltip(Some(tooltip_text.into_owned()), ctx);
});
}

fn render_comment(
Expand Down
12 changes: 11 additions & 1 deletion app/src/code_review/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,16 @@ pub fn init(app: &mut AppContext) {
id!("CodeReviewView") & !id!("IMEOpen"),
)]);

app.register_editable_bindings([
EditableBinding::new(
"code_review:submit_comments",
"Send review comments to Agent",
CodeReviewAction::SubmitComments,
)
.with_context_predicate(id!("CodeReviewView"))
.with_key_binding("cmdorctrl-enter"),
]);

diff_menu::init(app);
diff_selector::init(app);
git_dialog::init(app);
Expand Down Expand Up @@ -145,7 +155,7 @@ fn is_file_autogenerated(file_path: &Path, content: Option<&str>) -> bool {
/// A [`SingletonEntity`] that the tracks events for the code review model throughought the app.
/// We need this because toasts are emitted in the Workspace, and want a click handler that triggers
/// behavior in a _specific_ review pane. We use this model get around restrictions that make it hard
/// to emit a CodeReviewView typed action from the toast because it's not in the view responder chain of the
/// to emit a CodeReviewView typed action from the toast because it's not in the view reponder chain of the
/// Workspace.
pub struct GlobalCodeReviewModel;

Expand Down
10 changes: 8 additions & 2 deletions app/src/notebooks/editor/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,12 +363,18 @@ pub fn init(app: &mut AppContext) {
.with_key_binding("cmdorctrl-enter"),
]);

// When shell command execution is disabled (e.g., comment editors),
// When shell command execution is disabled (e.g., inline comment editors),
// Cmd/Ctrl+Enter emits CmdEnter instead of running commands.
// The EditorIsEditable guard is required: without it, Selectable
// RichTextEditorViews (read-only comment cards, dormant CommentEditors)
// would match this binding and consume the keystroke before it can
// bubble up to CodeReviewView's SubmitComments handler.
app.register_fixed_bindings([FixedBinding::new(
"cmdorctrl-enter",
EditorViewAction::CmdEnter,
id!("RichTextEditorView") & !id!("CanExecuteShellCommands"),
id!("RichTextEditorView")
& !id!("CanExecuteShellCommands")
& id!("EditorIsEditable"),
)]);

// When shell command execution is disabled (e.g., comment editors),
Expand Down