Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

treeview: add horizontal scrolling #1717

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

takase1121
Copy link
Member

This is taken from Pragtical but I wasn't able to cherry pick the commit, so I ended up recreating it.

This is taken from Pragtical but I wasn't able to cherry pick the commit,
so I ended up recreating it.

Co-authored-by: Jefferson González <[email protected]>
@Guldoman
Copy link
Member

Guldoman commented Feb 4, 2024

A few issues:

  • Clicking/hovering entries is broken when scrolled.
  • Clicking on directories resets the scroll location.
  • Clicking and not releasing directories removes the scrollbar.
    Simpler way to reproduce is making it show the tooltip first, then clicking without releasing.

Edit:

  • The maximum scrollable size is calculated only with the visible entries, so the horizontal scrollable size changes while scrolling vertically.
  • Clicking on directories resets the calculated scrollable size.

@jgmdev
Copy link
Member

jgmdev commented Feb 4, 2024

Prevent resetting h scrollbar position when not needed, always allow clicking an item to its right:

diff --git a/data/plugins/treeview.lua b/data/plugins/treeview.lua
index fb0aee30..20d30768 100644
--- a/data/plugins/treeview.lua
+++ b/data/plugins/treeview.lua
@@ -254,7 +254,7 @@ function TreeView:on_mouse_moved(px, py, ...)
 
   local item_changed, tooltip_changed
   for item, x,y,w,h in self:each_item() do
-    if px > x and py > y and px <= x + w and py <= y + h then
+    if px > x and py > y and px <= self.size.x and py <= y + h then
       item_changed = true
       self.hovered_item = item
 
@@ -445,7 +445,7 @@ function TreeView:draw()
   local doc = core.active_view.doc
   local active_filename = doc and core.project_absolute_path(doc.abs_filename or "")
 
-  local ox = math.abs(self:get_content_offset())
+  local sw, ox = 0, math.abs(self:get_content_offset())
   for item, x,y,w,h in self:each_item() do
     if y + h >= _y and y < _y + _h then
       local w = self:draw_item(
@@ -454,9 +454,10 @@ function TreeView:draw()
         item == self.hovered_item,
         x, y, w, h
       ) + ox
-      self.scroll_width = math.max(w, self.scroll_width)
+      sw = math.max(w, sw)
     end
   end
+  self.scroll_width = sw
 
   self:draw_scrollbar()
   if self.hovered_item and self.tooltip.x and self.tooltip.alpha > 0 then
@@ -515,7 +516,6 @@ function TreeView:toggle_expand(toggle, item)
   if not item then return end
 
   if item.type == "dir" then
-    self.scroll_width = 0
     if type(toggle) == "boolean" then
       item.expanded = toggle
     else

@Guldoman
Copy link
Member

Guldoman commented Feb 4, 2024

Only remaining issue is that the horizontal scroll position changes while scrolling vertically, when the maximum scrollable size becomes smaller than the current position.

If we want to keep the behavior that limits the maximum scroll size to the visible items, we should avoid setting it to a value lower than the current scroll position, and only adjust it when the user actually horizontally scrolls to below the max.

data/plugins/treeview.lua Outdated Show resolved Hide resolved
@Guldoman
Copy link
Member

About waiting until the user releases the scrollbar to reduce the scrollable size:

Without waiting:

Video.del.2024-02-28.04-35-41.webm

Waiting:

Video.del.2024-02-28.04-36-19.webm

What do you think?
If you want to test it, you just need to add

and not self.h_scrollbar.dragging

here

elseif ss > self.scroll.x + self.size.x then

@takase1121
Copy link
Member Author

About waiting until the user releases the scrollbar to reduce the scrollable size:

Without waiting:

Video.del.2024-02-28.04-35-41.webm
Waiting:

Video.del.2024-02-28.04-36-19.webm
What do you think? If you want to test it, you just need to add

and not self.h_scrollbar.dragging

here

elseif ss > self.scroll.x + self.size.x then

I think waiting is the way to go, not waiting is just a bit weird.

@Guldoman
Copy link
Member

Mmm, it looks like we need a core.redraw = true somewhere for when the mouse is released after dragging:

Video.del.2024-02-29.03-59-30.webm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants