Hi,
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:
----- Doug Torrance <dtorra...@monmouthcollege.edu> a écrit :
> [...]
>
> diff --git a/WINGs/WINGs/WINGs.h b/WINGs/WINGs/WINGs.h
> index 0104e03..ff9eaef 100644
> --- a/WINGs/WINGs/WINGs.h
> +++ b/WINGs/WINGs/WINGs.h
> @@ -349,6 +349,8 @@ typedef struct W_Pixmap WMPixmap;
> typedef struct W_Font WMFont;
> typedef struct W_Color WMColor;
>
> +typedef struct WMTheme WMTheme;
> +
For consistency, should not this be:
struct W_Theme WMTheme
> typedef struct W_Screen WMScreen;
>
> typedef struct W_View WMView;
> @@ -881,6 +883,8 @@ unsigned short WMGetColorAlpha(WMColor *color);
>
> char* WMGetColorRGBDescription(WMColor *color);
>
> +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
> /* ---[ WINGs/widgets.c ]------------------------------------------------- */
>
> WMScreen* WMWidgetScreen(WMWidget *w);
> diff --git a/WINGs/WINGs/WINGsP.h b/WINGs/WINGs/WINGsP.h
> index de01412..df5cfc4 100644
> --- a/WINGs/WINGs/WINGsP.h
> +++ b/WINGs/WINGs/WINGsP.h
> @@ -64,6 +64,17 @@ typedef struct W_DraggingInfo {
> struct W_DragDestinationInfo* destInfo; /* infos needed by destination */
> } W_DraggingInfo;
>
> +/* ---[ wcolor.c ]----------------------------------------------------- */
> +
> +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?
> [...]
> @@ -265,6 +276,8 @@ typedef struct W_Screen {
> struct W_View *modalView;
> unsigned modalLoop:1;
> unsigned ignoreNextDoubleClick:1;
> +
> + WMTheme *theme;
> } W_Screen;
I would suggest to not use a pointer here, but use the structure in place.
> [...]
> - scr->white = WMCreateRGBColor(scr, 0xffff, 0xffff, 0xffff,
> True);
> + scr->white = scr->theme->highlight;
> [...]
> - scr->black = WMCreateRGBColor(scr, 0, 0, 0, True);
> + scr->black = scr->theme->foreground;
I'm not sure this is the right way to do it. I would think that 'white' should
be white, and not some other color; should not it be the widget that use
scr->theme.xxx instead of explicit scr->white?
> [...]
> +WMTheme *W_GetTheme(WMScreen *scr)
Considering what the function does, I am not sure the name is right. This name
suggests me:
WMTheme *W_GetTheme(WMScreen *scr) { return scr->theme; }
Where what is does is more loading the theme and installing it (W_InstallTheme?)
> +{
> + 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.
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
