search

git-up / GitUp

The Git interface you've been missing all your life has finally arrived.
http://gitup.co
GNU General Public License v3.0
11.53k stars 1.28k forks source link

Small scroll inconsistency in Quick View #727

Open jason5122 opened 3 years ago

jason5122 commented 3 years ago

In Quick View for a commit, the changed files are listed on the left and the diff is on the right. As you scroll through the diff, the highlighted file changes to correspond to the diff. The normal behavior is shown below:

Top

top rest

Bottom

bottom rest

However, there is a situation in which no file is highlighted on the left, shown below. If you scroll up past the topmost file, nothing is selected. If you scroll up past the bottommost file, the bottommost file remains selected.

Top, overscroll

top scroll

Bottom, overscroll

bottom scroll

Is this intentional?

lucasderraugh commented 3 years ago

I noticed this just the other day. Fixing the top one is a simple bounds check to make sure we don't select above the top. The bottom is trickier because the selection is based off of the top file visible, which if the files are small changes can result in it being a few more files down.

jason5122 commented 3 years ago

Is there anything wrong with the bottom overscroll one? I just added it for comparison with the top overscroll but I might have missed it.

lucasderraugh commented 3 years ago

For the bottom case, the list on the left has selected the file at the top of the right side, it's not the last file on the right. To see it, you have to manually scroll on the left list.

jason5122 commented 3 years ago

Ah I see. I actually thought it was correct as technically the file on the top of the right window isn't the bottommost file, but a few files above. However, you're right that it doesn't make much sense, since you get something like this if you select the last file(s) on the left list:

https://user-images.githubusercontent.com/34594853/102896476-72cda500-4434-11eb-88f7-fd9572b6b590.mov

Maybe we should allow for an overscroll so that the bottom file can truly be on the bottom? I'd say that would make the most sense. I could try to create a pull request but I'm very new to macOS GUIs, so I'd need some time to learn first.

lucasderraugh commented 3 years ago

Maybe just fixing the top one would be a good fix for now. The bottom selection is difficult to resolve and would change some scrolling behavior most likely.

jason5122 commented 3 years ago

Agreed. Is the top an easy fix?

lucasderraugh commented 3 years ago

I think it would be relatively straightforward. I haven't looked at the code, but my guess would be that the selection is being updated by NSScrollView callbacks for position and then we'd check which row/section is in that position.