Add hover-select mode to listview widget

[haxllo]

Summary

• Add hover-select mode to listview: scroll events move viewport without changing selection, item under cursor is auto-selected
• Fix hover-select lag caused by redundant hover update in draw loop
• Map scrollbar to viewport offset in hover-select mode so it tracks position correctly

Changes

• Add hover_select flag, mouse position tracking, viewport offset management
• Remove listview_update_hover_from_mouse from listview_draw (was causing feedback loop)
• Scrollbar uses viewport offset instead of selected item when hover-select is active

Test plan

• rofi -show drun -hover-select — hover should be smooth, no lag
• Scroll to bottom — scrollbar should reach the end

[Copilot]

Pull request overview

This PR adds a “hover-select” interaction mode to the listview widget, where mouse-wheel scrolling moves the viewport without changing selection, and the item under the cursor becomes selected automatically. It also adjusts scrollbar behavior in hover-select mode and attempts to reduce hover update lag.

Changes:

• Introduces hover_select state, mouse position tracking, and a separate viewport_offset for scroll position.
• Updates mouse scroll handling to advance viewport_offset and re-evaluate hovered selection.
• Changes scrollbar handle mapping to track viewport_offset when hover-select is enabled, and adds a (currently unused) hover-update callback API.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
source/widgets/listview.c	Adds hover-select state, viewport offset scrolling, hover-from-mouse selection update, and scrollbar mapping changes.
source/view.c	Wires hover-select config into listview and registers a hover-update callback.
include/widgets/listview.h	Exposes new hover-select setter and hover-update callback API/docs.

Comments suppressed due to low confidence (4)

source/widgets/listview.c:831

• listview_element_motion_notify stores x/y into lv->mouse_x/mouse_y, but these coordinates are relative to the hovered element widget (view.c converts to relative before calling motion_notify). listview_update_hover_from_mouse later interprets mouse_y as listview-relative, which can make hover-update-after-scroll compute the wrong row (often negative after subtracting padding) unless the mouse moved again. Consider either removing these assignments here and only tracking mouse coordinates in listview_find_mouse_target (listview-relative), or translating element-relative coordinates into listview coordinates before storing.

