From 549cfe02be2f2ce3fff74dd24196e2022cc2eba2 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Thu, 31 Oct 2024 14:35:13 +0100 Subject: [PATCH] fix(ux): configuration fixes (#3713) * fix(startup): try create config folder if it doesn't exist * fix(configuration): tab bar ui * fix(configuration): rebind ctrl-s to ctrl-a * fix(configuration): remove extra rebinding leaders screen --- .../configuration/src/presets_screen.rs | 56 +++++++++++-------- .../src/rebind_leaders_screen.rs | 24 ++++---- .../configuration/src/ui_components.rs | 2 +- zellij-utils/src/home.rs | 9 +++ zellij-utils/src/input/config.rs | 5 ++ 5 files changed, 61 insertions(+), 35 deletions(-) diff --git a/default-plugins/configuration/src/presets_screen.rs b/default-plugins/configuration/src/presets_screen.rs index 0551c895..9a29b790 100644 --- a/default-plugins/configuration/src/presets_screen.rs +++ b/default-plugins/configuration/src/presets_screen.rs @@ -85,7 +85,7 @@ impl PresetsScreen { self.reset_selected_index(); } should_render = true; - } else if key.bare_key == BareKey::Char('s') && key.has_modifiers(&[KeyModifier::Ctrl]) { + } else if key.bare_key == BareKey::Char('a') && key.has_modifiers(&[KeyModifier::Ctrl]) { if let Some(selected_index) = self.take_selected_index() { let write_to_disk = true; self.reconfigure(selected_index, write_to_disk); @@ -93,12 +93,19 @@ impl PresetsScreen { should_render = true; } } else if key.bare_key == BareKey::Char('l') && key.has_no_modifiers() { - self.rebind_leaders_screen = Some( - RebindLeadersScreen::default() - .with_rebinding_for_presets() - .with_mode_info(self.latest_mode_info.clone()), - ); - should_render = true; + // for the time being this screen has been disabled because it was deemed too confusing + // and its use-cases are very limited (it's possible to achieve the same results by + // applying a preset and then rebinding the leader keys) + // + // the code is left here in case someone feels strongly about implementing this on + // their own, and because at the time of writing I'm a little ambiguous about this + // decision. At some point it should be refactored away + // self.rebind_leaders_screen = Some( + // RebindLeadersScreen::default() + // .with_rebinding_for_presets() + // .with_mode_info(self.latest_mode_info.clone()), + // ); + // should_render = true; } else if (key.bare_key == BareKey::Esc && key.has_no_modifiers()) || key.is_key_with_ctrl_modifier(BareKey::Char('c')) { @@ -149,9 +156,16 @@ impl PresetsScreen { should_render = true; } } else if key.bare_key == BareKey::Char('l') && key.has_no_modifiers() { - self.rebind_leaders_screen = - Some(RebindLeadersScreen::default().with_rebinding_for_presets()); - should_render = true; + // for the time being this screen has been disabled because it was deemed too confusing + // and its use-cases are very limited (it's possible to achieve the same results by + // applying a preset and then rebinding the leader keys) + // + // the code is left here in case someone feels strongly about implementing this on + // their own, and because at the time of writing I'm a little ambiguous about this + // decision. At some point it should be refactored away + // self.rebind_leaders_screen = + // Some(RebindLeadersScreen::default().with_rebinding_for_presets()); + // should_render = true; } else if (key.bare_key == BareKey::Esc && key.has_no_modifiers()) || key.is_key_with_ctrl_modifier(BareKey::Char('c')) { @@ -730,16 +744,14 @@ impl PresetsScreen { }; } fn render_help_text_setup_wizard(&self, rows: usize, cols: usize) { - let full_help_text = - "Help: <↓↑> - navigate, - apply & save, - change leaders, - close"; - let short_help_text = "Help: <↓↑> / / / "; + let full_help_text = "Help: <↓↑> - navigate, - apply & save, - close"; + let short_help_text = "Help: <↓↑> / / "; if cols >= full_help_text.chars().count() { print_text_with_coordinates( Text::new(full_help_text) .color_range(2, 6..10) .color_range(2, 23..30) - .color_range(2, 47..50) - .color_range(2, 69..74), + .color_range(2, 47..=50), 0, rows, None, @@ -750,8 +762,7 @@ impl PresetsScreen { Text::new(short_help_text) .color_range(2, 6..10) .color_range(2, 13..20) - .color_range(2, 23..26) - .color_range(2, 29..34), + .color_range(2, 23..=27), 0, rows, None, @@ -794,16 +805,16 @@ impl PresetsScreen { } } fn render_help_text_main(&self, rows: usize, cols: usize) { - let full_help_text = "Help: <↓↑> - navigate, - apply, - apply & save, - leaders, - close"; - let short_help_text = "Help: <↓↑> / / / / "; + let full_help_text = + "Help: <↓↑> - navigate, - apply, - apply & save, - close"; + let short_help_text = "Help: <↓↑> / / / "; if cols >= full_help_text.chars().count() { print_text_with_coordinates( Text::new(full_help_text) .color_range(2, 6..10) .color_range(2, 23..30) .color_range(2, 40..48) - .color_range(2, 65..68) - .color_range(2, 80..85), + .color_range(2, 65..=69), 0, rows, None, @@ -815,8 +826,7 @@ impl PresetsScreen { .color_range(2, 6..10) .color_range(2, 13..20) .color_range(2, 23..31) - .color_range(2, 34..37) - .color_range(2, 40..45), + .color_range(2, 34..=38), 0, rows, None, diff --git a/default-plugins/configuration/src/rebind_leaders_screen.rs b/default-plugins/configuration/src/rebind_leaders_screen.rs index b0da0a60..f87f3cfa 100644 --- a/default-plugins/configuration/src/rebind_leaders_screen.rs +++ b/default-plugins/configuration/src/rebind_leaders_screen.rs @@ -47,10 +47,12 @@ impl Default for RebindLeadersScreen { } impl RebindLeadersScreen { - pub fn with_rebinding_for_presets(mut self) -> Self { - self.is_rebinding_for_presets = true; - self - } + // temporarily commented out for the time being because the extra leaders screen was deemed a bit + // confusing, see commend in key + // pub fn with_rebinding_for_presets(mut self) -> Self { + // self.is_rebinding_for_presets = true; + // self + // } pub fn with_mode_info(mut self, latest_mode_info: Option) -> Self { self.latest_mode_info = latest_mode_info; self.hard_reset_ui_state(); @@ -558,11 +560,11 @@ impl RebindLeadersScreen { if self.is_rebinding_for_presets { return self.render_help_text_for_presets_rebinding(rows, cols); } - let help_text_long = "Help: <←↓↑→> - navigate, - select, - apply, - save, - reset, - close"; - let help_text_medium = "Help: <←↓↑→/SPACE> - navigate/select, - apply/save, - reset, - close"; + let help_text_long = "Help: <←↓↑→> - navigate, - select, - apply, - save, - reset, - close"; + let help_text_medium = "Help: <←↓↑→/SPACE> - navigate/select, - apply/save, - reset, - close"; let help_text_short = - "Help: <←↓↑→>// select/ save/ reset/"; - let help_text_minimum = "<←↓↑→>/////"; + "Help: <←↓↑→>// select/ save/ reset/"; + let help_text_minimum = "<←↓↑→>/////"; if cols >= help_text_long.chars().count() { print_text_with_coordinates( Text::new(help_text_long) @@ -698,8 +700,8 @@ impl RebindLeadersScreen { } fn handle_default_preset_key(&mut self, key: KeyWithModifier) -> bool { let should_render = true; - if key.bare_key == BareKey::Insert - && key.has_no_modifiers() + if key.bare_key == BareKey::Char('a') + && key.has_modifiers(&[KeyModifier::Ctrl]) && !self.is_rebinding_for_presets { let write_to_disk = true; @@ -1201,7 +1203,7 @@ impl RebindLeadersScreen { self.is_rebinding_for_presets = is_rebinding_for_presets; } fn handle_unlock_first_key(&mut self, key: KeyWithModifier) -> bool { - if key.bare_key == BareKey::Char('s') + if key.bare_key == BareKey::Char('a') && key.has_modifiers(&[KeyModifier::Ctrl]) && !self.is_rebinding_for_presets { diff --git a/default-plugins/configuration/src/ui_components.rs b/default-plugins/configuration/src/ui_components.rs index d34488d4..4ccccf9c 100644 --- a/default-plugins/configuration/src/ui_components.rs +++ b/default-plugins/configuration/src/ui_components.rs @@ -35,7 +35,7 @@ pub fn top_tab_menu(cols: usize, current_screen: &Screen, colors: &Palette) { } let switch_key = Text::new("").color_range(3, ..).opaque(); print_text_with_coordinates(switch_key, 0, 0, None, None); - print!("\u{1b}[{};{}H{}", 0, starting_positions.0 - 1, bg_color); + print!("\u{1b}[{};{}H{}", 0, starting_positions.0, bg_color); print_ribbon_with_coordinates(first_ribbon, starting_positions.0, 0, None, None); print_ribbon_with_coordinates(second_ribbon, starting_positions.1, 0, None, None); } diff --git a/zellij-utils/src/home.rs b/zellij-utils/src/home.rs index e0f12399..9bde367c 100644 --- a/zellij-utils/src/home.rs +++ b/zellij-utils/src/home.rs @@ -67,6 +67,15 @@ pub fn home_config_dir() -> Option { } } +pub fn try_create_home_config_dir() { + if let Some(user_dirs) = BaseDirs::new() { + let config_dir = user_dirs.home_dir().join(CONFIG_LOCATION); + if let Err(e) = std::fs::create_dir_all(config_dir) { + log::error!("Failed to create config dir: {:?}", e); + } + } +} + pub fn get_layout_dir(config_dir: Option) -> Option { config_dir.map(|dir| dir.join("layouts")) } diff --git a/zellij-utils/src/input/config.rs b/zellij-utils/src/input/config.rs index 0e7547d8..8075ea0d 100644 --- a/zellij-utils/src/input/config.rs +++ b/zellij-utils/src/input/config.rs @@ -291,6 +291,11 @@ impl Config { } // returns true if the config was not previouly written to disk and we successfully wrote it pub fn write_config_to_disk_if_it_does_not_exist(config: String, opts: &CliArgs) -> bool { + if opts.config.is_none() { + // if a config file path wasn't explicitly specified, we try to create the default + // config folder + home::try_create_home_config_dir(); + } match Config::config_file_path(opts) { Some(config_file_path) => { if config_file_path.exists() {