From 9741e9ab8bcbb71ec54f974ab149897996cc2c20 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Tue, 30 Sep 2025 12:27:23 -0300 Subject: [PATCH] rules library: Improve delineation of default and non-default rules (#39209) Closes https://github.com/zed-industries/zed/issues/39183 This PR adds UI improvements to clarify the concept of "default rules" and how they separate from regular rules. This is mostly motivated by the issue linked above, where it clarified that the star icon was communicating a "favoriting" affordance, which is not correct with how rules work in Zed. When you tag/attach a rule as default, it will always be included in every prompt, together with the agent's system prompt and project rules (if they exist). Hopefully, this will make understanding better. Here's how it looks like now? https://github.com/user-attachments/assets/435d3af7-e8a6-4646-8f00-94a409bd5f42 Release Notes: - Improve rules library UI to better communicate the concept of default rules vs. regular rules. --- assets/icons/paperclip.svg | 3 + crates/icons/src/icons.rs | 1 + crates/rules_library/src/rules_library.rs | 391 ++++++++++++++-------- 3 files changed, 248 insertions(+), 147 deletions(-) create mode 100644 assets/icons/paperclip.svg diff --git a/assets/icons/paperclip.svg b/assets/icons/paperclip.svg new file mode 100644 index 0000000000000000000000000000000000000000..7a864103c013823096b523f3e0f56db2d7e76009 --- /dev/null +++ b/assets/icons/paperclip.svg @@ -0,0 +1,3 @@ + + + diff --git a/crates/icons/src/icons.rs b/crates/icons/src/icons.rs index 0f05e58c27c48c37043fe90f64b4f03968b22752..4c026bc2b82deebb0cb24da40d476b2cd91bb012 100644 --- a/crates/icons/src/icons.rs +++ b/crates/icons/src/icons.rs @@ -165,6 +165,7 @@ pub enum IconName { Option, PageDown, PageUp, + Paperclip, Pencil, PencilUnavailable, Person, diff --git a/crates/rules_library/src/rules_library.rs b/crates/rules_library/src/rules_library.rs index c11dde072fe2a89ca3373f4490384709cff901e7..34dc0eaa2cc816d8b78bb0ac394993d0c6df78eb 100644 --- a/crates/rules_library/src/rules_library.rs +++ b/crates/rules_library/src/rules_library.rs @@ -22,8 +22,7 @@ use std::time::Duration; use theme::ThemeSettings; use title_bar::platform_title_bar::PlatformTitleBar; use ui::{ - Context, IconButtonShape, KeyBinding, ListItem, ListItemSpacing, ParentElement, Render, - SharedString, Styled, Tooltip, Window, div, prelude::*, + Divider, KeyBinding, ListItem, ListItemSpacing, ListSubHeader, Render, Tooltip, prelude::*, }; use util::{ResultExt, TryFutureExt}; use workspace::{Workspace, client_side_decorations}; @@ -136,6 +135,7 @@ pub fn open_rules_library( window_bounds: Some(WindowBounds::Windowed(bounds)), window_background: cx.theme().window_background_appearance(), window_decorations: Some(window_decorations), + window_min_size: Some(size(px(800.), px(600.))), // 4:3 Aspect Ratio ..Default::default() }, |window, cx| { @@ -179,10 +179,16 @@ struct RuleEditor { _subscriptions: Vec, } +enum RulePickerEntry { + Header(SharedString), + Rule(PromptMetadata), + Separator, +} + struct RulePickerDelegate { store: Entity, selected_index: usize, - matches: Vec, + filtered_entries: Vec, } enum RulePickerEvent { @@ -195,10 +201,10 @@ enum RulePickerEvent { impl EventEmitter for Picker {} impl PickerDelegate for RulePickerDelegate { - type ListItem = ListItem; + type ListItem = AnyElement; fn match_count(&self) -> usize { - self.matches.len() + self.filtered_entries.len() } fn no_matches_text(&self, _window: &mut Window, cx: &mut App) -> Option { @@ -215,16 +221,24 @@ impl PickerDelegate for RulePickerDelegate { } fn set_selected_index(&mut self, ix: usize, _: &mut Window, cx: &mut Context>) { - self.selected_index = ix; - if let Some(prompt) = self.matches.get(self.selected_index) { - cx.emit(RulePickerEvent::Selected { - prompt_id: prompt.id, - }); + self.selected_index = ix.min(self.filtered_entries.len().saturating_sub(1)); + + if let Some(RulePickerEntry::Rule(rule)) = self.filtered_entries.get(self.selected_index) { + cx.emit(RulePickerEvent::Selected { prompt_id: rule.id }); + } + + cx.notify(); + } + + fn can_select(&mut self, ix: usize, _: &mut Window, _: &mut Context>) -> bool { + match self.filtered_entries.get(ix) { + Some(RulePickerEntry::Rule(_)) => true, + Some(RulePickerEntry::Header(_)) | Some(RulePickerEntry::Separator) | None => false, } } fn placeholder_text(&self, _window: &mut Window, _cx: &mut App) -> Arc { - "Search...".into() + "Search…".into() } fn update_matches( @@ -235,24 +249,72 @@ impl PickerDelegate for RulePickerDelegate { ) -> Task<()> { let cancellation_flag = Arc::new(AtomicBool::default()); let search = self.store.read(cx).search(query, cancellation_flag, cx); - let prev_prompt_id = self.matches.get(self.selected_index).map(|mat| mat.id); + + let prev_prompt_id = self + .filtered_entries + .get(self.selected_index) + .and_then(|entry| { + if let RulePickerEntry::Rule(rule) = entry { + Some(rule.id) + } else { + None + } + }); + cx.spawn_in(window, async move |this, cx| { - let (matches, selected_index) = cx + let (filtered_entries, selected_index) = cx .background_spawn(async move { let matches = search.await; + let (default_rules, non_default_rules): (Vec<_>, Vec<_>) = + matches.iter().partition(|rule| rule.default); + + let mut filtered_entries = Vec::new(); + + if !default_rules.is_empty() { + filtered_entries.push(RulePickerEntry::Header("Default Rules".into())); + + for rule in default_rules { + filtered_entries.push(RulePickerEntry::Rule(rule.clone())); + } + + filtered_entries.push(RulePickerEntry::Separator); + } + + for rule in non_default_rules { + filtered_entries.push(RulePickerEntry::Rule(rule.clone())); + } + let selected_index = prev_prompt_id .and_then(|prev_prompt_id| { - matches.iter().position(|entry| entry.id == prev_prompt_id) + filtered_entries.iter().position(|entry| { + if let RulePickerEntry::Rule(rule) = entry { + rule.id == prev_prompt_id + } else { + false + } + }) }) - .unwrap_or(0); - (matches, selected_index) + .unwrap_or_else(|| { + filtered_entries + .iter() + .position(|entry| matches!(entry, RulePickerEntry::Rule(_))) + .unwrap_or(0) + }); + + (filtered_entries, selected_index) }) .await; this.update_in(cx, |this, window, cx| { - this.delegate.matches = matches; - this.delegate.set_selected_index(selected_index, window, cx); + this.delegate.filtered_entries = filtered_entries; + this.set_selected_index( + selected_index, + Some(picker::Direction::Down), + true, + window, + cx, + ); cx.notify(); }) .ok(); @@ -260,10 +322,8 @@ impl PickerDelegate for RulePickerDelegate { } fn confirm(&mut self, _secondary: bool, _: &mut Window, cx: &mut Context>) { - if let Some(prompt) = self.matches.get(self.selected_index) { - cx.emit(RulePickerEvent::Confirmed { - prompt_id: prompt.id, - }); + if let Some(RulePickerEntry::Rule(rule)) = self.filtered_entries.get(self.selected_index) { + cx.emit(RulePickerEvent::Confirmed { prompt_id: rule.id }); } } @@ -276,87 +336,115 @@ impl PickerDelegate for RulePickerDelegate { _: &mut Window, cx: &mut Context>, ) -> Option { - let rule = self.matches.get(ix)?; - let default = rule.default; - let prompt_id = rule.id; - - let element = ListItem::new(ix) - .inset(true) - .spacing(ListItemSpacing::Sparse) - .toggle_state(selected) - .child( - h_flex() - .h_5() - .line_height(relative(1.)) - .child(Label::new(rule.title.clone().unwrap_or("Untitled".into()))), - ) - .end_slot::(default.then(|| { - IconButton::new("toggle-default-rule", IconName::StarFilled) - .toggle_state(true) - .icon_color(Color::Accent) - .icon_size(IconSize::Small) - .shape(IconButtonShape::Square) - .tooltip(Tooltip::text("Remove from Default Rules")) - .on_click(cx.listener(move |_, _, _, cx| { - cx.emit(RulePickerEvent::ToggledDefault { prompt_id }) - })) - })) - .end_hover_slot( - h_flex() - .gap_1() - .child(if prompt_id.is_built_in() { - div() - .id("built-in-rule") - .child(Icon::new(IconName::FileLock).color(Color::Muted)) - .tooltip(move |window, cx| { - Tooltip::with_meta( - "Built-in rule", - None, - BUILT_IN_TOOLTIP_TEXT, - window, - cx, - ) - }) - .into_any() - } else { - IconButton::new("delete-rule", IconName::Trash) - .icon_color(Color::Muted) - .icon_size(IconSize::Small) - .shape(IconButtonShape::Square) - .tooltip(Tooltip::text("Delete Rule")) - .on_click(cx.listener(move |_, _, _, cx| { - cx.emit(RulePickerEvent::Deleted { prompt_id }) - })) - .into_any_element() - }) - .child( - IconButton::new("toggle-default-rule", IconName::Star) - .toggle_state(default) - .selected_icon(IconName::StarFilled) - .icon_color(if default { Color::Accent } else { Color::Muted }) + match self.filtered_entries.get(ix)? { + RulePickerEntry::Header(title) => Some( + ListSubHeader::new(title.clone()) + .end_slot( + IconButton::new("info", IconName::Info) + .style(ButtonStyle::Transparent) .icon_size(IconSize::Small) - .shape(IconButtonShape::Square) - .map(|this| { - if default { - this.tooltip(Tooltip::text("Remove from Default Rules")) + .icon_color(Color::Muted) + .tooltip(Tooltip::text( + "Default Rules are attached by default with every new thread.", + )) + .into_any_element(), + ) + .inset(true) + .into_any_element(), + ), + RulePickerEntry::Separator => Some( + h_flex() + .py_1() + .child(Divider::horizontal()) + .into_any_element(), + ), + RulePickerEntry::Rule(rule) => { + let default = rule.default; + let prompt_id = rule.id; + + Some( + ListItem::new(ix) + .inset(true) + .spacing(ListItemSpacing::Sparse) + .toggle_state(selected) + .child( + h_flex() + .h_5() + .line_height(relative(1.)) + .child(Label::new(rule.title.clone().unwrap_or("Untitled".into()))), + ) + .end_slot::(default.then(|| { + IconButton::new("toggle-default-rule", IconName::Paperclip) + .toggle_state(true) + .icon_color(Color::Accent) + .icon_size(IconSize::Small) + .tooltip(Tooltip::text("Remove from Default Rules")) + .on_click(cx.listener(move |_, _, _, cx| { + cx.emit(RulePickerEvent::ToggledDefault { prompt_id }) + })) + })) + .end_hover_slot( + h_flex() + .child(if prompt_id.is_built_in() { + div() + .id("built-in-rule") + .child(Icon::new(IconName::FileLock).color(Color::Muted)) + .tooltip(move |window, cx| { + Tooltip::with_meta( + "Built-in rule", + None, + BUILT_IN_TOOLTIP_TEXT, + window, + cx, + ) + }) + .into_any() } else { - this.tooltip(move |window, cx| { - Tooltip::with_meta( - "Add to Default Rules", - None, - "Always included in every thread.", - window, - cx, - ) - }) - } - }) - .on_click(cx.listener(move |_, _, _, cx| { - cx.emit(RulePickerEvent::ToggledDefault { prompt_id }) - })), - ), - ); - Some(element) + IconButton::new("delete-rule", IconName::Trash) + .icon_color(Color::Muted) + .icon_size(IconSize::Small) + .tooltip(Tooltip::text("Delete Rule")) + .on_click(cx.listener(move |_, _, _, cx| { + cx.emit(RulePickerEvent::Deleted { prompt_id }) + })) + .into_any_element() + }) + .child( + IconButton::new("toggle-default-rule", IconName::Plus) + .selected_icon(IconName::Dash) + .toggle_state(default) + .icon_size(IconSize::Small) + .icon_color(if default { + Color::Accent + } else { + Color::Muted + }) + .map(|this| { + if default { + this.tooltip(Tooltip::text( + "Remove from Default Rules", + )) + } else { + this.tooltip(move |window, cx| { + Tooltip::with_meta( + "Add to Default Rules", + None, + "Always included in every thread.", + window, + cx, + ) + }) + } + }) + .on_click(cx.listener(move |_, _, _, cx| { + cx.emit(RulePickerEvent::ToggledDefault { prompt_id }) + })), + ), + ) + .into_any_element(), + ) + } + } } fn render_editor( @@ -387,7 +475,7 @@ impl RulesLibrary { window: &mut Window, cx: &mut Context, ) -> Self { - let (selected_index, matches) = if let Some(rule_to_select) = rule_to_select { + let (_selected_index, _matches) = if let Some(rule_to_select) = rule_to_select { let matches = store.read(cx).all_prompt_metadata(); let selected_index = matches .iter() @@ -399,19 +487,20 @@ impl RulesLibrary { (0, vec![]) }; - let delegate = RulePickerDelegate { + let picker_delegate = RulePickerDelegate { store: store.clone(), - selected_index, - matches, + selected_index: 0, + filtered_entries: Vec::new(), }; let picker = cx.new(|cx| { - let picker = Picker::uniform_list(delegate, window, cx) + let picker = Picker::list(picker_delegate, window, cx) .modal(false) .max_height(None); picker.focus(window, cx); picker }); + Self { title_bar: if !cfg!(target_os = "macos") { Some(cx.new(|cx| PlatformTitleBar::new("rules-library-title-bar", cx))) @@ -701,14 +790,22 @@ impl RulesLibrary { if let Some(prompt_id) = prompt_id { if picker .delegate - .matches + .filtered_entries .get(picker.delegate.selected_index()) - .is_none_or(|old_selected_prompt| old_selected_prompt.id != prompt_id) - && let Some(ix) = picker - .delegate - .matches - .iter() - .position(|mat| mat.id == prompt_id) + .is_none_or(|old_selected_prompt| { + if let RulePickerEntry::Rule(rule) = old_selected_prompt { + rule.id != prompt_id + } else { + true + } + }) + && let Some(ix) = picker.delegate.filtered_entries.iter().position(|mat| { + if let RulePickerEntry::Rule(rule) = mat { + rule.id == prompt_id + } else { + false + } + }) { picker.set_selected_index(ix, None, true, window, cx); } @@ -1014,8 +1111,6 @@ impl RulesLibrary { .justify_end() .child( IconButton::new("new-rule", IconName::Plus) - .style(ButtonStyle::Transparent) - .shape(IconButtonShape::Square) .tooltip(move |window, cx| { Tooltip::for_action("New Rule", &NewRule, window, cx) }) @@ -1059,13 +1154,15 @@ impl RulesLibrary { h_flex() .group("active-editor-header") .pt_2() - .px_2p5() + .pl_1p5() + .pr_2p5() .gap_2() .justify_between() .child( div() .w_full() .on_action(cx.listener(Self::move_down_from_title)) + .pl_1() .border_1() .border_color(transparent_black()) .rounded_sm() @@ -1107,7 +1204,6 @@ impl RulesLibrary { h_flex() .h_full() .flex_shrink_0() - .gap(DynamicSpacing::Base04.rems(cx)) .children(rule_editor.token_count.map(|token_count| { let token_count: SharedString = token_count.to_string().into(); @@ -1160,7 +1256,6 @@ impl RulesLibrary { .into_any() } else { IconButton::new("delete-rule", IconName::Trash) - .icon_size(IconSize::Small) .tooltip(move |window, cx| { Tooltip::for_action( "Delete Rule", @@ -1177,7 +1272,6 @@ impl RulesLibrary { }) .child( IconButton::new("duplicate-rule", IconName::BookCopy) - .icon_size(IconSize::Small) .tooltip(move |window, cx| { Tooltip::for_action( "Duplicate Rule", @@ -1194,38 +1288,41 @@ impl RulesLibrary { }), ) .child( - IconButton::new("toggle-default-rule", IconName::Star) - .icon_size(IconSize::Small) - .toggle_state(rule_metadata.default) - .selected_icon(IconName::StarFilled) - .icon_color(if rule_metadata.default { - Color::Accent + IconButton::new( + "toggle-default-rule", + IconName::Paperclip, + ) + .toggle_state(rule_metadata.default) + .icon_color(if rule_metadata.default { + Color::Accent + } else { + Color::Muted + }) + .map(|this| { + if rule_metadata.default { + this.tooltip(Tooltip::text( + "Remove from Default Rules", + )) } else { - Color::Muted - }) - .map(|this| { - if rule_metadata.default { - this.tooltip(Tooltip::text( - "Remove from Default Rules", - )) - } else { - this.tooltip(move |window, cx| { - Tooltip::with_meta( - "Add to Default Rules", - None, - "Always included in every thread.", - window, - cx, - ) - }) - } - }) - .on_click(|_, window, cx| { + this.tooltip(move |window, cx| { + Tooltip::with_meta( + "Add to Default Rules", + None, + "Always included in every thread.", + window, + cx, + ) + }) + } + }) + .on_click( + |_, window, cx| { window.dispatch_action( Box::new(ToggleDefaultRule), cx, ); - }), + }, + ), ), ), ) @@ -1234,8 +1331,8 @@ impl RulesLibrary { .on_action(cx.listener(Self::focus_picker)) .on_action(cx.listener(Self::inline_assist)) .on_action(cx.listener(Self::move_up_from_body)) - .flex_grow() .h_full() + .flex_grow() .child( h_flex() .py_2()