static gboolean listview_element_motion_notify(widget *wid, gint x, gint y) {
  listview *lv = (listview *)wid->parent;
  lv->mouse_x = x;
  lv->mouse_y = y;
  lv->mouse_hovering = TRUE;

source/widgets/listview.c:860

• listview_update_hover_from_mouse computes idx as last_offset + row, which ignores column layouts and reverse placement. In multi-column layouts or when reverse is enabled, this can select a different item than the one actually under the cursor after scrolling. Consider deriving the hovered index from both mouse_x and mouse_y using the same layout math as draw/positioning (row+column), and account for reverse when mapping y to row.

  unsigned int row = rel_y / item_height;
  unsigned int max = MIN(lv->cur_elements, lv->req_elements - lv->last_offset);
  if (row < max) {
    unsigned int idx = lv->last_offset + row;
    if (idx != listview_get_selected(lv)) {

source/widgets/listview.c:501

• Mapping the scrollbar handle to viewport_offset updates the visual position, but scrollbar interactions still call listview_set_selected() (see scrollbar.c), which does not update viewport_offset. In hover-select mode, scrolling is driven by viewport_offset, so dragging/clicking the scrollbar will not actually move the viewport. Consider adding a listview API for setting viewport offset and using it from the scrollbar when hover_select is active (or updating viewport_offset in listview_set_selected when in hover-select mode).

  if (lv->hover_select) {
    unsigned int max_offset = lv->req_elements > lv->max_elements
                                  ? lv->req_elements - lv->max_elements
                                  : 0;
    scrollbar_set_max_value(lv->scrollbar, max_offset);
    scrollbar_set_handle_length(lv->scrollbar, 1);
    scrollbar_set_handle(lv->scrollbar, lv->viewport_offset);
  } else {
    scrollbar_set_max_value(lv->scrollbar, lv->req_elements);
    scrollbar_set_handle_length(lv->scrollbar, lv->cur_columns * lv->max_rows);
    if (lv->reverse) {
      scrollbar_set_handle(lv->scrollbar, lv->req_elements - lv->selected - 1);
    } else {
      scrollbar_set_handle(lv->scrollbar, lv->selected);
    }

source/widgets/listview.c:1330

• hover_update_callback/hover_update_udata are stored via listview_set_hover_update_callback, but the callback is never invoked anywhere in listview. This makes the new API (and view.c wiring) effectively dead code and the header doc misleading. Either call the callback at the appropriate points (e.g., after changing viewport_offset / scroll operations) or remove the callback fields and setter until needed.

void listview_set_hover_update_callback(listview *lv, listview_hover_update_cb cb, void *udata) {
  if (lv) {
    lv->hover_update_callback = cb;
    lv->hover_update_udata = udata;
  }
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[haxllo]

Thanks for the thorough analysis. I looked into this and decided not to change it because:

1. When req_elements <= max_elements, all items fit on screen — there's nothing to scroll, so the scrollbar is cosmetic only. The handle_length=1 renders a minimal handle which is fine for this case.

2. The scrollbar widget internally guards against division by zero — scrollbar_draw() uses (length - 1) only when length > 1.

3. For the normal case (max_offset > 0), handle_length=1 means the handle represents 1 visible page worth of items, which matches the viewport-based model where each scroll step is one item.

That said, if you'd prefer I adjust the handle sizing, happy to follow your guidance.

[DaveDavenport]

We use G_GNUC_UNUSED for this.
https://docs.gtk.org/glib/macros.html

[haxllo]

Fixed — removed the callback entirely as it was dead code (hover re-evaluation is already handled by listview_update_hover_from_mouse in the scroll handlers).

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (5)

source/widgets/listview.c:489

• In hover-select mode scrollbar_set_max_value(lv->scrollbar, max_offset) can set sb->length to 1 when max_offset==0 (because scrollbar clamps to at least 1). scrollbar_draw() divides by (sb->length - 1) without guarding for length==1, which results in a division-by-zero (inf/NaN) and can break rendering. Ensure the value passed to scrollbar_set_max_value is >= 2 (or change the scrollbar range to represent max_offset + 1 positions), or add a guard in the scrollbar widget to handle length==1 safely.

                                  ? lv->req_elements - lv->max_elements
                                  : 0;
    scrollbar_set_max_value(lv->scrollbar, max_offset);
    scrollbar_set_handle_length(lv->scrollbar, 1);
    scrollbar_set_handle(lv->scrollbar, lv->viewport_offset);

source/widgets/listview.c:827

• listview_element_motion_notify receives coordinates relative to the hovered element (see widget_xy_to_relative usage in rofi_view_handle_mouse_motion), but it stores them directly into lv->mouse_x/mouse_y. listview_update_hover_from_mouse later treats these as listview-relative coordinates, so scroll-triggered hover updates can select the wrong row. Convert (x,y) to listview-relative coordinates (e.g., add the element widget's x/y offsets, or store absolute coordinates from listview_find_mouse_target only) before saving.

static gboolean listview_element_motion_notify(widget *wid, gint x, gint y) {
  listview *lv = (listview *)wid->parent;
  lv->mouse_x = x;
  lv->mouse_y = y;
  lv->mouse_hovering = TRUE;

source/widgets/listview.c:856

• listview_update_hover_from_mouse maps the mouse position to a row index using only rel_y / item_height. This ignores mouse_x and the current packing mode/column count, so in multi-column layouts (or horizontal packing) the item under the cursor can be misidentified. Consider computing both row+column (using mouse_x) or reusing the existing per-element hit testing logic to find the box under the cursor and then derive the selected index.

  unsigned int row = rel_y / item_height;
  unsigned int max = MIN(lv->cur_elements, lv->req_elements - lv->last_offset);
  if (row < max) {
    unsigned int idx = lv->last_offset + row;
    if (idx != listview_get_selected(lv)) {

source/widgets/listview.c:770

• In hover-select mode, wheel scrolling increments viewport_offset by 1 element unconditionally. For horizontal packing, the existing scroll logic (scroll_continious_rows) scrolls in row-sized steps (menu_columns elements) to preserve grid alignment, and max_offset should be computed in row terms as well. Update the step size and max calculation based on pack_direction/menu_columns so horizontal layouts don’t shift the grid by partial rows.

      unsigned int max_offset = lv->req_elements > lv->max_elements
                                    ? lv->req_elements - lv->max_elements
                                    : 0;
      if (lv->viewport_offset < max_offset) {
        lv->viewport_offset++;

source/widgets/listview.c:866

• listview_sync_positions duplicates substantial layout logic from listview_draw (offset calculation, spacing/width math, widget_move/resize loop). This increases the risk of future divergence and does extra work on every scroll tick even though scrolling only needs last_offset to be updated before calling listview_update_hover_from_mouse. Consider extracting a shared helper (or making listview_sync_positions only update last_offset/state) to reduce duplication and per-scroll overhead.

static void listview_sync_positions(listview *lv) {
  unsigned int offset = 0;
  if (lv->scroll_type == LISTVIEW_SCROLL_PER_PAGE) {
    offset = scroll_per_page(lv);
  } else if (lv->pack_direction == ROFI_ORIENTATION_VERTICAL) {

[DaveDavenport]

I works well (did a quick test), except for clicking on the scrollbar, that is now broken?

[haxllo]

I works well (did a quick test), except for clicking on the scrollbar, that is now broken?

let me look into it

[haxllo]

I works well (did a quick test), except for clicking on the scrollbar, that is now broken?

fixed it ,the issue was that clicking/dragging the scrollbar called listview_set_selected which only changed the selection index but never updated viewport_offset so the list appeared not to scroll. I added listview_scrollbar_scroll_to that properly updates viewport_offset, syncs positions, and redraws in hover-select mode

"><also bundled a few UX improvements while at it: jump on press instead of release, page up/down on track clicks, pressed-state handle color, auto-hide when everything fits, and a wider click target.

[DaveDavenport]

Thanks!

[haxllo]

Thanks!

Would you consider merging it, or are there still issues that need to be addressed first?

[DaveDavenport]

I've asked others to give it a quick test. To see if they have remarks.

[lbonn]

With -hover-select, leaving the cursor in place and moving the selection with arrow keys does not auto-scroll anymore.