FredrikNoren / ungit

The easiest way to use git. On any platform. Anywhere.
MIT License
10.43k stars 636 forks source link

Click & Drag Selection on Patch Changes #1421

Open Sogolumbo opened 3 years ago

Sogolumbo commented 3 years ago

Current behaviour: When selecting Patch all lines are checked. The only way to change the selection is to click on all checkboxes of the lines that should be deselected. To me this feels very inefficient and a little bit annoying, especially with long commits (>100 lines) which need patching the most.

Feature proposal: Make it possible to select or deselect multiple lines at once by clicking and holding on the first checkbox and releasing the mouse on the last line. To make this feature easier to understand and find it would be nice if the state of the checkboxes would change while moving the mouse around.

More ideas: It would be nice if the clickable area of the checkboxes would be a little bit bigger (maybe a square which fills the whole height of the line). A button which selects/deselects all checkboxes would also be very useful.

wmertens commented 3 years ago

Ideally, you'd simply select a bunch of lines and then there's a popup to select/deselect. This works easily with touch, too.

Also, a toggle all lines button would be great.

Sogolumbo commented 3 years ago

Ideally, you'd simply select a bunch of lines and then there's a popup to select/deselect.

What you're wishing for could also be done with a 'invert selection' button, right?

What I'm talking about is something different: being able to select/deselect many specific lines at once. This is useful when you have hundreds of changed lines and roughly half of them need to go into one commit, the rest somewhere else.

Sogolumbo commented 3 years ago

About the touch surfaces: The feature might work best with double tap & drag on touch surfaces.

wmertens commented 3 years ago

How about a select strip: when you use the mouse you need to click and drag to toggle lines. For touch, you simly need to touch and drag. The first state you toggle is the one that is repeated across the drag. That seems very fast to me.

It needs to be a strip because otherwise you can't scroll any more on touch surfaces.

And if you hit a screen boundary, it scrolls the window for you while continuing the drag?

simonwiles commented 3 years ago

I've actually written a simple ~15 line change which allows clicking (or tapping) any part of the line to toggle the checkbox. I've was gonna tidy it up and offer it as a PR but life is coming at us all rather fast at the minute, huh... It doesn't have any dragging support, but it's a really simple change that improves usability considerably in the kind of scenario @Sogolumbo is describing (at least, for me imo). It doesn't implement the fancy UI described here (which sounds cool btw), but I reckon it's one of those 80/20 things -- it eases a lot of the pain with a minimally invasive intervention. If no ones actually currently working on a more involved solution to this issue, it might be a worthwhile halfway house in the interim?

wmertens commented 3 years ago

Any change is an improvement here :)

Sogolumbo commented 3 years ago

Sounds good. Can you check whether it's still possible to select and copy the text from the file? I think that's an important feature which should not be compromised.

simonwiles commented 3 years ago

Yeah, that's a common part of my workflow too, so I know that still works. I've just noticed I also have a local change that prevents the selection of an additional space in front of every line when you select and copy a block in this way, 'cos that always annoys me. I'll find some time to make sure it's sound and PR it as soon as I can.

simonwiles commented 3 years ago

PR #1429 raised for consideration.

gera2ld commented 3 years ago

I think buttons or checkboxes are not very effective either. How about using plain text selections as done in SourceTree? I think SourceTree does a great job in this. The text selections are not meant for exact content selection but rather a range to indicate which lines are involved. In this way no checkbox or button is needed. If we don't select anything, the whole diff is patched. Otherwise the lines including selected text will be patched.

wmertens commented 3 years ago

@gera2ld That actually involves more clicking, because you can only have one selection: Select something, move to buttons, click "patch", repeat, and you still have to show the lines that are part of the patch.

gera2ld commented 3 years ago

@wmertens But it should be easier when you try to pick 10 lines from 20 lines of changes.

What about this:

In this way we don't need checkboxes any more, highlighting the picked changes is enough.

ps. It looks weird somehow, since ungit does not have a staged status, so the picked and unpicked changes are always shown together. pps. I think using checkboxes is not a good choice, especially when there might be hundreds of them.

Sogolumbo commented 3 years ago

@gera2ld From what I understand right now your proposal would have almost the same workflow with less possibilities.

What I suggested was to apply a text selection like control to the checkboxes that represent the lines. You argue that checkboxes are no good choice. Why do you think so? I think they're ok. Everybody understands the meaning of checkboxes when text selection is less clear. I think selecting lines directly (rather than checkboxes) and displaying that with different background colors of the lines would be a nice solution design wise but it will not be immediately clear what the colors mean.

There's another thing: sometimes I find an error in the code while committing. Sometimes I want to keep a part of the old version. In these cases I want to be able to select and copy the text straight from the commit/patch text area. I admit that this might be an unusual use case. If so I'm ok with using the whole line as a "click area".

gera2ld commented 3 years ago

From what I understand right now your proposal would have almost the same workflow with less possibilities.

The results should be the same. The text selection is used to select lines and you have to click another button like "toggle" to check/uncheck the selected lines or checkboxes. So it's just a way to handle multiple checkboxes at a time.

Everybody understands the meaning of checkboxes when text selection is less clear.

The problem is, you can hardly tell which rows are picked from a large block of code text when the only difference is in the checkbox at the beginning of each line. If the picked and unpicked code lines are marked with different colors (maybe different opacities are better), there is no need to have checkboxes. I guess it's also easy to understand that a line with more opaque background is picked or staged. Actually I think it's much better to display staged/unstaged code lines separately (as in SourceTree), then we don't need to care about the colors or checkboxes and it should be even clearer.

Sogolumbo commented 3 years ago

I just had a look at Sourcetree. I think they have a very powerful set of tools for staging. One of the most important ones is to autodetect blocks of codes in each file that can be added and removed with one click. The difference between Source Tree and ungit is that Source Tree has a seperate section for staged changes. For that design it makes sense to have a workflow with a seperate ("add/remove from staging area") button.

But for the current design which has no explicit staging area the Click & Drag seems to be more efficient. I think I got this Idea from the GitHub App which also hides the staging area and has a wide selection strip that supports Click & Drag.

If you really want a seperate staging area maybe add your Ideas to #99 and #98.