On 11/05/2013 05:58 PM, Christophe wrote:
> Although I do like the idea brought by the patch, there are a few things in
> this implementation that I would suggest to do differently:
Absolutely! I'm a mathematician, not a programmer, so I appreciate all
the input I can get!
>> +WMTheme* W_GetTheme(WMScreen *scr);
> I think this should appear in the visible API of the toolkit.
> A theme is something internal to the toolkit, the application using it should
> not see it
>
I'm a little confused here. Are you suggesting that themes should be
visible or not? Also, what determines this? Is it WINGs.h vs.
WINGsP.h? I couldn't find any documentation on what goes where, so I
just tried to follow the example of what was already there.
>> +typedef struct WMTheme {
>> + WMColor *background;
>> + WMColor *foreground;
>> + WMColor *shadow;
>> + WMColor *highlight;
>> + WMColor *unselectedTabBackground;
>> + WMColor *unselectedTabHighlight;
>> +} WMTheme;
> I would suggest to rethink the names and organisation of this structure,
> because there will need more, for example: background is for what? the
> standard bg of the window I suppose, but text fields are likely to have a
> different background, aren't they? and the same happens for the foreground,
> no?
>
Absolutely. As it stands now, the patch is mostly just a quick hack to
replace what WINGs historically has called "white", "black", "gray", and
"darkGray". I chose names that represented what these colors (most of
the time) are used for. There are definitely major issues with these
choices. (White is a big problem. It is used for button highlights,
selected text, and textfield backgrounds, all of which are very different.)
The ultimate solution I think would be to gut the hardcoded names
entirely and create more descriptive names. I'm beginning to work on
this, but it will take some time. :)
>> +{
>> + WMTheme *theme;
>> + WMUserDefaults *defaults;
>> +
>> + char *background;
>> + char *foreground;
>> + char *shadow;
>> + char *highlight;
>> + char *unselectedTabBackground;
>> + char *unselectedTabHighlight;
>> +
>> + defaults = WMGetStandardUserDefaults();
>> +
>> + if (defaults) {
>> + background = WMGetUDStringForKey(defaults, "Background");
>> [...]
>> +
>> + theme->background = WMCreateNamedColor(scr,background,True);
>> + if (!theme->background)
>> + theme->background = WMCreateRGBColor(scr, 0xaeba, 0xaaaa,
>> 0xaeba, True);
> I'm not sure this behaves as good as you expect.
>
What do you mean?
Thanks again. I really appreciate it!
Doug
