About the current API… The GSTheme color lookup API includes a method -colorNamed:state:, but the state is almost never used based on a quick grep over the Gui source code (only once in NSMenuItemCell.m). So I would remove the last argument and encode the state within the color name. NSColor API names the color like that, this approach would avoid to introduce two color naming schemes (GTheme vs NSColor) for the themable colors.
The state arguments makes sense with the tile lookup methods, but for the themable colors I'm not convinced.
Having color methods declared in GSTheme (such as the toolbar one) as syntactic sugar over -colorNamed: doesn't go against the current color lookup. It might even be a good way to clearly document the colors that can be customized.
I like your new themeControlState method. We should use this in all the other places where it is useful, for example in the method backgroundColor.
Yes, it could even be used in other classes such as -[NSButtonCell drawBezelWithFrame:inView:].
This is actually the only place where you use one of your new colour methods outside of the GSTheme class, maybe we should just change the lookup key here to menuItemBackgroundColor and adjust all the themes?
For -[NSMenuItemCell backgroundColor], I would suggest to remove:
color = [[GSTheme theme] colorNamed: @"NSMenuItem" state: state];
if (color == nil)
and use either:
if ((state == GSThemeHighlightedState) || (state == GSThemeSelectedState))
{
color = [NSColor selectedMenuItemColor];
}
else
{
color = [[GSTheme theme] menuItemBackgroundColor];
}
or
if ((state == GSThemeHighlightedState) || (state == GSThemeSelectedState))
{
color = [NSColor selectedMenuItemColor];
}
else
{
color = [[GSTheme theme] colorNamed: @"menuItemBackgroundColor"];
}
Should I use this last solution?
Then there is this call in your code:
NSInterfaceStyleForKey(@"NSMenuInterfaceStyle", nil);
We have plenty of similar calls all over the place, still I think this is wrong. When ever possible we should pass in a responder as the second argument of this function call. Every view is allowed to have its specific interface style and at least the standard drawing code should respect this. Themes may handle this differently or just ignore that value.
I'll remove this call in -menuSeparatorColor, it isn't needed now that the Windows theme uses native menus as Gregory pointed it out in the bug report #34792.