raised & inset borders
Locked
 
|
Looks like there is a regression with raised and inset borders. The method #initialize in SimpleBorder sets baseColor to
"Color transparent", which prevents #trackColorFrom: in its subclasses to work correctly. Since it is unclear to me whether it would have unintended side effects if the initialization of baseColor was removed, I've added a check for transparent baseColor in all three trackColorFrom: methods, but that looks a bit ugly. Levente, the method is from April 2016 and carries your initials - do you remember what the motivation for this change was? Cheers, Hans-Martin |
|
|
Hi Hans-Martin,
My intention was to always initialize baseColor, because nil was used as a substitute of Color transparent back and forth for seemingly no reason. I must have overlooked those #trackColorFrom: methods, and I think replacing ifNil: with isTransparent ifTrue: in them is the right fix. If you have a better solution in mind, let me know. Or I'll push the above fix this week to the Trunk. Levente On Tue, 1 Nov 2016, Hans-Martin Mosner wrote: > Looks like there is a regression with raised and inset borders. The method #initialize in SimpleBorder sets baseColor to > "Color transparent", which prevents #trackColorFrom: in its subclasses to work correctly. > > Since it is unclear to me whether it would have unintended side effects if the initialization of baseColor was removed, > I've added a check for transparent baseColor in all three trackColorFrom: methods, but that looks a bit ugly. > > Levente, the method is from April 2016 and carries your initials - do you remember what the motivation for this change was? > > > Cheers, > > Hans-Martin > > > |
|
|
Didn’t my fix for this get into the update stream? A few weeks back I hit this ‘bug’ and suggested simply not initialising baseColor at all.
With it left nil, any use of trackColorFrom: is able to set the inset colour ok. > On 03-11-2016, at 3:22 PM, Levente Uzonyi <[hidden email]> wrote: > > Hi Hans-Martin, > > My intention was to always initialize baseColor, because nil was used as a substitute of Color transparent back and forth for seemingly no reason. > I must have overlooked those #trackColorFrom: methods, and I think replacing ifNil: with isTransparent ifTrue: in them is the right fix. > If you have a better solution in mind, let me know. Or I'll push the above fix this week to the Trunk. > > Levente > > On Tue, 1 Nov 2016, Hans-Martin Mosner wrote: > >> Looks like there is a regression with raised and inset borders. The method #initialize in SimpleBorder sets baseColor to >> "Color transparent", which prevents #trackColorFrom: in its subclasses to work correctly. >> >> Since it is unclear to me whether it would have unintended side effects if the initialization of baseColor was removed, >> I've added a check for transparent baseColor in all three trackColorFrom: methods, but that looks a bit ugly. >> >> Levente, the method is from April 2016 and carries your initials - do you remember what the motivation for this change was? >> >> >> Cheers, >> >> Hans-Martin >> >> >> > tim -- tim Rowledge; [hidden email]; http://www.rowledge.org/tim Fractured Idiom:- LE ROI EST MORT. JIVE LE ROI - The King is dead. No kidding. |
|
|
I don't see your changes in my fully updated Trunk image, but I don't see
either why is it good to mix instances of Color with nil, where nil is just a replacement of a single color: Color transparent. Just have a look at how #baseColor: was implemented: | cc | cc := aColor isTransparent ifTrue:[nil] ifFalse:[aColor]. baseColor = cc ifTrue:[^self]. baseColor := cc. ... And what #baseColor was before: ^baseColor ifNil:[Color transparent] There's no reason not to initialize baseColor if it is intended to be Color transparent by default. Levente On Thu, 3 Nov 2016, tim Rowledge wrote: > Didn’t my fix for this get into the update stream? A few weeks back I hit this ‘bug’ and suggested simply not initialising baseColor at all. > With it left nil, any use of trackColorFrom: is able to set the inset colour ok. > >> On 03-11-2016, at 3:22 PM, Levente Uzonyi <[hidden email]> wrote: >> >> Hi Hans-Martin, >> >> My intention was to always initialize baseColor, because nil was used as a substitute of Color transparent back and forth for seemingly no reason. >> I must have overlooked those #trackColorFrom: methods, and I think replacing ifNil: with isTransparent ifTrue: in them is the right fix. >> If you have a better solution in mind, let me know. Or I'll push the above fix this week to the Trunk. >> >> Levente >> >> On Tue, 1 Nov 2016, Hans-Martin Mosner wrote: >> >>> Looks like there is a regression with raised and inset borders. The method #initialize in SimpleBorder sets baseColor to >>> "Color transparent", which prevents #trackColorFrom: in its subclasses to work correctly. >>> >>> Since it is unclear to me whether it would have unintended side effects if the initialization of baseColor was removed, >>> I've added a check for transparent baseColor in all three trackColorFrom: methods, but that looks a bit ugly. >>> >>> Levente, the method is from April 2016 and carries your initials - do you remember what the motivation for this change was? >>> >>> >>> Cheers, >>> >>> Hans-Martin >>> >>> >>> >> > > > tim > -- > tim Rowledge; [hidden email]; http://www.rowledge.org/tim > Fractured Idiom:- LE ROI EST MORT. JIVE LE ROI - The King is dead. No kidding. > > > > |
|
|
> On 03-11-2016, at 5:06 PM, Levente Uzonyi <[hidden email]> wrote: > > I don't see your changes in my fully updated Trunk image, Hmm, did I remember to actually stick them in trunk… > [snip] > > There's no reason not to initialize baseColor if it is intended to be Color transparent by default. A good point, though I’m not sure initialising a colour to transparent for a supposedly contrasting inset border is the best idea. It makes it just that bit harder to see what is going on when you are exploring the code and trying to set a border on a morph to see what happens! I don’t know in detail what the intent was for the border colours. It seems to me there are two different cases a) the border is supposed to be that darker/lighter of the owning morph’s main colour b) the border is explicitly set to some other colour which then doesn’t change is the owner colour does. If we accept those two then there has to be a simple way to signal to the code which is in use. nil is a fairly normal way to do that sort of thing but I don’t mind how it gets done. tim -- tim Rowledge; [hidden email]; http://www.rowledge.org/tim Earth is 98% full...please delete anyone you can. |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |