Skip to content

Commit 5bb7018

Browse files
Fix diff button when Show code review button toggle is off (warpdotdev#9600)
Closes warpdotdev#9196. ### Description Two `show_code_review_button` gates were dropping panel-open requests on the floor when the user had hidden the toolbar button: **1. Data-path gate at `Workspace::setup_code_review_panel` (`view.rs:7982`)** ```rust if !*TabSettings::as_ref(ctx).show_code_review_button { return; } ``` `update_right_panel_open_state` calls into this whenever the right panel is being opened (chip click, `Shift+Cmd+=` keybinding, etc.), so the early return silently swallowed every explicit user action. **2. Render-path gate at `Workspace::render_config_panel` and `render_config_panel_maximized` (`view.rs:18981` / `19040`)** ```rust if !item.is_available(app) || !item.is_panel() { return None; } … if !HeaderToolbarItemKind::CodeReview.is_available(app) { return None; } ``` `HeaderToolbarItemKind::is_available` for `CodeReview` returns `*TabSettings::as_ref(app).show_code_review_button.value()` (`header_toolbar_item.rs:89`). So even after fix #1 set `pane_group.right_panel_open = true` and `setup_code_review_panel` ran, the next render frame saw `is_available() == false` and returned `None` — the `right_panel_view` was never added to the layout. This second gate is what @moirahuang flagged when their local repro still showed nothing happening after the first fix landed. The data was correct; the panel was just never composed into the UI. ### Fix 1. **Drop the early return at `setup_code_review_panel`.** The setting is meant to gate only the toolbar button's visibility (already enforced correctly by `header_toolbar_item.rs::is_available`, which feeds `render_header_toolbar_button` at `view.rs:17276`). 2. **Switch panel-render call sites from `is_available` → `is_supported`.** `is_available`'s own doc-comment says it's specifically *"Whether this item should be shown in the **toolbar** — checks both `is_supported` and user show/hide preferences."* Using it to gate panel rendering conflates two unrelated concerns. Panel rendering should only care about whether the feature is compiled in (`is_supported`), not whether the user has hidden the toolbar button. For `CodeReview`, `is_supported` is `cfg!(feature = "local_fs")`. For the other variants in the same match (`TabsPanel`, `ToolsPanel`), `is_available` already equals `is_supported` (default `_ => true` arm in the inner match), so behaviour is unchanged. `AgentManagement` and `NotificationsMailbox` return `None` unconditionally inside `render_config_panel`, so the change is moot for them too. ### Caller audit for `setup_code_review_panel` 5 call sites in `view.rs`: 1. `view.rs:3681` — `TransferredTab` flow, only runs when the source tab already had `right_panel_open == true`. 2. `view.rs:8136` — `update_right_panel_open_state` with `should_open == true`. **The diff-button path** that warpdotdev#9196 is about. 3. `view.rs:13372` — `PaneFocused` event, gated on `right_panel_open` already true. 4. `view.rs:13490` — `RepoChanged` event, gated on `right_panel_open` already true. 5. `view.rs:14458` — session env update, gated on `right_panel_open` already true. None of these need the `show_code_review_button` gate — they're either explicit user actions or gated on `right_panel_open` already being open. The toolbar button toggle continues to do its job at `render_header_toolbar_button` independently. ### Testing Reproduced @moirahuang's test locally on macOS 26.4.1 (Apple Silicon) against `WarpOss.app` built from this branch: 1. Settings → "Show code review button" → **OFF** 2. `echo "x" >> README.md` inside a git repo 3. Click the diff stats chip on the prompt (`+1 -0`) **Result:** Code review panel opens on the right showing the diff, while the toolbar button stays hidden — exactly the expected behaviour from issue warpdotdev#9196. Inverse case (toggle ON) also verified: toolbar button visible, panel still works the same. - `cargo fmt -p warp -- --check` passes. - `cargo nextest` skipped locally — Metal toolchain unavailable on my machine, mirroring warpdotdev#9277. CI will exercise the change. ### Server API No server changes. ### Agent Mode Not applicable. ### Changelog Entries `CHANGELOG-BUG-FIX`: The diff button on the terminal prompt now opens the code review panel even when the toolbar's "Show code review button" toggle is disabled (regression from a recent release). Co-authored-by: anshul-garg27 <13553550+anshul-garg27@users.noreply.github.com>
1 parent ad21d67 commit 5bb7018

3 files changed

Lines changed: 74 additions & 7 deletions

File tree

app/src/workspace/tab_settings.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,10 @@ impl HeaderToolbarChipSelection {
269269
Self::Custom { right, .. } => right.clone(),
270270
}
271271
}
272+
273+
pub fn contains_item(&self, item: &super::header_toolbar_item::HeaderToolbarItemKind) -> bool {
274+
self.left_items().contains(item) || self.right_items().contains(item)
275+
}
272276
}
273277

274278
settings::macros::implement_setting_for_enum!(

app/src/workspace/tab_settings_tests.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use super::*;
22
use crate::test_util::settings::initialize_settings_for_tests;
3+
use crate::workspace::header_toolbar_item::HeaderToolbarItemKind;
34
use settings::Setting;
45
use warpui::{App, SingletonEntity};
56

@@ -56,3 +57,45 @@ fn show_vertical_tab_panel_in_restored_windows_uses_vertical_tabs_path() {
5657
"show_panel_in_restored_windows"
5758
);
5859
}
60+
61+
#[test]
62+
fn header_toolbar_chip_selection_default_contains_code_review() {
63+
let config = HeaderToolbarChipSelection::Default;
64+
assert!(config.contains_item(&HeaderToolbarItemKind::CodeReview));
65+
}
66+
67+
#[test]
68+
fn header_toolbar_chip_selection_custom_without_code_review_reports_absent() {
69+
let config = HeaderToolbarChipSelection::Custom {
70+
left: vec![
71+
HeaderToolbarItemKind::TabsPanel,
72+
HeaderToolbarItemKind::ToolsPanel,
73+
],
74+
right: vec![HeaderToolbarItemKind::NotificationsMailbox],
75+
};
76+
assert!(!config.contains_item(&HeaderToolbarItemKind::CodeReview));
77+
assert!(config.contains_item(&HeaderToolbarItemKind::TabsPanel));
78+
assert!(config.contains_item(&HeaderToolbarItemKind::ToolsPanel));
79+
assert!(config.contains_item(&HeaderToolbarItemKind::NotificationsMailbox));
80+
assert!(!config.contains_item(&HeaderToolbarItemKind::AgentManagement));
81+
}
82+
83+
#[test]
84+
fn header_toolbar_chip_selection_custom_with_code_review_on_left_reports_present() {
85+
let config = HeaderToolbarChipSelection::Custom {
86+
left: vec![HeaderToolbarItemKind::CodeReview],
87+
right: vec![],
88+
};
89+
assert!(config.contains_item(&HeaderToolbarItemKind::CodeReview));
90+
}
91+
92+
#[test]
93+
fn header_toolbar_chip_selection_custom_empty_reports_all_absent() {
94+
let config = HeaderToolbarChipSelection::Custom {
95+
left: vec![],
96+
right: vec![],
97+
};
98+
for item in HeaderToolbarItemKind::all_items() {
99+
assert!(!config.contains_item(&item));
100+
}
101+
}

app/src/workspace/view.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7988,10 +7988,6 @@ impl Workspace {
79887988
context: Option<&CodeReviewPaneContext>,
79897989
ctx: &mut ViewContext<Self>,
79907990
) {
7991-
if !*TabSettings::as_ref(ctx).show_code_review_button {
7992-
return;
7993-
}
7994-
79957991
// If context is provided, use it directly. Otherwise, derive from active pane group.
79967992
let context_data: Option<(
79977993
Option<PathBuf>,
@@ -18294,6 +18290,18 @@ impl Workspace {
1829418290
self.render_config_panel_maximized(pane_group, &config, app),
1829518291
app,
1829618292
);
18293+
} else if !config.contains_item(&HeaderToolbarItemKind::CodeReview) {
18294+
Self::add_panel_with_separator(
18295+
&mut main_content,
18296+
&mut prev_panel_added,
18297+
self.render_config_panel(
18298+
&HeaderToolbarItemKind::CodeReview,
18299+
pane_group,
18300+
&config,
18301+
app,
18302+
),
18303+
app,
18304+
);
1829718305
}
1829818306
} else if !is_right_maximized {
1829918307
main_content = main_content.with_child(Shrinkable::new(1.0, terminal_content).finish());
@@ -18924,6 +18932,18 @@ impl Workspace {
1892418932
self.render_config_panel_maximized(pane_group, &config, app),
1892518933
app,
1892618934
);
18935+
} else if !config.contains_item(&HeaderToolbarItemKind::CodeReview) {
18936+
Self::add_panel_with_separator(
18937+
&mut panels_view,
18938+
&mut prev_panel_added,
18939+
self.render_config_panel(
18940+
&HeaderToolbarItemKind::CodeReview,
18941+
pane_group,
18942+
&config,
18943+
app,
18944+
),
18945+
app,
18946+
);
1892718947
}
1892818948
}
1892918949

@@ -18983,7 +19003,7 @@ impl Workspace {
1898319003
}
1898419004

1898519005
/// Renders a configurable panel for the given toolbar item, if it is open.
18986-
/// Returns `None` if the panel should not be rendered (item not available,
19006+
/// Returns `None` if the panel should not be rendered (item not supported,
1898719007
/// panel not open, or item is not a panel type).
1898819008
fn render_config_panel(
1898919009
&self,
@@ -18992,7 +19012,7 @@ impl Workspace {
1899219012
config: &HeaderToolbarChipSelection,
1899319013
app: &AppContext,
1899419014
) -> Option<Box<dyn Element>> {
18995-
if !item.is_available(app) || !item.is_panel() {
19015+
if !item.is_supported(app) || !item.is_panel() {
1899619016
return None;
1899719017
}
1899819018
match item {
@@ -19038,7 +19058,7 @@ impl Workspace {
1903819058
if !pane_group.right_panel_open || !pane_group.is_right_panel_maximized {
1903919059
return None;
1904019060
}
19041-
if !HeaderToolbarItemKind::CodeReview.is_available(app) {
19061+
if !HeaderToolbarItemKind::CodeReview.is_supported(app) {
1904219062
return None;
1904319063
}
1904419064
Some(Shrinkable::new(1.0, ChildView::new(&self.right_panel_view).finish()).finish())

0 commit comments

Comments
 (0)