Correctly compute placeholder text for buffer search query editor (#3749)

Nathan Sobo created 2 years ago

Rather than relying on the focused element, instead explicitly pass the
focus handle for the query editor when determining the prev/next
bindings. Only compute these values once.

Release Notes:

- N/A

Change summary

crates/editor2/src/editor.rs        | 11 ++
crates/editor2/src/element.rs       |  3 
crates/search2/src/buffer_search.rs | 96 ++++++++++++++----------------
3 files changed, 55 insertions(+), 55 deletions(-)

Detailed changes

crates/editor2/src/editor.rs

@@ -1995,13 +1995,20 @@ impl Editor {
         self.collaboration_hub = Some(hub);
     }
 
+    pub fn placeholder_text(&self) -> Option<&str> {
+        self.placeholder_text.as_deref()
+    }
+
     pub fn set_placeholder_text(
         &mut self,
         placeholder_text: impl Into<Arc<str>>,
         cx: &mut ViewContext<Self>,
     ) {
-        self.placeholder_text = Some(placeholder_text.into());
-        cx.notify();
+        let placeholder_text = Some(placeholder_text.into());
+        if self.placeholder_text != placeholder_text {
+            self.placeholder_text = placeholder_text;
+            cx.notify();
+        }
     }
 
     pub fn set_cursor_shape(&mut self, cursor_shape: CursorShape, cx: &mut ViewContext<Self>) {

crates/editor2/src/element.rs

@@ -1722,11 +1722,12 @@ impl EditorElement {
             return Vec::new();
         }
 
-        // When the editor is empty and unfocused, then show the placeholder.
+        // Show the placeholder when the editor is empty
         if snapshot.is_empty() {
             let font_size = self.style.text.font_size.to_pixels(cx.rem_size());
             let placeholder_color = cx.theme().styles.colors.text_placeholder;
             let placeholder_text = snapshot.placeholder_text();
+
             let placeholder_lines = placeholder_text
                 .as_ref()
                 .map_or("", AsRef::as_ref)

crates/search2/src/buffer_search.rs

@@ -102,65 +102,57 @@ impl EventEmitter<Event> for BufferSearchBar {}
 impl EventEmitter<workspace::ToolbarItemEvent> for BufferSearchBar {}
 impl Render for BufferSearchBar {
     type Element = Div;
+
     fn render(&mut self, cx: &mut ViewContext<Self>) -> Self::Element {
-        // let query_container_style = if self.query_contains_error {
-        //     theme.search.invalid_editor
-        // } else {
-        //     theme.search.editor.input.container
-        // };
         if self.dismissed {
             return div();
         }
+
         let supported_options = self.supported_options();
 
-        let previous_query_keystrokes = cx
-            .bindings_for_action(&PreviousHistoryQuery {})
-            .into_iter()
-            .next()
-            .map(|binding| {
-                binding
-                    .keystrokes()
-                    .iter()
-                    .map(|k| k.to_string())
-                    .collect::<Vec<_>>()
-            });
-        let next_query_keystrokes = cx
-            .bindings_for_action(&NextHistoryQuery {})
-            .into_iter()
-            .next()
-            .map(|binding| {
-                binding
-                    .keystrokes()
-                    .iter()
-                    .map(|k| k.to_string())
-                    .collect::<Vec<_>>()
-            });
-        let new_placeholder_text = match (previous_query_keystrokes, next_query_keystrokes) {
-            (Some(previous_query_keystrokes), Some(next_query_keystrokes)) => {
-                format!(
-                    "Search ({}/{} for previous/next query)",
-                    previous_query_keystrokes.join(" "),
-                    next_query_keystrokes.join(" ")
-                )
-            }
-            (None, Some(next_query_keystrokes)) => {
-                format!(
-                    "Search ({} for next query)",
-                    next_query_keystrokes.join(" ")
-                )
-            }
-            (Some(previous_query_keystrokes), None) => {
-                format!(
-                    "Search ({} for previous query)",
-                    previous_query_keystrokes.join(" ")
-                )
+        if self.query_editor.read(cx).placeholder_text().is_none() {
+            let query_focus_handle = self.query_editor.focus_handle(cx);
+            let up_keystrokes = cx
+                .bindings_for_action_in(&PreviousHistoryQuery {}, &query_focus_handle)
+                .into_iter()
+                .next()
+                .map(|binding| {
+                    binding
+                        .keystrokes()
+                        .iter()
+                        .map(|k| k.to_string())
+                        .collect::<Vec<_>>()
+                });
+            let down_keystrokes = cx
+                .bindings_for_action_in(&NextHistoryQuery {}, &query_focus_handle)
+                .into_iter()
+                .next()
+                .map(|binding| {
+                    binding
+                        .keystrokes()
+                        .iter()
+                        .map(|k| k.to_string())
+                        .collect::<Vec<_>>()
+                });
+
+            let placeholder_text =
+                up_keystrokes
+                    .zip(down_keystrokes)
+                    .map(|(up_keystrokes, down_keystrokes)| {
+                        Arc::from(format!(
+                            "Search ({}/{} for previous/next query)",
+                            up_keystrokes.join(" "),
+                            down_keystrokes.join(" ")
+                        ))
+                    });
+
+            if let Some(placeholder_text) = placeholder_text {
+                self.query_editor.update(cx, |editor, cx| {
+                    editor.set_placeholder_text(placeholder_text, cx);
+                });
             }
-            (None, None) => String::new(),
-        };
-        let new_placeholder_text = Arc::from(new_placeholder_text);
-        self.query_editor.update(cx, |editor, cx| {
-            editor.set_placeholder_text(new_placeholder_text, cx);
-        });
+        }
+
         self.replacement_editor.update(cx, |editor, cx| {
             editor.set_placeholder_text("Replace with...", cx);
         });