[haiku-commits] Re: r39630 - in haiku/trunk/src: add-ons/decorators/SATDecorator servers/app
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Thu, 25 Nov 2010 22:59:42 +0100
On 2010-11-25 at 21:31:12 [+0100], Stephan Aßmus <superstippi@xxxxxx> wrote:
> Am Donnerstag, den 25.11.2010, 21:07 +0100 schrieb ingo_weinhold@xxxxxx:
> > +void
> > +SATDecorator::GetComponentColors(Region component, ComponentColors
> > _colors)
> > +{
> > + switch (component) {
> > + case REGION_TAB:
> > + if (fBordersHighlighted) {
> > + _colors[COLOR_TAB_FRAME_LIGHT] =
> > kHighlightFrameColors[0];
> > + _colors[COLOR_TAB_FRAME_DARK] = kHighlightFrameColors[5];
> > + } else {
> > + _colors[COLOR_TAB_FRAME_LIGHT] = kFrameColors[0];
> > + _colors[COLOR_TAB_FRAME_DARK] = kFrameColors[3];
> > + }
>
> this is something that came to my mind from reading the initial patch:
> The type name "Region" is perhaps misleading especially in the
> app_server context. In the above method, you already call the parameter
> "component". Another option is "part", which seems to be used in the Qt
> API. Perhaps "DecoratorPart" (unless it's already "Decorator::Part"
> anyway). What do you think?
The reason why I called it "component" in this context instead of "region" is
because the semantics is somewhat different. I intended the Region in the
base class to identify the functional aspect, while in
DefaultDecorator::GetComponentColors() I mean the concrete graphical
components of the default decorator. Admittedly I was just too lazy to
introduce a new enum for that purpose. Probably should do just that.
Regarding the name of Decorator::Region, I doubt that there's much potential
for confusing it with other regions in the app server, but I don't mind, if
anyone renames it (after I'm done working in this area).
CU, Ingo
Other related posts:
