https://kotlinlang.org logo
Join Slack
Powered by
*Feedback*: `color = (+MaterialTheme.colors()).pri...
# compose
l
Feedback: 
color = (+MaterialTheme.colors()).primary
is way uglier than what it used to be: 
color = +themeColor { primary }
βž• 2
Please revert? πŸ™πŸΌ
m
Altough it will be better once the unaryPlus operator goes away i have to agree with the fact that the themeColor function is way nicer.
l
both are using the 
unaryPlus
so I was not taking that in consideration
MaterialTheme.colors().primary is meh
imho
s
What will 
unaryPlus
be replaced with? Is it known?
l
No replacement, it will just go away
If anyone wants the updated version working with current code πŸ˜›
Untitled
πŸ‘ 1
k
MaterialTheme object won't be reachable from a library perspective, πŸ€” wonder why the choice to get rid of themeColor
t
Well, why not to create an extension for that, @Luca Nicoletti? I strictly against that ui core must have all kinds of bells and whistles considered "helpful" now
Nwm, you did that in the thread comments ;)
🧌 1
l
@themishkun I was not asking to have both ways, I just provided a feedback saying the β€œold” way looked nicer to me πŸ™‚
s
The old way looks much cleaner to me as well
a
The old way got way into that too clever by half territory. It also tested pretty badly in user studies.
l
and how did the new one test? 🧌
a
It's the form participants from the first tests expected πŸ™‚ the selector block thing turns autocomplete into jumping through hoops and we only did it in the first place as a bandaid for the unary+
I'm really curious what aspects of the first one you liked
l
It reads clearer, it doesn’t look it’s accessing a 
Singleton
, the selector block looks way more 
kotlin-ish
Plus: 
themeColor
lets me think I’m accessing the current theme, whether 
MaterialTheme.colors()
doesn’t feel I’m using the 
theme
I might have setup in outer 
@Composable
functions
a
If it's the length, we've tossed around shortening it to just 
Material.colors.primary
with some debate around effect property getters being a good or bad idea, with I think some degree of team consensus that it encourages less local val aliasing and is probably a good thing. (Since you want to read the underlying ambient in as tight of a scope as possible to avoid unnecessary recompositions)
k
unary+
-> it was helpful for me to quickly scan the code and find the system hooks which gets a provided element. 
theme
-> function is a great generic solution doesn't matter if it is material or Company specific design. 
lambda
-> as @Luca Nicoletti mentioned super flexible for merging and anything really to apply on the style before usage. it is Kotlin like.
l
It’s not about the length at all
I’d rather have longer code but more readable/understandable
βž• 1
a
Dropping the word "theme" entirely is more about breaking people from expecting the mess that is Android's existing theme system
l
Happy to participate in any future user-testing 🧌
πŸ‘Œ 1
a
Noted! πŸ™‚
❀️ 2
l
a
Separate debates there πŸ™‚
l
What about the idea of not accessing the 
currentTheme
but the default 
Material
with the new solution?
Neither 
Material.colors.primary
nor 
(+MaterialTheme.colors()).primary
suggest I’m accessing the current theme
a
It's a potential point of confusion for sure, we're going to monitor.
As for the lambda, you're only a 
.let {}
away if you need it and it doesn't make sense to make everyone else pay the cognitive overhead just so that it can be cute
(or 
.run
)
l
a
We can run some things remotely and some of our UX researchers travel
βœ”οΈ 1
l
Don’t ask, just book
πŸ˜„ 1
Kidding, ofc
a
I'm not πŸ™‚ I don't think we have more studies scheduled before the end of the year though
l
πŸ‘πŸΌ
a
Would have to check
Anyway this is likely to shift a bit more but the lambda doesn't pull its weight and isn't likely to return
The question of what convention could consistently suggest ambient reads without using the word, "theme" is still open
l
style 🧌
🧌 2
s
Lol so the word theme is hated, lol
k
I would not enforce the lambda if there is no reason πŸ™‚ the current singleton pattern tho super java πŸ˜„
βž• 1
a
@Sebi Sheldin Sebastian kind of. We've found it sets an expectation of defaultButtonStyle and such which is a hard nope
@kioba do you feel the same about 
Dispatchers.Main
?
k
you mean 
AndroidSchedulers.mainThread()
πŸ˜„
btw yeah, my first question would be how to inject this dependency, and that quickly ends up in questions around 
dagger
s
@Adam Powell I confess it did give me those expectations, I was thinking of the Material () widget in flutter which we use to wrap buttons and similar widgets which need to rely on a parent material theme
l
I mean, defaulting to 
Material
is even worse in my opinion, what if I have my design system?
a
@kioba yeah dagger is on our minds independent of this as well
πŸ’― 1
s
Override..lol
a
@Luca Nicoletti that's exactly why I want material in the name here. If you have your own design system, get your own ambients. πŸ™‚ These are what the material widgets appeal to and the general vocabulary they use for cascading customization
l
That has a point πŸ‘πŸΌ
a
It's the reason why Android themes and styles are such a mess today, they try to be universal across design systems and so you get this union of the past 10 years of Android design where some stuff you can squint at and decide to reuse and other stuff is just esoteric namespace dodging
I was part of it getting this bad and do not intend to repeat it 😁
πŸ˜‚ 1
❀️ 1
Yep!
Animate them, etc.
😍 5
k
so the goal is that the UI libraries will provide their own ambients and the app developers has to include it in their tree?
l
a
@kioba design system libraries, yes.
πŸ‘ 1
Oh and also set a strong precedent of those having defaults so if you don't have a provider in your tree you get something reasonable
t
@kioba @Adam Powell Dagger does not solve the singleton-ish problem. You still need to call it like a service locator (I am looking at you, 
DaggerComponent.inject(this)
) to skip the burden of passing theme through parent arguments. Current solution seems okay, because it is still β€œscoped” to the tree.
a
Sorry, didn't mean to imply that, only that dagger is definitely on our minds since compose doesn't depend on object construction
which singleton-ish problem are we still talking about? I'm lost in the long thread above πŸ™‚ the syntax of 
MaterialTheme.colors
certainly has the appearance of a singleton, but it's an effect call that reads the tree-local ambient same as the dev02 apis
l
I think we were talking about that, but just as a syntax looking
I mentioned it looks like a Singleton, didn’t say it is
t
Maybe I misunderstood you, communication is quite hard πŸ˜‰
a
it's a very long thread in a very narrow UI πŸ™‚
l
Let’s 
also send to # compose
for every message to help readability 🧌
rage4 3
a
I'm really not a fan of the aggressive threading customs in the kotlinlang slack here, imo slack works much better if you just let it free-flow like irc Β―\_(ツ)_/Β―
s
Readability definitely should be prime concern with the little I understand about Compose it's wow factor is so high I started praying this becomes the standard way of developing in Android
a
me too. I have intentions that kotlin+compose would be a good way to teach first-time programmers how to code
println("Hello, world!")
but for guis πŸ™‚
anyway the 
MaterialTheme
object anchor is kind of an experimental discoverability and grouping hook for now
l
Well that didn’t start its career so good πŸ˜›
a
I'm not sure that 
val currentMaterialColors
or similar at the top level would be better
l
So, what’s the idea behind 
Material
? Isn’t it a 
theme
?
s
We need something like the Context API in React
a
we have it, it's called 
Ambient
πŸ™‚
πŸ‘ 1
l
ambientColors
?
a
(probably going to be renamed to 
TreeLocal
because 
Ambient
sounds too cool and mysterious)
it needs to evoke that same general feeling of guilty unease that 
ThreadLocal
does to discourage casual overuse πŸ˜‰
πŸ˜‚ 2
@Luca Nicoletti I kind of like "current" as the descriptor just because it's a bit more plain language that speaks to usage more than precise mechanism
l
currentAmbientColors
s
I should stop comparing Compose to flutter, I'm thinking Provider and Inherited Widgets, probably getting too ahead of myself
l
currentTreeColors
a
but exact name aside, there's a small handful of these and finding them is a bit of a question
which is where 
MaterialTheme
as the anchor came from
there's a reasonable argument that applied consistently, a prefix of, 
current
could become a habit that then autocompletes successfully
but it starts getting kind of long in some cases, especially if you start namespacing for design system for sitting them next to others in the same import space
l
Let’s leave namings to 
copy
and not to us developers πŸ˜‚
k
btw @Adam Powell, this was a super awesome thread! We could understand a lot about Compose πŸ˜„ Thanks for taking the time πŸ‘
πŸ‘ 1
❀️ 2
βž• 3
8 Views
Open in Slack
PreviousNext
Powered by