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

Implementation of issue #1582 #1875

Merged
merged 60 commits into from
Jul 8, 2013
Merged

Implementation of issue #1582 #1875

merged 60 commits into from
Jul 8, 2013

Conversation

jbialobr
Copy link
Member

I think it is ready for tests and review.

jbialobr and others added 30 commits March 28, 2013 22:12
Conflicts:
	GitCommands/Config/ConfigFile.cs
Conflicts:
	GitCommands/GitCommands.csproj
Conflicts:
	GitCommands/Settings/AppSettings.cs
	GitExtensionsTest
	GitUI/GitUICommands.cs
Submodule GitExtensionsTest:
    > 604b166 - Settings
Conflicts:
	GitCommands/Git/GitModule.cs
	GitExtensionsTest
	GitUI/CommandsDialogs/FormCheckoutBranch.cs
@feinstaub
Copy link
Contributor

I very much like that the << is hidden when Effective is not chosen:
2013-06-10_21-20-55_settings - revision links
2013-06-10_21-21-52_settings - revision links

@feinstaub
Copy link
Contributor

In "Revision links" settings: when creating a new "category" in Effective it will be stored in "Global for all repos" settings source. Is this intended?

@jbialobr
Copy link
Member Author

Yes, the idea is to set setting at the lowest possible level.

@feinstaub
Copy link
Contributor

"lowest" in the the sense of "with most impact"?

@jbialobr
Copy link
Member Author

"lowest" - the lowest level that has to be overriden to take effect on effective view

@jbialobr
Copy link
Member Author

There is exception for "Distributed with current repository" - it can't be set via effective view.

@feinstaub
Copy link
Contributor

so the more "low" the more "global" the setting is stored? EDIT: Or am I thinking in the wrong terms?

@jbialobr
Copy link
Member Author

Yes

@feinstaub
Copy link
Contributor

Two small thinks:

  1. Why not using standard check box for "Enabled" (with check box on the left)?
    2013-06-10_21-50-58_settings - revision links
  2. There are two search text boxes: just for convenience to have two expressions combined with "or"?

@jbialobr
Copy link
Member Author

  1. I think this way is consistent with other captions.
  2. Search pattern is for matching fragment, but not necessary whole matched fragment has to be used in a link. For the case you don't want whole matched fragment you can use "Nested pattern" (I can see caption on your picture) to extract desired fragment. Here is an example https://github.com/gitextensions/gitextensions/blob/jb/RevisionLinks%231582/GitExtensions.settings#L23.

@feinstaub
Copy link
Contributor

  1. I've seen that at a few other places in GitExt but not in other software that I use. I find it a bit confusing (although I see the intention of the equal aligment of the boxes; I like it for example more when I am able to also click on the text caption to trigger the checkbox)
  2. Now I see the "Nested pattern", too. But there is a bug: when I open the Settings Dialog and then go to Revision links I see the caption. As soon as I choose for example "Local for current repo" the caption disappears as in the screenshot

@jbialobr
Copy link
Member Author

  1. Could you provide srceenshot that shows how you would like to place it on the form?
  2. Thanks, fixed.

@vbjay
Copy link
Contributor

vbjay commented Jun 11, 2013

The checkboxes are fine like that. If all labels are aligned that way then it is clear where the checkbox's caption is. The search in is clear that you are choosing different places to search in. Enabled doesn't have a sub item. It is a setting of its own.

@feinstaub
Copy link
Contributor

  1. Maybe like this:
    2013-06-11_18-39-54_settings - revision links

or this (right aligned rest of the captions):
2013-06-11_18-41-36_settings - revision links

or put Enabled to top:
2013-06-11_18-46-08_settings - revision links

In my view the "Enabled" checkbox serves a different purpose compared to the other controls ("enabled/not enabled" vs. "configuration details") and therefore their is no need to make it look "consistent".

@jbialobr
Copy link
Member Author

For me it looks like a developer forgot to align this checkbox in your examples.

@feinstaub
Copy link
Contributor

similar example taken from VLC with an Enable checkbox:
2013-06-11_21-38-33_preferences

MS Word autocorrection options with "out of line" checkbox:
2013-06-11_21-45-18_autokorrektur

Inkscape with checkboxes and other controls:
2013-06-11_21-43-14_inkscape preferences shift ctrl p

So don't get me wrong, I'm not so much eager to discuss aesthetics of the current design vs. my screenshots but to point out that the checkbox box without caption on the right is very non-standard and I actually cannot come up with a popular program that uses this non-standard design.

@jbialobr
Copy link
Member Author

Maybe next to the name textbox is better?
obraz

@feinstaub
Copy link
Contributor

Maybe next to the name textbox is better?

I like this solution. :) (in the screenshot maybe the Name textbox width could be a bit larger for more elaborate names).

@jbialobr
Copy link
Member Author

Thanks, ok.

Conflicts:
	GitUI/CommandsDialogs/FormSettings.Designer.cs
	GitUI/CommandsDialogs/SettingsDialog/Pages/GitExtensionsSettingsPage.cs
	GitUI/UserControls/RevisionGrid.cs
@jbialobr jbialobr merged commit c5bbcef into master Jul 8, 2013
@KindDragon KindDragon deleted the jb/RevisionLinks#1582 branch July 8, 2013 20:25
jbialobr referenced this pull request in gitextensions/GitExtensionsDoc Jul 14, 2013
PKRoma pushed a commit to PKRoma/gitextensions that referenced this pull request Aug 20, 2014
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

4 participants