The icons now check the colours for all possible states of the button, and I've added a unit test to keep codecov happy.

The Tk checkbutton does allow setting a different image for the selected state using -selectimage, but I don't think it supports different text colours for the selected state, so I assumed whatever is set as the text colour must work for both states. Should I just default to the black icon if selectcolor isn't dark then?

Also, can I ask if you're using any library to set the colours in your example, or are you setting them yourself?

Should I just default to the black icon if selectcolor isn't dark then?

Yes.

Also, can I ask if you're using any library to set the colours in your example, or are you setting them yourself?

You mean the sceenshot?

That's just the standard KDE "breeze dark" theme.

Yes.

Alright, that should be easy to do. Will update soon.

That's just the standard KDE "breeze dark" theme.

Ah, I've never used KDE myself so not familiar with how Tk reads its theme files. Out of interest, how does a regular tk.Checkbutton(master, indicatoron=False, text="sometext") look in the default and selected states? If it does somehow set a different foreground colour for the selected state, it should be possible to retrieve and apply that to the icon.

You mean from:

import tkinter as tk
 
app = tk.Tk() 
app.geometry('150x100')
bt1 = tk.Checkbutton(app, text='Check Box', indicatoron=False) 
bt1.grid(column=0, row=0)
bt2 = tk.Checkbutton(app, text='Check Box', indicatoron=False) 
bt2.grid(column=0, row=1)
app.mainloop()

grafik

Seems it doesn't play well with the theme either.

That's exactly what I meant, thanks! In that case since the theme doesn't change the foreground colour for the selected state, it should be okay to just use the default icon colour if the background is light.

@timhoffm this should now cover all relevant cases, could you confirm that it also works with your themes please? Should also be more readable, at the cost of storing both images even when not needed.

Of course, ToolbarTk calls _Button with itself as an argument, so all new functions either have to be nested or called explicitly from the class...

Test should hopefully be fixed now.

@daniilS there's a difference in the selected buttons depending on mouse hover. To keep it simple I'd use the same image for hovered and unhovered. It's important that both are readable. It's not so important to indicate the hover state.

Unselected:
grafik

Selected with mouse hover:
grafik

Selected without mouse hover:
grafik

@timhoffm Tk lets you specify the background colour for the hover state, but only differentiates between images depending on whether the button is selected or not, so the same image is already being used.

I think the actual issue might be that on X11, Tk has some special logic for the selected state, rather than using selectcolor directly. Could you try the latest commit to see if that fixes it? (On Windows, I can't reproduce your screenshots: )
Unselected:
unselected
Unselected, hover/click:
unselected-active
Selected:
selected
Selected, hover/click:
selected-active

Unselected:
grafik

Unselected, hover/click:
grafik

Selected:
grafik

Selected, hover/click:
grafik

There's now hover-change for unselected, but that's ok by me. It's all readable.

The background colour of the hover state is set by the theme, not this PR. Looks like the colours of the icons are now working on X11 too then. I've squashed the commit with the X11 fix, so this is ready for review/merge now.

Rebased and updated for the new test test scheme (twice).

Thanks @tacaswell, anything else I need to do on this or good to go now?

@daniilS If you could confirm that I did not break this while rebasing I'll go ahead and merge it!

@tacaswell all working so should be good to merge!