From df8adc8b11632f99639b69680ad24bffe1d5c5c0 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Wed, 12 Feb 2025 16:46:42 -0600 Subject: [PATCH] Fix linux zeta modifiers display (#24764) Improves rendering of Zeta keybind shortcuts on Linux Before: ![image](https://github.com/user-attachments/assets/9b6a61f7-dade-480f-a864-acdcede05957) After: (with muting modifier changes merged) ![image](https://github.com/user-attachments/assets/dd616d29-ac2e-4c8b-bf9b-5d74f8e4f1c4) Release Notes: - N/A --------- Co-authored-by: Michael Co-authored-by: Agus --- Cargo.lock | 1 + crates/editor/src/editor.rs | 57 +++++++++++++++----------- crates/editor/src/element.rs | 2 +- crates/ui/Cargo.toml | 5 ++- crates/ui/src/components/keybinding.rs | 51 ++++++++++++----------- crates/util/src/util.rs | 22 ++++++++++ 6 files changed, 89 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a5648efab0..038a102e10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14414,6 +14414,7 @@ dependencies = [ "strum", "theme", "ui_macros", + "util", "windows 0.58.0", ] diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9931dca5b0..66b76991a6 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -161,7 +161,7 @@ use sum_tree::TreeMap; use text::{BufferId, OffsetUtf16, Rope}; use theme::{ActiveTheme, PlayerColor, StatusColors, SyntaxTheme, ThemeColors, ThemeSettings}; use ui::{ - h_flex, prelude::*, ButtonSize, ButtonStyle, Disclosure, IconButton, IconName, IconSize, + h_flex, prelude::*, ButtonSize, ButtonStyle, Disclosure, IconButton, IconName, IconSize, Key, Tooltip, }; use util::{defer, maybe, post_inc, RangeExt, ResultExt, TakeUntilExt, TryFutureExt}; @@ -5657,29 +5657,39 @@ impl Editor { fn render_edit_prediction_accept_keybind(&self, window: &mut Window, cx: &App) -> Option
{ let accept_binding = self.accept_edit_prediction_keybind(window, cx); let accept_keystroke = accept_binding.keystroke()?; - let colors = cx.theme().colors(); - let accent_color = colors.text_accent; - let editor_bg_color = colors.editor_background; - let bg_color = editor_bg_color.blend(accent_color.opacity(0.1)); + + let is_platform_style_mac = PlatformStyle::platform() == PlatformStyle::Mac; + + let modifiers_color = if accept_keystroke.modifiers == window.modifiers() { + Color::Accent + } else { + Color::Muted + }; h_flex() .px_0p5() - .gap_1() - .bg(bg_color) + .when(is_platform_style_mac, |parent| parent.gap_0p5()) .font(theme::ThemeSettings::get_global(cx).buffer_font.clone()) .text_size(TextSize::XSmall.rems(cx)) - .children(ui::render_modifiers( + .child(h_flex().children(ui::render_modifiers( &accept_keystroke.modifiers, PlatformStyle::platform(), - Some(if accept_keystroke.modifiers == window.modifiers() { - Color::Accent - } else { - Color::Muted - }), + Some(modifiers_color), Some(IconSize::XSmall.rems().into()), - false, - )) - .child(accept_keystroke.key.clone()) + true, + ))) + .when(is_platform_style_mac, |parent| { + parent.child(accept_keystroke.key.clone()) + }) + .when(!is_platform_style_mac, |parent| { + parent.child( + Key::new( + util::capitalize(&accept_keystroke.key), + Some(Color::Default), + ) + .size(Some(IconSize::XSmall.rems().into())), + ) + }) .into() } @@ -5808,13 +5818,13 @@ impl Editor { }, ) .child(Label::new("Hold").size(LabelSize::Small)) - .children(ui::render_modifiers( + .child(h_flex().children(ui::render_modifiers( &accept_keystroke.modifiers, PlatformStyle::platform(), Some(Color::Default), Some(IconSize::Small.rems().into()), - true, - )) + false, + ))) .into_any(), ); } @@ -5858,6 +5868,7 @@ impl Editor { let has_completion = self.active_inline_completion.is_some(); + let is_platform_style_mac = PlatformStyle::platform() == PlatformStyle::Mac; Some( h_flex() .min_w(min_width) @@ -5886,8 +5897,8 @@ impl Editor { .child( h_flex() .font(theme::ThemeSettings::get_global(cx).buffer_font.clone()) - .gap_1() - .children(ui::render_modifiers( + .when(is_platform_style_mac, |parent| parent.gap_1()) + .child(h_flex().children(ui::render_modifiers( &accept_keystroke.modifiers, PlatformStyle::platform(), Some(if !has_completion { @@ -5896,8 +5907,8 @@ impl Editor { Color::Default }), None, - true, - )), + false, + ))), ) .child(Label::new("Preview").into_any_element()) .opacity(if has_completion { 1.0 } else { 0.4 }), diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 9a5b7da6ff..4cea82664b 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -3815,7 +3815,7 @@ impl EditorElement { let mut element = h_flex() .items_start() .child( - div() + h_flex() .bg(cx.theme().colors().editor_background) .border(BORDER_WIDTH) .shadow_sm() diff --git a/crates/ui/Cargo.toml b/crates/ui/Cargo.toml index ba7c89a8a6..c15f4c2c1d 100644 --- a/crates/ui/Cargo.toml +++ b/crates/ui/Cargo.toml @@ -16,7 +16,7 @@ path = "src/ui.rs" chrono.workspace = true component.workspace = true gpui.workspace = true -itertools = { workspace = true, optional = true } +itertools.workspace = true linkme.workspace = true menu.workspace = true serde.workspace = true @@ -26,13 +26,14 @@ story = { workspace = true, optional = true } strum = { workspace = true, features = ["derive"] } theme.workspace = true ui_macros.workspace = true +util.workspace = true [target.'cfg(windows)'.dependencies] windows.workspace = true [features] default = [] -stories = ["dep:itertools", "dep:story"] +stories = ["dep:story"] # cargo-machete doesn't understand that linkme is used in the component macro [package.metadata.cargo-machete] diff --git a/crates/ui/src/components/keybinding.rs b/crates/ui/src/components/keybinding.rs index 50083c251a..8883dce899 100644 --- a/crates/ui/src/components/keybinding.rs +++ b/crates/ui/src/components/keybinding.rs @@ -92,7 +92,7 @@ impl RenderOnce for KeyBinding { self.platform_style, None, self.size, - false, + true, )) .map(|el| { el.child(render_key(&keystroke, self.platform_style, None, self.size)) @@ -110,7 +110,7 @@ pub fn render_key( let key_icon = icon_for_key(keystroke, platform_style); match key_icon { Some(icon) => KeyIcon::new(icon, color).size(size).into_any_element(), - None => Key::new(capitalize(&keystroke.key), color) + None => Key::new(util::capitalize(&keystroke.key), color) .size(size) .into_any_element(), } @@ -145,10 +145,12 @@ pub fn render_modifiers( platform_style: PlatformStyle, color: Option, size: Option, - standalone: bool, + trailing_separator: bool, ) -> impl Iterator { + #[derive(Clone)] enum KeyOrIcon { Key(&'static str), + Plus, Icon(IconName), } @@ -200,23 +202,34 @@ pub fn render_modifiers( .into_iter() .filter(|modifier| modifier.enabled) .collect::>(); - let last_ix = filtered.len().saturating_sub(1); - filtered + let platform_keys = filtered .into_iter() - .enumerate() - .flat_map(move |(ix, modifier)| match platform_style { - PlatformStyle::Mac => vec![modifier.mac], - PlatformStyle::Linux if standalone && ix == last_ix => vec![modifier.linux], - PlatformStyle::Linux => vec![modifier.linux, KeyOrIcon::Key("+")], - PlatformStyle::Windows if standalone && ix == last_ix => { - vec![modifier.windows] - } - PlatformStyle::Windows => vec![modifier.windows, KeyOrIcon::Key("+")], + .map(move |modifier| match platform_style { + PlatformStyle::Mac => Some(modifier.mac), + PlatformStyle::Linux => Some(modifier.linux), + PlatformStyle::Windows => Some(modifier.windows), + }); + + let separator = match platform_style { + PlatformStyle::Mac => None, + PlatformStyle::Linux => Some(KeyOrIcon::Plus), + PlatformStyle::Windows => Some(KeyOrIcon::Plus), + }; + + let platform_keys = itertools::intersperse(platform_keys, separator.clone()); + + platform_keys + .chain(if modifiers.modified() && trailing_separator { + Some(separator) + } else { + None }) + .flatten() .map(move |key_or_icon| match key_or_icon { KeyOrIcon::Key(key) => Key::new(key, color).size(size).into_any_element(), KeyOrIcon::Icon(icon) => KeyIcon::new(icon, color).size(size).into_any_element(), + KeyOrIcon::Plus => "+".into_any_element(), }) } @@ -389,7 +402,7 @@ pub fn text_for_keystroke(keystroke: &Keystroke, platform_style: PlatformStyle) let key = match keystroke.key.as_str() { "pageup" => "PageUp", "pagedown" => "PageDown", - key => &capitalize(key), + key => &util::capitalize(key), }; text.push_str(key); @@ -397,14 +410,6 @@ pub fn text_for_keystroke(keystroke: &Keystroke, platform_style: PlatformStyle) text } -fn capitalize(str: &str) -> String { - let mut chars = str.chars(); - match chars.next() { - None => String::new(), - Some(first_char) => first_char.to_uppercase().collect::() + chars.as_str(), - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 9fd802a09c..79abd1065b 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -787,6 +787,28 @@ impl<'a> PartialOrd for NumericPrefixWithSuffix<'a> { } } +/// Capitalizes the first character of a string. +/// +/// This function takes a string slice as input and returns a new `String` with the first character +/// capitalized. +/// +/// # Examples +/// +/// ``` +/// use util::capitalize; +/// +/// assert_eq!(capitalize("hello"), "Hello"); +/// assert_eq!(capitalize("WORLD"), "WORLD"); +/// assert_eq!(capitalize(""), ""); +/// ``` +pub fn capitalize(str: &str) -> String { + let mut chars = str.chars(); + match chars.next() { + None => String::new(), + Some(first_char) => first_char.to_uppercase().collect::() + chars.as_str(), + } +} + fn emoji_regex() -> &'static Regex { static EMOJI_REGEX: LazyLock = LazyLock::new(|| Regex::new("(\\p{Emoji}|\u{200D})").unwrap());