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

Improve mouse support #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Tacolizard
Copy link

Scrolling the mouse now scrolls the cursor instead of the entire tree.
Clicking on an unselected item will select it. Clicking a selected item will open it. (Makes it simpler to see what's going on, and makes it easier to switch between panes using the mouse)

I haven't made any plugins before so I copied a bit of existing code that I'm not sure is necessary.

Copy link
Collaborator

@sum01 sum01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs some fixes, as mentioned in the review.

While I wouldn't decline this purely because of double-click, just know I'm not really a fan of that idea.

I like the moving of the cursor though instead of scrolling, good idea.

@@ -1041,6 +1041,24 @@ function onCursorUp(view)
selectline_if_tree(view)
end

--scroll cursor when mousewheel is scrolled
function onScrollDown(view)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This & onScrollUp should be pre instead of on. This is because a pre action is triggered before performing the action, while on means it's triggered after the action occurs.

Think of it in the sequence of actions:

  1. You try to scroll down, triggering the preScrollDown, but not actually performing the action.
  2. If preScrollDown has return false in it, the scroll down action is canceled, stopping here. Otherwise, it continues to 3.
  3. The scroll action is actually performed.
  4. The onScrollDown callback is triggered. This doesn't listen for any return's

end
end

function onScrollUp(view)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, pre instead of on

tree_view.Buf.Cursor:Down()
select_line()
-- not sure if return false is required, I'm new to this
return false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return false tells Micro to not actually perform the action, but that doesn't work if it's on.

try_open_at_y(new_y)

--open item if it is selected, otherwise select it.
if tree_view.Buf.Cursor.Loc.Y == new_y then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this chunk really necessary? I feel having to double-click to open is kind of pointless, although I am used to single-click's on my linux DE.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer double click but it also solves the problem that if you use a mouse click to switch to the tree pane it will enter whatever directory the cursor is on

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That problem could be solved without requiring double-click, I think?

Possibly by adding in a section to the pre callbacks to check for if view ~= tree_view, getting cursor xy position, then checking if it's anywhere inside the dimensions of the tree. If it is, you could set the current view to the tree_view, then perform the scroll action.

But I'm not exactly sure how you'd get the xy of the tree. Probably possible..

@sum01
Copy link
Collaborator

sum01 commented Apr 22, 2018

@Tacolizard Are you still working on this?

I don't mind if you leave the double-click as-is, but I can't merge if you don't make the changes to using pre over on.

@Lisiadito
Copy link

would love to see these fixes merged. @Tacolizard are you still interested in in doing these changes? otherwise I would fix this PR by opening a new one if thats fine @sum01 ?

@sum01
Copy link
Collaborator

sum01 commented Jul 27, 2019

Fine by me either way.

@octoshrimpy
Copy link

any updates on this?

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.

4 participants