Add extension overloads for LazyListScope#34
Conversation
|
Need to update README section talking about 3 kinds of APIs to 4 (https://github.com/zhanghai/ComposePreference#preferences) as well. |
Goooler
force-pushed
the
reload-extensions-for-list-scope
branch
from
633038b to
dce7eca
Compare
|
Done. |
|
Any updates? |
|
Please resolve the pending comments. |
|
I think it has already been addressed: https://github.com/zhanghai/ComposePreference/compare/633038b6cdfffe4e1f035bb06d56a7cb43ec8933..dce7eca772a241fc1583878fb65a8db623ecf1ee. |
|
Sorry, there is nothing about your review comment. |
| } | ||
| } | ||
|
|
||
| @Suppress("NOTHING_TO_INLINE") |
There was a problem hiding this comment.
Why inline if nothing to inline? This is app code so seems to me we should rely on compiler and JIT optimizations.
There was a problem hiding this comment.
(Didn't have time to think about this, but alternatively what about crossinline?)
There was a problem hiding this comment.
I'm afraid we can't. If we mark crossinline for:
onValueChangeortitle, the compiler errors for:
Illegal usage of inline parameter 'crossinline onValueChange: (Boolean) -> Unit'. Add 'noinline' modifier to the parameter declaration.
iconorsummary, the compiler errors like:
Inline parameter 'crossinline icon: ComposableFunction0<Unit>? = ...' of 'fun LazyListScope.checkboxPreference(key: String, value: Boolean, noinline onValueChange: (Boolean) -> Unit, noinline title: ComposableFunction0<Unit>, modifier: Modifier = ..., enabled: Boolean = ..., crossinline icon: ComposableFunction0<Unit>? = ..., crossinline summary: ComposableFunction0<Unit>? = ...): Unit' cannot be nullable. Add 'noinline' modifier to the parameter declaration or make its type not nullable.
There was a problem hiding this comment.
Ack on icon and summary, they are indeed noinline in the existing overload due to being nullable. But I'm not following why title can't be crossinline here, since it is crossinline in the existing overload. Could you please help explain that?
There was a problem hiding this comment.
Nevermind, that's because the other overload is using title in its own body, but here title is passed on as a parameter. The same goes for onValueChange.
That said, if there's no lambda that can be inlined, why don't we just follow the lint and don't use inline function?
There was a problem hiding this comment.
We can. I inlined them here just to align the styles from the existing ones.
| 1. A `LazyListScope.*Preference` extension function, which is the easiest way to use preferences in this library, and helps developers to avoid boilerplates like having to specify the key twice for the `LazyColumn` and the `Preference`. | ||
| 2. A `*Preference` composable that takes a `MutableState`, which allows developers to bring in any kind of state they currently have. | ||
| 3. A `*Preference` composable that takes `value` and `onValueChange`, which allows developers to use the preference without a state and even in non-preference scenarios. | ||
| 2. A `LazyListScope.*Preference` extension function, reloads the first one for `value` and `onValueChange` params. |
There was a problem hiding this comment.
I assume you meant overloads instead of reloads? Something like , which is an overload of the first extension function but accepts value and onValueChange.
|
Oh, sorry I didn't realize |
It's handy for reading and updating values from ViewModels.
Goooler
force-pushed
the
reload-extensions-for-list-scope
branch
from
dce7eca to
de046cd
Compare
| } | ||
| } | ||
|
|
||
| @Suppress("NOTHING_TO_INLINE") |
There was a problem hiding this comment.
Ack on icon and summary, they are indeed noinline in the existing overload due to being nullable. But I'm not following why title can't be crossinline here, since it is crossinline in the existing overload. Could you please help explain that?
Co-authored-by: Hai Zhang <4469895+zhanghai@users.noreply.github.com>
2 participants
It's handy for reading and updating values from ViewModels.