Просмотр исходного кода

fix(gtk): address popover focus stealing and minibuffer rendering bugs

Stopped GtkCompletionPopup from stealing focus by removing row->grab_focus(). Adjusted popover positioning to appear above minibuffer. Fixed double-rendering of minibuffer text by rendering only suffix for completion. Prevented minibuffer from disappearing by always drawing background.
Bernardo Magri 1 месяц назад
Родитель
Сommit
795933094b
4 измененных файлов с 78 добавлено и 29 удалено
  1. 1 1
      documentation/PLAN.md
  2. 18 1
      src/gtk_completion_popup.cpp
  3. 36 8
      src/gtk_editor.cpp
  4. 23 19
      src/gtk_renderer.cpp

+ 1 - 1
documentation/PLAN.md

@@ -138,7 +138,7 @@ Lumacs/
 - ✅ **GTK Enhancements**: (Fully completed - Phase A).
 - ✅ **Minibuffer & Command System**: Minibuffer core logic, history management, and completion are fully centralized and integrated with the Command System (Phases Z and C completed).
     - **Advanced Completion UI**: Completed (Implemented popup completion window with descriptions and better visual feedback).
-    - **ISearch & TUI Polish**: ✅ Fixed. Exposed ISearch direction control, fixed TUI rendering overlap, fixed TUI Backspace, GTK rendering artifacts, and GTK Popover sizing/scrolling. Implemented `y_or_n_p` input handling.
+    - **Minibuffer & Popover Polish**: ✅ Fixed. Exposed ISearch direction control, fixed TUI rendering and Backspace, fixed GTK minibuffer double-rendering and disappearance, and polished GTK Popover positioning and focus management.
 - ✅ **Theme System Refactoring**:
     - [x] Implemented `editor:create_and_register_theme` Lua API to allow theme definition from Lua.
     - [x] Factored all hardcoded C++ themes (`default`, `everforest-dark`, `dracula`, `solarized-dark`, `nord`, `gruvbox-light`) into individual Lua files (`lua/themes/*.lua`).

+ 18 - 1
src/gtk_completion_popup.cpp

@@ -132,7 +132,24 @@ void GtkCompletionPopup::set_active_row(size_t index) {
         Gtk::ListBoxRow* row = list_box_.get_row_at_index(static_cast<int>(index));
         if (row) {
             list_box_.select_row(*row);
-            row->grab_focus(); // Ensure the selected row is visible and focused
+            
+            // Scroll to row without grabbing focus
+            double x, y;
+            if (row->translate_coordinates(list_box_, 0, 0, x, y)) {
+                auto vadj = list_scrolled_window_.get_vadjustment();
+                if (vadj) {
+                    double page_size = vadj->get_page_size();
+                    double value = vadj->get_value();
+                    double row_height = row->get_allocated_height();
+                    
+                    if (y < value) {
+                        vadj->set_value(y);
+                    } else if (y + row_height > value + page_size) {
+                        vadj->set_value(y + row_height - page_size);
+                    }
+                }
+            }
+            
             active_index_ = index;
         }
     }

+ 36 - 8
src/gtk_editor.cpp

