oak-team/oak-gpui
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix keybind hints flickering in certain scenarios (#40927)

Browse Source 
Closes #39172

This refactors when we resolve UI keybindings in an effort to reduce
flickering whilst painting these: Previously, we would always resolve
these upon creating the binding. This could lead to cases where the
corresponding context was not yet available and no binding could be
resolved, even if the binding was then available on the next presented
frame. Following that, on the next rerender of whatever requested this
keybinding, the keybind for that context would then be found, we would
render that and then also win a layout shift in that process, as we went
from nothing rendered to something rendered between these frames.

With these changes, this now happens less often, because we only look
for the keybinding once the context can actually be resolved in the
window.

| Before | After | 
| --- | --- |
|
https://github.com/user-attachments/assets/adebf8ac-217d-4c7f-ae5a-bab3aa0b0ee8
|
https://github.com/user-attachments/assets/70a82b4b-488f-4a9f-94d7-b6d0a49aada9
|

Also reduced cloning in the keymap editor in this process, since that
requiered changing due to this anyway.

Release Notes:

- Fixed some cases where keybinds would appear with a slight delay,
causing a flicker in the process
This commit is contained in:
Finn Evers
2025-10-22 19:52:38 +00:00
committed by GitHub
parent ed5b9a4705
commit f393138711
80 changed files with 662 additions and 975 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/keymap_editor/src/keymap_editor.rs
+39 -32
View File
@@ -1,6 +1,7 @@
use std::{
cmp::{self},
ops::{Not as _, Range},
rc::Rc,
sync::Arc,
time::Duration,
};
@@ -173,7 +174,7 @@ impl FilterState {
#[derive(Debug, Default, PartialEq, Eq, Clone, Hash)]
struct ActionMapping {
keystrokes: Vec<KeybindingKeystroke>,
keystrokes: Rc<[KeybindingKeystroke]>,
context: Option<SharedString>,
}
@@ -235,7 +236,7 @@ struct ConflictState {
}
type ConflictKeybindMapping = HashMap<
Vec<KeybindingKeystroke>,
Rc<[KeybindingKeystroke]>,
Vec<(
Option<gpui::KeyBindingContextPredicate>,
Vec<ConflictOrigin>,
@@ -257,7 +258,7 @@ impl ConflictState {
.context
.and_then(|ctx| gpui::KeyBindingContextPredicate::parse(&ctx).ok());
let entry = action_keybind_mapping
.entry(mapping.keystrokes)
.entry(mapping.keystrokes.clone())
.or_default();
let origin = ConflictOrigin::new(binding.source, index);
if let Some((_, origins)) =
@@ -685,8 +686,7 @@ impl KeymapEditor {
.unwrap_or(KeybindSource::Unknown);
let keystroke_text = ui::text_for_keybinding_keystrokes(key_binding.keystrokes(), cx);
let ui_key_binding = ui::KeyBinding::new_from_gpui(key_binding.clone(), cx)
.vim_mode(source == KeybindSource::Vim);
let binding = KeyBinding::new(key_binding, source);
let context = key_binding
.predicate()
@@ -717,7 +717,7 @@ impl KeymapEditor {
StringMatchCandidate::new(index, &action_information.humanized_name);
processed_bindings.push(ProcessedBinding::new_mapped(
keystroke_text,
ui_key_binding,
binding,
context,
source,
action_information,
@@ -975,12 +975,11 @@ impl KeymapEditor {
if conflict.is_user_keybind_conflict() {
base_button_style(index, IconName::Warning)
.icon_color(Color::Warning)
.tooltip(|window, cx| {
.tooltip(|_window, cx| {
Tooltip::with_meta(
"View conflicts",
Some(&ToggleConflictFilter),
"Use alt+click to show all conflicts",
window,
cx,
)
})
@@ -995,12 +994,11 @@ impl KeymapEditor {
}))
} else if self.search_mode.exact_match() {
base_button_style(index, IconName::Info)
.tooltip(|window, cx| {
.tooltip(|_window, cx| {
Tooltip::with_meta(
"Edit this binding",
Some(&ShowMatchingKeybinds),
"This binding is overridden by other bindings.",
window,
cx,
)
})
@@ -1011,12 +1009,11 @@ impl KeymapEditor {
}))
} else {
base_button_style(index, IconName::Info)
.tooltip(|window, cx| {
.tooltip(|_window, cx| {
Tooltip::with_meta(
"Show matching keybinds",
Some(&ShowMatchingKeybinds),
"This binding is overridden by other bindings.\nUse alt+click to edit this binding",
window,
cx,
)
})
@@ -1348,10 +1345,25 @@ impl HumanizedActionNameCache {
}
}
#[derive(Clone)]
struct KeyBinding {
keystrokes: Rc<[KeybindingKeystroke]>,
source: KeybindSource,
}
impl KeyBinding {
fn new(binding: &gpui::KeyBinding, source: KeybindSource) -> Self {
Self {
keystrokes: Rc::from(binding.keystrokes()),
source,
}
}
}
#[derive(Clone)]
struct KeybindInformation {
keystroke_text: SharedString,
ui_binding: ui::KeyBinding,
binding: KeyBinding,
context: KeybindContextString,
source: KeybindSource,
}
@@ -1359,7 +1371,7 @@ struct KeybindInformation {
impl KeybindInformation {
fn get_action_mapping(&self) -> ActionMapping {
ActionMapping {
keystrokes: self.ui_binding.keystrokes.clone(),
keystrokes: self.binding.keystrokes.clone(),
context: self.context.local().cloned(),
}
}
@@ -1401,7 +1413,7 @@ enum ProcessedBinding {
impl ProcessedBinding {
fn new_mapped(
keystroke_text: impl Into<SharedString>,
ui_key_binding: ui::KeyBinding,
binding: KeyBinding,
context: KeybindContextString,
source: KeybindSource,
action_information: ActionInformation,
@@ -1409,7 +1421,7 @@ impl ProcessedBinding {
Self::Mapped(
KeybindInformation {
keystroke_text: keystroke_text.into(),
ui_binding: ui_key_binding,
binding,
context,
source,
},
@@ -1427,8 +1439,8 @@ impl ProcessedBinding {
}
fn keystrokes(&self) -> Option<&[KeybindingKeystroke]> {
self.ui_key_binding()
.map(|binding| binding.keystrokes.as_slice())
self.key_binding()
.map(|binding| binding.keystrokes.as_ref())
}
fn keybind_information(&self) -> Option<&KeybindInformation> {
@@ -1446,9 +1458,8 @@ impl ProcessedBinding {
self.keybind_information().map(|keybind| &keybind.context)
}
fn ui_key_binding(&self) -> Option<&ui::KeyBinding> {
self.keybind_information()
.map(|keybind| &keybind.ui_binding)
fn key_binding(&self) -> Option<&KeyBinding> {
self.keybind_information().map(|keybind| &keybind.binding)
}
fn keystroke_text(&self) -> Option<&SharedString> {
@@ -1599,12 +1610,11 @@ impl Render for KeymapEditor {
.tooltip({
let focus_handle = focus_handle.clone();
move |window, cx| {
move |_window, cx| {
Tooltip::for_action_in(
"Search by Keystroke",
&ToggleKeystrokeSearch,
&focus_handle.clone(),
window,
cx,
)
}
@@ -1636,7 +1646,7 @@ impl Render for KeymapEditor {
let filter_state = self.filter_state;
let focus_handle = focus_handle.clone();
move |window, cx| {
move |_window, cx| {
Tooltip::for_action_in(
match filter_state {
FilterState::All => "Show Conflicts",
@@ -1646,7 +1656,6 @@ impl Render for KeymapEditor {
},
&ToggleConflictFilter,
&focus_handle.clone(),
window,
cx,
)
}
@@ -1698,12 +1707,11 @@ impl Render for KeymapEditor {
.icon_size(IconSize::Small),
{
let focus_handle = focus_handle.clone();
move |window, cx| {
move |_window, cx| {
Tooltip::for_action_in(
"View Default...",
&zed_actions::OpenKeymapFile,
&focus_handle,
window,
cx,
)
}
@@ -1745,12 +1753,11 @@ impl Render for KeymapEditor {
let keystroke_focus_handle =
self.keystroke_editor.read(cx).focus_handle(cx);
move |window, cx| {
move |_window, cx| {
Tooltip::for_action_in(
"Toggle Exact Match Mode",
&ToggleExactKeystrokeMatching,
&keystroke_focus_handle,
window,
cx,
)
}
@@ -1856,13 +1863,13 @@ impl Render for KeymapEditor {
)
.into_any_element();
let keystrokes = binding.ui_key_binding().cloned().map_or(
let keystrokes = binding.key_binding().map_or(
binding
.keystroke_text()
.cloned()
.unwrap_or_default()
.into_any_element(),
IntoElement::into_any_element,
|binding| ui::KeyBinding::from_keystrokes(binding.keystrokes.clone(), binding.source).into_any_element()
);
let action_arguments = match binding.action().arguments.clone()
@@ -2301,7 +2308,7 @@ impl KeybindingEditorModal {
.map_err(InputError::error)?;
let action_mapping = ActionMapping {
keystrokes: new_keystrokes,
keystrokes: Rc::from(new_keystrokes.as_slice()),
context: new_context.map(SharedString::from),
};