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

Commit Form: Display current branch name #4189

Merged
merged 1 commit into from Dec 18, 2017

Conversation

pmiossec
Copy link
Member

@pmiossec pmiossec commented Dec 3, 2017

next to the branch creation button

Screenshots after:
image

Has been tested on (remove any that don't apply):

  • GIT 2.14
  • Windows 10

@RussKie
Copy link
Member

RussKie commented Dec 4, 2017

Could you please explain the benefits.
Users can see the current branch in the title of the commit dialog.
The next release also carries #3993.

@pmiossec
Copy link
Member Author

pmiossec commented Dec 4, 2017

I was aware of all of that but feel it was not enough.

In fact, the branch name in the title, even if being useful, is hard to read and even much more harder to discover. I think very few user know it's there (written in small with other dense data and not where you look) and use it.

Even if I know it's here, I much often close the form just to see it in the browse form.

And also, the data is displayed just next to the place where you are just before committing and there is a lot more chance that you see that you are not in the good branch when writing the commit message (you have quite no effort to do to see it while you have to deliberately to look at it when just displayed in the title bar). It fills the principle that the data must be displayed the nearer the place it should be used.

And it make much more sense to put the information here since we added the "create a branch" button here

@pmiossec pmiossec force-pushed the display_current_branch_name branch 2 times, most recently from bb5117f to bacf470 Compare December 13, 2017 21:13
@RussKie
Copy link
Member

RussKie commented Dec 14, 2017

Sorry Philippe, I am still unconvinced this change enhances the usability of the form. To me it adds visual clutter to the already busy dialog...

What does everyone else think? @jbialobr @KindDragon @gerhardol @vbjay @mdonatas @Drake103 @gpongelli

. . .

I can offer you an alternative task, if you wish to make the dialog more usable - add a toolstrip menu to it, which will host all available features to the user - no more hidden context menus in various controls or various drop downs in different parts.
And if we could add a ribbon in the next version, we could take the usability to the new level.

@jbialobr
Copy link
Member

I think that it is not a good place to show the current branch. Perhaps to be more consistent it could be placed in the status bar at the bottom of the window next to the Commiter info.

@mdonatas
Copy link
Contributor

I've never encountered problems with this as I tend to check the active branch before commit. Also like RussKie said it's visible in the title and I agree that this would not add much benefit but definitely would add clutter.

@jbialobr
Copy link
Member

I've never encountered problems with this as I tend to check the active branch before commit.

It is useful when you commit submodules changes by opening the commit dialog from the commit dialog of the superproject.

@pmiossec
Copy link
Member Author

I've never encountered problems with this as I tend to check the active branch before commit.

@mdonatas I like this comment. It seems to prove exactly what I try to fix here ;) I see it like you were obliged to adopt an habit because there was a lack when you use GitExtensions the first times.
I adopt exactly the same but found it boring when, for different reason, I forgot the branch I am one and is obliged to close or resize the form to be able to see it in the browse form. I decide to do the PR the day I commit on the wrong branch and I though I could have seen it if it was not hidden in the title bar but where your eyes are looking when committing.

And this argument is not at all relevant since the button "create branch" has been added to the commit form... If we verify on which branch we are, their is no need to have the button because we already have created the branch before opening the commit form. And how do we know (easily/ergonomically) on which branch we are?

Also like RussKie said it's visible in the title

Can you tell me, honestly, which users (except you and me that are power users of GitExtensions) are looking for information in the title bar of an application? Do you find it discoverable and ergonomic? Could you rembember when you discovered that the branch name was displayed here?

I ask to my team of 20 and (quite) none was aware of that. The title bar is a summary and usefull data must be displayed also elsewhere in the form.

I agree that this would not add much benefit but definitely would add clutter.

&

I think that it is not a good place to show the current branch. Perhaps to be more consistent it could be placed in the status bar at the bottom of the window next to the Commiter info.

It seems that everybody agree on the fact that it's not displayed in the right place. I hesitate between this place, next to the "create branch" button, and the status bar.
I choose this place because I agree with the fact that a data should be displayed as close as possible to the place where it will be used.

But I'm OK to change that to display it in the status bar.
I think it's at least as much relevant to display the branch name somewhere as to display the commit identity displayed in the left (who read that? ;) )
I think it won't fix as efficiently (because few users are reading the status bar) but it probably should be enough and I'm OK with that.

@gerhardol
Copy link
Member

To make this conversation even more complicated: I would like to have the commit in the Browse form. Some discussions in #4031, will probably open a new issue regarding this (or continue on the really old one). I have played around with it but not submitted anything as there will be a lot of duplicated code. This addition does not change much in this matter.

However, a non-modal commit dialogue will introduce a different way of working where you commit to where you stand, not change where you stand while committing. So you do not have to improve the information where you stand right now (in the dialogue).

I do not have a strong opinion about this specific change: I maybe prefer the way it is displayed but also feel like this is adding code we do not really need.

@vbjay
Copy link
Contributor

vbjay commented Dec 14, 2017

I like the idea of showing the branch name. I just don't like where it's located. I think it should be at the top of the form inside and info panel. Things like last commit summary and what branch is being committed to could be in a group at the top. That way, we don't have to fool with limited horizontal space and we could make the info panel scrollable to limit the vertical space. The panel could be collapsed and remember that state.

@gpongelli
Copy link
Contributor

gpongelli commented Dec 14, 2017 via email

@RussKie
Copy link
Member

RussKie commented Dec 15, 2017

It looks like we have some sort of general agreement.

@pmiossec could please add the current branch information to the status bar, to the right of the committer info. Something like:
image

@gpongelli please raise a new feature request for GPG related changes.

thank you all

@gpongelli
Copy link
Contributor

@gpongelli please raise a new feature request for GPG related changes.

Actually I've no idea on which changes submit with PR.
I'll open a new issue to discuss about the UI and logic change, I would avoid an "over 200 comment" PR again :)

@pmiossec
Copy link
Member Author

I have put the branch name in the status bar.

The look of the new version:
image

@RussKie
Copy link
Member

RussKie commented Dec 17, 2017 via email

@RussKie
Copy link
Member

RussKie commented Dec 18, 2017

@pmiossec the only questions remains - the license attached to the added image.
Please update \GitUI\Resources\Icons\originals\_Icon Sources.txt with the source and the licence for the image.
We also already have \GitUI\Resources\Icons\originals\branch-led-orig.png image, which looks very similar to the one you've added. I'm not sure if you've seen it.

@RussKie RussKie added this to the 2.51 milestone Dec 18, 2017
@pmiossec
Copy link
Member Author

We also already have \GitUI\Resources\Icons\originals\branch-led-orig.png image, which looks very similar to the one you've added. I'm not sure if you've seen it.

I search for it but didn't find it (and it seems strange to me that it doesn't exist). Happy you found it. I will replace the icon...

@pmiossec
Copy link
Member Author

@RussKie done. Thanks for spotting the icon!

@RussKie RussKie merged commit ee6cafa into gitextensions:master Dec 18, 2017
@RussKie
Copy link
Member

RussKie commented Dec 18, 2017

Good work, thank you

@jbialobr
Copy link
Member

Wouldn't it be better to group the branch name with the commiter on the left side? On the right side there is commit form specific data. On the left side there could be global data area.

@pmiossec pmiossec deleted the display_current_branch_name branch January 8, 2018 12:26
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.

None yet

7 participants