@@ -200,6 +200,11 @@ void GtkEditor::set_core(EditorCore* core) {
 // Helper to recursively find and redraw all drawing areas
 void GtkEditor::queue_redraw_all_windows(Gtk::Widget* widget) {
     if (!widget) return;
+
+    // If we are redrawing the main content root, also redraw the minibuffer
+    if (widget == content_widget_ && minibuffer_drawing_area_) {
+        minibuffer_drawing_area_->queue_draw();
+    }
     
     if (auto drawing_area = dynamic_cast<Gtk::DrawingArea*>(widget)) {
         drawing_area->queue_draw();
@@ -480,21 +485,42 @@ void GtkEditor::rebuild_layout() {
     if (!root_layout) return;
 
     // Remove existing content
-    if (content_widget_) {
-        window_->unset_child();
-    }
+    window_->unset_child();
 
-    // Clear the drawing area reference since we're rebuilding
+    // Clear references
     drawing_area_ = nullptr;
+    minibuffer_drawing_area_ = nullptr;
 
     // Initialize cached active window to prevent focus jumping
     cached_active_window_ = core_->active_window();
 
-    // Create new layout based on the tree
+    // Create Root VBox
+    auto root_box = Gtk::make_managed<Gtk::Box>(Gtk::Orientation::VERTICAL);
+
+    // Create Content Layout
     content_widget_ = create_widget_for_layout_node(root_layout);
     if (content_widget_) {
-        window_->set_child(*content_widget_);
+        root_box->append(*content_widget_);
+        content_widget_->set_expand(true);
     }
+
+    // Create Minibuffer DrawingArea
+    minibuffer_drawing_area_ = Gtk::make_managed<Gtk::DrawingArea>();
+    // Approximate height for minibuffer (line height + padding). 
+    // We don't have exact metrics here yet, but 40px is safe for Monospace 12.
+    minibuffer_drawing_area_->set_content_height(40); 
+    minibuffer_drawing_area_->set_expand(false);
+    
+    minibuffer_drawing_area_->set_draw_func([this](const Cairo::RefPtr<Cairo::Context>& cr, int width, int height) {
+        if (gtk_renderer_) {
+            gtk_renderer_->render_minibuffer(cr, width, height);
+        }
+    });
+
+    root_box->append(*minibuffer_drawing_area_);
+
+    // Set Root Box as Child
+    window_->set_child(*root_box);
 }
 
 // Create GTK widget tree from LayoutNode tree
@@ -654,9 +680,11 @@ void GtkEditor::show_completion_popup() {
     // We need to position the popover relative to the main window, pointing to the minibuffer area.
     Gdk::Rectangle rect;
     rect.set_x(0); // Minibuffer starts at x=0
-    rect.set_y(window_->get_height() - 1); // Last line of the window
+    // Point to the entire minibuffer area so the popover appears ABOVE it
+    int minibuffer_height = 40; // Match the height set in rebuild_layout
+    rect.set_y(window_->get_height() - minibuffer_height); 
     rect.set_width(window_->get_width());
-    rect.set_height(1); // Small height to represent the minibuffer line
+    rect.set_height(minibuffer_height); 
 
     completion_popup_->show_popup(candidates, 0, *window_, rect.get_x(), rect.get_y());
 }

+ 23 - 19
src/gtk_renderer.cpp

@@ -443,33 +443,32 @@ void GtkRenderer::render_modeline_for_window(const Cairo::RefPtr<Cairo::Context>
 void GtkRenderer::render_minibuffer(const Cairo::RefPtr<Cairo::Context>& cr, int width, int height) {
     if (!core_.theme_manager().active_theme()) return;
 
-    // Only render if minibuffer is active or a message is set
-    if (!core_.minibuffer_manager().is_active() && core_.last_message().empty()) {
-        return;
-    }
-    
-    // Calculate minibuffer position (bottom line with padding)
-    double minibuffer_y = height - line_height_ - PADDING_BOTTOM;
-    double minibuffer_x = PADDING_LEFT;
-    
     // Get theme colors
     auto theme = core_.theme_manager().active_theme();
     Color bg = theme->get_bg_color(ThemeElement::Background);
     Color fg = theme->get_fg_color(ThemeElement::Normal);
     
-    // Draw minibuffer background (slightly different shade)
+    // ALWAYS draw minibuffer background to prevent it from disappearing
     // Clear the ENTIRE surface first to avoid artifacts
     cr->set_source_rgb(bg.r / 255.0 * 0.9, bg.g / 255.0 * 0.9, bg.b / 255.0 * 0.9);
     cr->rectangle(0, 0, width, height); // Clear full dedicated area
     cr->fill();
     
-    // Draw separator line above minibuffer (optional if dedicated area)
-    // We can draw it at the very top 0
+    // Draw separator line above minibuffer
     cr->set_source_rgb(fg.r / 255.0 * 0.5, fg.g / 255.0 * 0.5, fg.b / 255.0 * 0.5);
     cr->set_line_width(1.0);
     cr->move_to(0, 0);
     cr->line_to(width, 0);
     cr->stroke();
+
+    // Only render text if minibuffer is active or a message is set
+    if (!core_.minibuffer_manager().is_active() && core_.last_message().empty()) {
+        return;
+    }
+    
+    // Calculate minibuffer position (bottom line with padding)
+    double minibuffer_y = height - line_height_ - PADDING_BOTTOM;
+    double minibuffer_x = PADDING_LEFT;
     
     // Prepare minibuffer text
     std::string minibuffer_text;
@@ -525,13 +524,18 @@ void GtkRenderer::render_minibuffer(const Cairo::RefPtr<Cairo::Context>& cr, int
         if (current_completion && input_part != *current_completion) {
             std::string completion_suffix = current_completion->substr(input_part.length());
             if (!completion_suffix.empty()) {
-                auto completion_layout = Pango::Layout::create(cr);
+                // Calculate width of existing text to position suffix
+                Pango::Rectangle ink_rect, logical_rect;
+                layout->get_pixel_extents(ink_rect, logical_rect);
+                double text_width = logical_rect.get_width();
+
+                auto completion_layout = Pango::Layout::create(main_drawing_area_.get_pango_context());
                 completion_layout->set_font_description(font_desc_);
-                completion_layout->set_text(minibuffer_text + completion_suffix);
+                completion_layout->set_text(completion_suffix); // Only draw suffix
                 
                 Pango::AttrList completion_attr_list;
-                if (auto face = theme->get_face("minibuffer-completion")) { // Assuming a face for completion
-                    apply_face_attributes(completion_attr_list, *face, static_cast<int>(minibuffer_text.length()), static_cast<int>(completion_suffix.length()));
+                if (auto face = theme->get_face("minibuffer-completion")) {
+                    apply_face_attributes(completion_attr_list, *face, 0, static_cast<int>(completion_suffix.length()));
                 } else {
                     // Fallback: dimmed foreground
                     Color dim_fg = fg;
@@ -539,14 +543,14 @@ void GtkRenderer::render_minibuffer(const Cairo::RefPtr<Cairo::Context>& cr, int
                     dim_fg.g = static_cast<unsigned char>(dim_fg.g * 0.7);
                     dim_fg.b = static_cast<unsigned char>(dim_fg.b * 0.7);
                     auto attr = Pango::Attribute::create_attr_foreground(dim_fg.r * 257, dim_fg.g * 257, dim_fg.b * 257);
-                    attr.set_start_index(static_cast<int>(minibuffer_text.length()));
-                    attr.set_end_index(static_cast<int>(minibuffer_text.length() + completion_suffix.length()));
+                    attr.set_start_index(0);
+                    attr.set_end_index(static_cast<int>(completion_suffix.length()));
                     completion_attr_list.insert(attr);
                 }
                 completion_layout->set_attributes(completion_attr_list);
 
                 cr->set_source_rgb(fg.r / 255.0, fg.g / 255.0, fg.b / 255.0);
-                cr->move_to(minibuffer_x, minibuffer_y);
+                cr->move_to(minibuffer_x + text_width, minibuffer_y); // Position after text
                 completion_layout->show_in_cairo_context(cr);
             }
         }