From 65138868670d71649a4f8a47fd9e507610c98215 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 16 May 2024 15:05:00 -0400 Subject: [PATCH] Don't scale context menus in editors with buffer font size (#11930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the changes in #11817, context menus within editors would get scaled by the `buffer_font_size` instead of the `ui_font_size`. This seems incorrect, as it results in context menus being sized inconsistently depending on what context they originate from. This PR makes it so that all context menus scale based on the `ui_font_size`. ### Before Screenshot 2024-05-16 at 2 43 19 PM ### After Screenshot 2024-05-16 at 2 43 01 PM Release Notes: - Changed context menus in editors to no longer scale with `buffer_font_size`. --- crates/ui/src/components/context_menu.rs | 236 ++++++++++++----------- 1 file changed, 123 insertions(+), 113 deletions(-) diff --git a/crates/ui/src/components/context_menu.rs b/crates/ui/src/components/context_menu.rs index e4c42fe5eb8ab1267cdc9438e0c7dad9e1786c8a..32af2eb59b618ec6a059dc1aba02ac4d93cf7826 100644 --- a/crates/ui/src/components/context_menu.rs +++ b/crates/ui/src/components/context_menu.rs @@ -1,13 +1,15 @@ use crate::{ h_flex, prelude::*, v_flex, Icon, IconName, KeyBinding, Label, List, ListItem, ListSeparator, - ListSubHeader, + ListSubHeader, WithRemSize, }; use gpui::{ px, Action, AnyElement, AppContext, DismissEvent, EventEmitter, FocusHandle, FocusableView, IntoElement, Render, Subscription, View, VisualContext, }; use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev}; +use settings::Settings; use std::{rc::Rc, time::Duration}; +use theme::ThemeSettings; enum ContextMenuItem { Separator, @@ -264,126 +266,134 @@ impl ContextMenuItem { impl Render for ContextMenu { fn render(&mut self, cx: &mut ViewContext) -> impl IntoElement { + let ui_font_size = ThemeSettings::get_global(cx).ui_font_size; + div().occlude().elevation_2(cx).flex().flex_row().child( - v_flex() - .min_w(px(200.)) - .track_focus(&self.focus_handle) - .on_mouse_down_out(cx.listener(|this, _, cx| this.cancel(&menu::Cancel, cx))) - .key_context("menu") - .on_action(cx.listener(ContextMenu::select_first)) - .on_action(cx.listener(ContextMenu::handle_select_last)) - .on_action(cx.listener(ContextMenu::select_next)) - .on_action(cx.listener(ContextMenu::select_prev)) - .on_action(cx.listener(ContextMenu::confirm)) - .on_action(cx.listener(ContextMenu::cancel)) - .when(!self.delayed, |mut el| { - for item in self.items.iter() { - if let ContextMenuItem::Entry { - action: Some(action), - .. - } = item - { - el = el.on_boxed_action( - &**action, - cx.listener(ContextMenu::on_action_dispatch), - ); + WithRemSize::new(ui_font_size).child( + v_flex() + .min_w(px(200.)) + .track_focus(&self.focus_handle) + .on_mouse_down_out(cx.listener(|this, _, cx| this.cancel(&menu::Cancel, cx))) + .key_context("menu") + .on_action(cx.listener(ContextMenu::select_first)) + .on_action(cx.listener(ContextMenu::handle_select_last)) + .on_action(cx.listener(ContextMenu::select_next)) + .on_action(cx.listener(ContextMenu::select_prev)) + .on_action(cx.listener(ContextMenu::confirm)) + .on_action(cx.listener(ContextMenu::cancel)) + .when(!self.delayed, |mut el| { + for item in self.items.iter() { + if let ContextMenuItem::Entry { + action: Some(action), + .. + } = item + { + el = el.on_boxed_action( + &**action, + cx.listener(ContextMenu::on_action_dispatch), + ); + } } - } - el - }) - .flex_none() - .child(List::new().children(self.items.iter_mut().enumerate().map( - |(ix, item)| { - match item { - ContextMenuItem::Separator => ListSeparator.into_any_element(), - ContextMenuItem::Header(header) => ListSubHeader::new(header.clone()) - .inset(true) - .into_any_element(), - ContextMenuItem::Entry { - toggled, - label, - handler, - icon, - action, - } => { - let handler = handler.clone(); - let menu = cx.view().downgrade(); - - let label_element = if let Some(icon) = icon { - h_flex() - .gap_1() - .child(Label::new(label.clone())) - .child(Icon::new(*icon)) + el + }) + .flex_none() + .child(List::new().children(self.items.iter_mut().enumerate().map( + |(ix, item)| { + match item { + ContextMenuItem::Separator => ListSeparator.into_any_element(), + ContextMenuItem::Header(header) => { + ListSubHeader::new(header.clone()) + .inset(true) .into_any_element() - } else { - Label::new(label.clone()).into_any_element() - }; + } + ContextMenuItem::Entry { + toggled, + label, + handler, + icon, + action, + } => { + let handler = handler.clone(); + let menu = cx.view().downgrade(); - ListItem::new(ix) - .inset(true) - .selected(Some(ix) == self.selected_index) - .when_some(*toggled, |list_item, toggled| { - list_item.start_slot(if toggled { - v_flex().flex_none().child( - Icon::new(IconName::Check).color(Color::Accent), - ) - } else { - v_flex().flex_none().size(IconSize::default().rems()) - }) - }) - .child( + let label_element = if let Some(icon) = icon { h_flex() - .w_full() - .justify_between() - .child(label_element) - .debug_selector(|| format!("MENU_ITEM-{}", label)) - .children(action.as_ref().and_then(|action| { - self.action_context - .as_ref() - .map(|focus| { - KeyBinding::for_action_in( - &**action, focus, cx, - ) - }) - .unwrap_or_else(|| { - KeyBinding::for_action(&**action, cx) - }) - .map(|binding| div().ml_1().child(binding)) - })), - ) - .on_click(move |_, cx| { - handler(cx); - menu.update(cx, |menu, cx| { - menu.clicked = true; - cx.emit(DismissEvent); + .gap_1() + .child(Label::new(label.clone())) + .child(Icon::new(*icon)) + .into_any_element() + } else { + Label::new(label.clone()).into_any_element() + }; + + ListItem::new(ix) + .inset(true) + .selected(Some(ix) == self.selected_index) + .when_some(*toggled, |list_item, toggled| { + list_item.start_slot(if toggled { + v_flex().flex_none().child( + Icon::new(IconName::Check).color(Color::Accent), + ) + } else { + v_flex() + .flex_none() + .size(IconSize::default().rems()) + }) }) - .ok(); - }) - .into_any_element() - } - ContextMenuItem::CustomEntry { - entry_render, - handler, - } => { - let handler = handler.clone(); - let menu = cx.view().downgrade(); - ListItem::new(ix) - .inset(true) - .selected(Some(ix) == self.selected_index) - .on_click(move |_, cx| { - handler(cx); - menu.update(cx, |menu, cx| { - menu.clicked = true; - cx.emit(DismissEvent); + .child( + h_flex() + .w_full() + .justify_between() + .child(label_element) + .debug_selector(|| format!("MENU_ITEM-{}", label)) + .children(action.as_ref().and_then(|action| { + self.action_context + .as_ref() + .map(|focus| { + KeyBinding::for_action_in( + &**action, focus, cx, + ) + }) + .unwrap_or_else(|| { + KeyBinding::for_action(&**action, cx) + }) + .map(|binding| div().ml_1().child(binding)) + })), + ) + .on_click(move |_, cx| { + handler(cx); + menu.update(cx, |menu, cx| { + menu.clicked = true; + cx.emit(DismissEvent); + }) + .ok(); + }) + .into_any_element() + } + ContextMenuItem::CustomEntry { + entry_render, + handler, + } => { + let handler = handler.clone(); + let menu = cx.view().downgrade(); + ListItem::new(ix) + .inset(true) + .selected(Some(ix) == self.selected_index) + .on_click(move |_, cx| { + handler(cx); + menu.update(cx, |menu, cx| { + menu.clicked = true; + cx.emit(DismissEvent); + }) + .ok(); }) - .ok(); - }) - .child(entry_render(cx)) - .into_any_element() + .child(entry_render(cx)) + .into_any_element() + } } - } - }, - ))), + }, + ))), + ), ) } }