Skip to content

Conversation

WizardOhio24
Copy link
Contributor

@WizardOhio24 WizardOhio24 commented Oct 4, 2020

Implements mouse scrolling for any lists.

Linked to #226

@WizardOhio24
Copy link
Contributor Author

I've set clippy to allow that the function has too many lines, I can split it up if you want but it seems that all the event stuff should be handled in event.

@extrawurst
Copy link
Collaborator

I've set clippy to allow that the function has too many lines, I can split it up if you want but it seems that all the event stuff should be handled in event.

that's fine for now, a potential refactor of this should go into a different issue anyway

@extrawurst
Copy link
Collaborator

I am not sure about this yet. if we do this we should do it right and check the bounds of the mouse and scroll in whatever element the mouse currently is. right now it scrolls whatever element is currently in the key-based focus

@WizardOhio24
Copy link
Contributor Author

That would require a lot of work. TUI does not have mouse support so that would have to be implemented first then when the scroll button is pressed/scrolled it can switch focus to the one under the mouse then scroll.
But I don't think it needs to focus to what's under the mouse. On windows, I think you can click on a window then no matter where the mouse is, when the scroll is used, the window which is currently selected is scrolled (I'm on manjaro, so I'm not sure). So, from a windows perspective, I think the PR implementation would be the expected implementation.

@extrawurst
Copy link
Collaborator

extrawurst commented Oct 10, 2020

TUI does not have mouse support

not sure what you mean.
lets consider the Diff for now: you already capture the mouse now we only need to use the col and row to check if we are in bounds of it. we can easily do this by not only saving the current_size on draw but the entire Rect. is col/row in bounds of this rect the scroll happened on Diff and we can scroll the scroll_top without having to even focus this component.

@WizardOhio24
Copy link
Contributor Author

Ah, Ok. I guess you could implement it like that. What I meant was that you cannot use the mouse to select a list item in TUI, but I guess that it probably wouldn't be that hard just to find out if the mouse is in a box.
I think still that there should be an option, so the user can have this behavior you're describing (which I prefer) or so the user must click on the box then scroll to scroll it (and if they move their mouse out of the box, it will still scroll the clicked box when they scroll).

@WizardOhio24
Copy link
Contributor Author

With this PR, I think I will come back to it after more mouse handling is implemented. This adds scrolling on the box selected by the keys but not the mouse(mouse selection is not implemented) and at the moment mouse selection is not a priority for me(This is after all a TUI), I did like being able to select with the keys and scroll with the mouse though.

But before closing this PR, currently there is glitchy scrolling in GitUI, this would fix it, then further down the line an option could be implemented to turn on or off mouse scrolling. I am happy to make any edits to the PR for mouse scrolling, but I don't think I'm going to go as far as mouse selection.

@WizardOhio24
Copy link
Contributor Author

WizardOhio24 commented Nov 5, 2020

I think I'll continue with Mouse Selection, but not in this PR. This PR will only deal with Mouse Scrolling on the selected box, but I might put in a PR in the future on Full Mouse Selection. So, for me, this is ready to be merged, after review.

@extrawurst
Copy link
Collaborator

@WizardOhio24 can you look into updating the branch to current master and fix the format?

@extrawurst extrawurst marked this pull request as draft December 19, 2020 16:41
@WizardOhio24 WizardOhio24 marked this pull request as ready for review February 2, 2021 21:19
@WizardOhio24
Copy link
Contributor Author

I think this is now ready to be reviewed/merged.

@extrawurst extrawurst merged commit 457f644 into gitui-org:master Feb 23, 2021
extrawurst pushed a commit that referenced this pull request Feb 24, 2021
extrawurst pushed a commit that referenced this pull request May 27, 2021
extrawurst pushed a commit that referenced this pull request May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants