[Cuis-dev] Theme>>useUniformColors considered harmful

[Juan Vuletich]

Sounds great to me.

Thanks,

-- 
Juan Vuletich
www.cuis-smalltalk.org
https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
https://github.com/jvuletich
https://www.linkedin.com/in/juan-vuletich-75611b3
@JuanVuletich


On 5/25/2020 8:26 AM, Casey Ransberger via Cuis-dev wrote:
> Yeah. Themes and the taskbar should only intersect where the taskbar 
> decides how it should look. Whether or not it’s shown should belong to 
> the taskbar itself (probably class-side, since there’s only one 
> taskbar,) and be exposed in Preferences.
>
> I’m ambivalent about whether or not an instance of the taskbar exists 
> while it isn’t being shown. It shouldn’t be eating up a bunch of 
> processing if we can’t see it, and it shouldn’t take up a lot of space 
> in the image in any event. May as well leave it allocated if it isn’t 
> being a bother, but better to measure things and figure out what’s 
> best if that’s a concern people have.
>
> This was what I was looking into when I encountered the pattern of 
> duplicated code in senders of #useUniformColors. I’d rather get rid of 
> all of these divergent paths before I even worry about where the “show 
> taskbar” bit lives.
>
> —Casey
>
> P.S.
>
> Also, the theme system isn’t done until existing windows and widgets 
> update themselves when you change themes. Much to do, there is, to say 
> it like Yoda would.
>
>> On May 25, 2020, at 3:47 AM, Luciano Notarfrancesco 
>> <luchiano at gmail.com <mailto:luchiano at gmail.com>> wrote:
>>
>> Sounds very good to me. And while you’re at it, perhaps we can remove 
>> useTaskbar as you suggested before, as long as we avoid randomly 
>> showing the taskbar again. Juan already removed the call to 
>> restoreDefaultDesktop, and we should also avoid adding the taskbar 
>> again in Theme class>>currentTheme:
>>
>> On Mon, 25 May 2020 at 5:33 PM, Casey Ransberger via Cuis-dev 
>> <cuis-dev at lists.cuis.st <mailto:cuis-dev at lists.cuis.st>> wrote:
>>
>>     I’m poking around in the vicinity of Theme, thinking about adding
>>     a class to Theme-Core. When I initially wrote Theme, I did a lot
>>     of gymnastics to keep it down to a single class, wanting to avoid
>>     unnecessarily adding a bunch of classes to the system. Also
>>     wanted to impress Juan, who I expected would be hesitant to add a
>>     bunch of complicated machinery to Cuis.
>>
>>     Looking back on the code something like a decade later, I think I
>>     was a little gung-ho with the one-class thing. Adding a single
>>     class UniformColorTheme (or maybe we can think of a better name
>>     like BoringColorTheme) seems to obviate #useUniformColors
>>     completely, and thus eliminate 13 send sites (which, I might add,
>>     all have a code duplication smell.) Each of these send sites
>>     involves branch logic that would go away, 3 or 4 lines of code
>>     each. I haven’t investigated this last one yet, but
>>     #defaultWindowColor might also go away, yielding a bit more in
>>     the way of complexity savings.
>>
>>     Here’s a typical send site of Theme>>useUniformColors.
>>
>>     Theme>>versionsBrowser
>>     ^ self useUniformColors
>>     ifTrue: [ self defaultWindowColor ]
>>     ifFalse: [ `(Color r: 0.869 g: 0.753 b: 1.0) duller` ]
>>
>>     I’m not sure if I wrote the above code or not.
>>
>>     It’s always ^self useUniformColors ifTrue:[] ifFalse[]. All 13 sites.
>>
>>     Between one and two methods would go away along with an
>>     additional 39+ lines of code. In exchange, we get a new class (my
>>     vote is for BoringTheme) with as many as 13 one-line methods. A
>>     cross-cutting concern goes away. We lose 5 LoC to a class
>>     definition, if we want to even count that, still a 21+ line win.
>>     I haven’t looked at the extra themes, but I’ll update them if I
>>     make the change, and I expect code savings there as well. More
>>     importantly, it will be more logical, more object oriented, less
>>     tricky. Right now to implement a theme, you need to know about
>>     this odd convention in these 13 methods, and it shouldn’t be that
>>     way. I went with methods instead of sticking instances of Color
>>     in ivars so we could experiment with functional expressions that
>>     yield arbitrary UI objects (originally there was going to be a
>>     lot more to this than colors, but life has a way of interrupting
>>     grand schemes.) Here though we’re throwing in logic to pick
>>     between two colors, which smells like the opposite idea.
>>
>>     Okay that’s the gist of it. Only performance impact should be
>>     negligible but positive (sends and branch logic that don’t need
>>     to happen anymore.) Theme-Core isn’t at all a crowded category so
>>     scrolling and cognitive load shouldn’t be an issue with adding a
>>     class here. I can probably absorb what I think used to be called
>>     GrayTheme out of the extra themes package into OfficeDull
>>     Win95KnockOff or whatever we decide to call it, so any methods
>>     there go away from the extra themes package.
>>
>>     Is everyone okay with me doing this?
>>
>>     —Casey
>>     -- 
>>     Cuis-dev mailing list
>>     Cuis-dev at lists.cuis.st <mailto:Cuis-dev at lists.cuis.st>
>>     https://lists.cuis.st/mailman/listinfo/cuis-dev
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20200526/ad763184/attachment-0001.htm>