Skip to content

gitk: make color preferences visually more pleasing and better usable - #2194

Open
j6t wants to merge 5 commits into
gitgitgadget:masterfrom
j6t:gitk-prefs-color-buttons
Open

gitk: make color preferences visually more pleasing and better usable#2194
j6t wants to merge 5 commits into
gitgitgadget:masterfrom
j6t:gitk-prefs-color-buttons

Conversation

@j6t

@j6t j6t commented Aug 5, 2026

Copy link
Copy Markdown

I find the user interface to set the color preferences a bit ugly for these reasons:

  • The color samples are not clickable as one would expect who enters the dialog. Instead, the description is the button that must be clicked.

  • Since the descriptive texts are different for the preferences, the width of the buttons are different, too.

  • The descriptions themselves are not always natural language (read: they are nerdy) and use abbreviations.

This series makes the descriptions static text and turns the color samples into the clickable buttons. It also makes the descriptions and dialog titles more natural language.

This is the dialog before the change:
Screenshot_color_buttons_before
And this is the dialog after the change:
Screenshot_color_buttons_after

cc: mark mlevedahl@gmail.com

@j6t

j6t commented Aug 5, 2026

Copy link
Copy Markdown
Author

/submit

@gitgitgadget

gitgitgadget Bot commented Aug 5, 2026

Copy link
Copy Markdown

Error: git --git-dir=pr-repo.git fetch --no-tags https://github.com/gitgitgadget/git/ -- +refs/notes/gitgitgadget:refs/notes/gitgitgadget +refs/heads/maint:refs/remotes/upstream/maint +refs/heads/seen:refs/remotes/upstream/seen +refs/heads/master:refs/remotes/upstream/master +refs/heads/next:refs/remotes/upstream/next +refs/pull/2194/head:refs/pull/2194/head +refs/pull/2194/merge:refs/pull/2194/merge failed: 128,
fatal: couldn't find remote ref refs/pull/2194/merge

@j6t

j6t commented Aug 5, 2026

Copy link
Copy Markdown
Author

Hmm, these patches are sourced directly from j6t/gitk/pull/48, so the edits lack the gitk-git/ prefix. Is anything special needed to handle this in Gitgitgadget?

@dscho

dscho commented Aug 5, 2026

Copy link
Copy Markdown
Member

Is anything special needed to handle this in Gitgitgadget?

Yes, I think we also need to mirror gitk/master in addition to git-gui/master, then...

@dscho
dscho changed the base branch from master to gitk/main August 5, 2026 12:32
@dscho
dscho changed the base branch from gitk/main to gitk/master August 5, 2026 12:32
@dscho

dscho commented Aug 5, 2026

Copy link
Copy Markdown
Member

@j6t I re-ran the workflow run that wanted to handle your /submit.

@gitgitgadget

gitgitgadget Bot commented Aug 5, 2026

Copy link
Copy Markdown

Error: Unrecognized project

@dscho

dscho commented Aug 5, 2026

Copy link
Copy Markdown
Member

Error: Unrecognized project

That's what I feared. There's special logic in GitGitGadget for Git GUI... And none yet for gitk.

@dscho

dscho commented Aug 5, 2026

Copy link
Copy Markdown
Member

There's special logic in GitGitGadget for Git GUI... And none yet for gitk.

See https://github.com/gitgitgadget/gitgitgadget/blob/cadedf4eec007b3798f61e825af876cc7adc09ba/lib/project-options.ts#L27-L31

@j6t

j6t commented Aug 5, 2026

Copy link
Copy Markdown
Author

@dscho What if I just rewrite the branch by cherry-picking the commits into the git code? The branch here would be abandoned once I merge my original PR in the gitk repository, but at least I should be able to use Gitgitgadget as the ML gateway.

dscho commented Aug 5, 2026

Copy link
Copy Markdown
Member

That would make for a fine stop-gap solution.

j6t added 5 commits August 5, 2026 20:02
After the user has selected a color in the Preferences dialog, the
helper proc prefspage_set_colorswatches is used update the colors shown
in the Preferences dialog. Use this proc also after the Preferences
dialog is constructed to show the initial colors. This keeps the
procedure that gives the UI elements their colors in a single place.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
The color selection elements in the Preferences dialog are set up in a
very uniform manner. The code doing that has grown in the past by simply
modifying a copy of a paragraph. Extract the varying parts into a list
and then operate the repeating parts in a foreach loop. This helps a
later change where we want to set up the UI elements in a different way.

Change the UI names "markbgsep" and "selbgsep" to drop the "sep", which
has obviously been left over by accident when "hunksep" was copied.

Change the suffix "but" to "btn", which resonates a bit better when
reading the code.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
When the user goes to the color preferences, the eye is automatically
drawn to the color samples. However, clicking on the sample with the
intent to change it does nothing. Instead, the text label also acts as
a button and must be clicked to change the color.

Turn the color samples into clickable buttons and the text labels into
static text. Use non-themed buttons because their color can be changed
by simply specifying the color properties. (Themed buttons would need
a lot more elaboration.)

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
The labels used to be clickable buttons. These would have looked funny
if they carried long texts. For this reason, abbreviations and "nerdy"
texts were used to keep them short. Since these labels are now static
text, bring them closer to natural language.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
When the user clicks a color preference, a color selection dialog is
presented whose title is provided in parts by the caller. The dialog
implementation must supply the rest of the title. This is unfriendly
for translations. Provide the full title by the caller. Rewrite the
texts to be more natural language.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
@j6t
j6t changed the base branch from gitk/master to master August 5, 2026 18:13
@j6t
j6t force-pushed the gitk-prefs-color-buttons branch from 22c2797 to 1e1bcfa Compare August 5, 2026 18:14
@j6t

j6t commented Aug 6, 2026

Copy link
Copy Markdown
Author

/submit

@gitgitgadget

gitgitgadget Bot commented Aug 6, 2026

Copy link
Copy Markdown

Submitted as pull.2194.git.1785998419.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2194/j6t/gitk-prefs-color-buttons-v1

To fetch this version to local tag pr-2194/j6t/gitk-prefs-color-buttons-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2194/j6t/gitk-prefs-color-buttons-v1

Comment thread gitk-git/gitk
@@ -11801,8 +11801,6 @@ proc prefspage_general {notebook} {
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mark wrote on the Git mailing list (how to reply to this email):

On 8/6/26 2:40 AM, Johannes Sixt via GitGitGadget wrote:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> After the user has selected a color in the Preferences dialog, the
> helper proc prefspage_set_colorswatches is used update the colors shown
> in the Preferences dialog. Use this proc also after the Preferences
> dialog is constructed to show the initial colors. This keeps the
> procedure that gives the UI elements their colors in a single place.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
This completes collecting the color settings to one place, Good!

@gitgitgadget

gitgitgadget Bot commented Aug 7, 2026

Copy link
Copy Markdown

User mark <mlevedahl@gmail.com> has been added to the cc: list.

Comment thread gitk-git/gitk
@@ -11834,75 +11832,72 @@ proc prefspage_colors {notebook} {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mark wrote on the Git mailing list (how to reply to this email):

On 8/6/26 2:40 AM, Johannes Sixt via GitGitGadget wrote:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> The color selection elements in the Preferences dialog are set up in a
> very uniform manner. The code doing that has grown in the past by simply
> modifying a copy of a paragraph. Extract the varying parts into a list
> and then operate the repeating parts in a foreach loop. This helps a
> later change where we want to set up the UI elements in a different way.
> 
> Change the UI names "markbgsep" and "selbgsep" to drop the "sep", which
> has obviously been left over by accident when "hunksep" was copied.
> 
> Change the suffix "but" to "btn", which resonates a bit better when
> reading the code.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  gitk-git/gitk | 114 ++++++++++++++++++++++++--------------------------
>  1 file changed, 55 insertions(+), 59 deletions(-)
> 
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index f5eb963b86..137940defb 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -11832,55 +11832,46 @@ proc prefspage_colors {notebook} {


> -
> -    label $page.linkfg -padx 40 -relief sunk
> -    ttk::button $page.linkfgbut -text [mc "Link"] \
> -        -command [list choosecolor linkfgcolor {} $page [mc "link"]]
> -    grid x $page.linkfgbut $page.linkfg -sticky w
> +
> +    set coloruielems [list \
> +        bg          bgcolor {} \
> +                    [mc "Background"] \
> +                    [mc "background"] \
> +        fg          fgcolor {} \
> +                    [mc "Foreground"] \
> +                    [mc "foreground"] \
The list is computed and used each time a dialog box is started from all static elements,
no real change. But this cleanly separates the per item data from the boiler plate,
definitely more readable. Good.

Comment thread gitk-git/gitk
linkfg linkfgcolor {} \
[mc "Link"] \
[mc "link"] \
]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mark wrote on the Git mailing list (how to reply to this email):

On 8/6/26 2:40 AM, Johannes Sixt via GitGitGadget wrote:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> When the user goes to the color preferences, the eye is automatically
> drawn to the color samples. However, clicking on the sample with the
> intent to change it does nothing. Instead, the text label also acts as
> a button and must be clicked to change the color.
> 
> Turn the color samples into clickable buttons and the text labels into
> static text. Use non-themed buttons because their color can be changed
> by simply specifying the color properties. (Themed buttons would need
> a lot more elaboration.)
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  gitk-git/gitk | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 137940defb..877b2ad27f 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -11867,10 +11867,10 @@ proc prefspage_colors {notebook} {
>      ]
>  
>      foreach {uielem colorvar idx label title} $coloruielems {
> -        label $page.$uielem -padx 40 -relief sunk
> -        ttk::button $page.${uielem}btn -text $label \
> +        ttk::label $page.$uielem -text $label
> +        button $page.${uielem}btn -padx 40 -pady 0 \
>              -command [list choosecolor $colorvar $idx $page $title]
> -        grid x $page.${uielem}btn $page.$uielem -sticky w
> +        grid x $page.$uielem $page.${uielem}btn -sticky w
>      }
>  
>      grid columnconfigure $page 2 -weight 1
> @@ -11896,7 +11896,7 @@ proc prefspage_set_colorswatches {page} {
>          linkfg    $linkfgcolor \
>      ]
>      foreach {uielem color} $coloruielems {
> -        $page.$uielem configure -background $color
> +        $page.${uielem}btn configure -background $color -activebackground $color
>      }
>  }
>  

The original approach has unthemed labels, allowing easy specification of color, and
themed buttons. With this change, the roles reverse, so swapping themed / unthemed makes
sense. Good.

But, the colored buttons do not stand out to my eye as much as in the prior approach.
Perhaps use -borderwidth 2? This is totally subjective, I'm ok if you leave this as-is.

Comment thread gitk-git/gitk
-command [list choosecolor linkfgcolor {} $page [mc "link"]]
grid x $page.linkfgbut $page.linkfg -sticky w

set coloruielems [list \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mark wrote on the Git mailing list (how to reply to this email):

On 8/6/26 2:40 AM, Johannes Sixt via GitGitGadget wrote:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> When the user clicks a color preference, a color selection dialog is
> presented whose title is provided in parts by the caller. The dialog
> implementation must supply the rest of the title. This is unfriendly
> for translations. Provide the full title by the caller. Rewrite the
> texts to be more natural language.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  gitk-git/gitk | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 9e735d91bf..454f54dca8 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -11836,34 +11836,34 @@ proc prefspage_colors {notebook} {
>      set coloruielems [list \
>          bg          bgcolor {} \
>                      [mc "Background"] \
> -                    [mc "background"] \
> +                    [mc "choose background color"] \
>          fg          fgcolor {} \
>                      [mc "Main text"] \
> -                    [mc "foreground"] \
> +                    [mc "choose main text color"] \

> @@ -12014,11 +12014,11 @@ proc choose_themeloader {prefspage} {
>      }
>  }
>  
> -proc choosecolor {v vi prefspage x} {
> +proc choosecolor {v vi prefspage title} {
>      global $v
>  
>      set c [tk_chooseColor -initialcolor [lindex [set $v] $vi] \
> -               -title [mc "Gitk: choose color for %s" $x]]
> +               -title "Gitk: $title"]
>      if {$c eq {}} return
>      lset $v $vi $c
>      set_gui_colors

This with the prior patch should help translation due to use of more common idioms and
complete phrases rather than jargon. Good.

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