Status Update
Comments
jo...@google.com <jo...@google.com> #2
object Button {
private val ButtonHorizontalPadding = 16.dp
private val ButtonVerticalPadding = 8.dp
/**
* The default inner padding used by [Button]
*/
val DefaultInnerPadding = InnerPadding(
start = ButtonHorizontalPadding,
top = ButtonVerticalPadding,
end = ButtonHorizontalPadding,
bottom = ButtonVerticalPadding
)
/**
* The default min width applied for the [Button].
* Note that you can override it by applying [Modifier.widthIn] directly on [Button].
*/
val DefaultMinWidth = 64.dp
/**
* The default min width applied for the [Button].
* Note that you can override it by applying [Modifier.heightIn] directly on [Button].
*/
val DefaultMinHeight = 36.dp
/**
* The default disabled background color used by Contained [Button]s
*/
@Composable
val defaultDisabledBackgroundColor
get(): Color = with(MaterialTheme.colors) {
// we have to composite it over surface here as if we provide a transparent background for
// Surface and non-zero elevation the artifacts from casting the shadow will be visible
// below the background.
onSurface.copy(alpha = 0.12f).compositeOver(surface)
}
/**
* The default disabled content color used by all types of [Button]s
*/
@Composable
val defaultDisabledContentColor
get(): Color = with(MaterialTheme.colors) {
EmphasisAmbient.current.disabled.applyEmphasis(onSurface)
}
}
I am not sure the composable getters should be placed in a file with such name.
in the same way this would be a mistake to rename this class to FooConstrants
object Scaffold {
/**
* The possible positions for a [FloatingActionButton] attached to a [Scaffold].
*/
enum class FabPosition {
/**
* Position FAB at the bottom of the screen in the center, above the [BottomAppBar] (if it
* exists)
*/
Center,
/**
* Position FAB at the bottom of the screen at the end, above the [BottomAppBar] (if it
* exists)
*/
End
}
}
and what about this use case, it was about namespacing, not defining constants:
object TabRow {
/**
* Data class that contains information about a tab's position on screen
*
* @property left the left edge's x position from the start of the [TabRow]
* @property right the right edge's x position from the start of the [TabRow]
* @property width the width of this tab
*/
@Immutable
data class TabPosition internal constructor(val left: Dp, val width: Dp) {
val right: Dp get() = left + width
}
/**
* Positions and animates the given [indicator] between tabs when [selectedIndex] changes.
*/
@Composable
fun IndicatorContainer(
tabPositions: List<TabPosition>,
selectedIndex: Int,
indicator: @Composable () -> Unit
) {
...
}
}
ha...@gmail.com <ha...@gmail.com> #3
Yeah, point taken, although that might be a matter of semantics. One alternative way of looking at it is that enums and various helper functions are technically all constants, as they aren't values that change over time. IIRC this is partly why FooConstants
was chosen over FooDefaults
.
I seem to recall that also being mentioned at API Council, and while the name might not be a perfect fit for all circumstances, it is still valuable to be consistent and the new name is better than overloading the same name as the widget which would lead to confusion. If you have an idea for an alternate naming pattern which we haven't considered, I'm sure we'd be happy to re-evaluate.
jo...@google.com <jo...@google.com> #4
ap...@google.com <ap...@google.com> #5
Would IndicatorContainer
be used outside a TabRow
? If not, perhaps it should be thrown on a receiver scope to make it easy to use from within the proper scope and hard to use from outside a TabRow? The API for IndicatorContainer
as it stands today perhaps isn't super discoverable?
I don't have a strong opinion about where the functions live, and there are some judgement calls there, but I don't think that should block the renaming of these objects. Maybe +George or +Adam will have opinions here.
pr...@google.com <pr...@google.com> #6
Interestingly, just looked back at the API Council pod notes, and it makes explicit mention of these caveat:
Scaffold.FabPosition enum conflicts with the above decision about ScaffoldConstants
-> Move FabPosition enum to top-level
Revisit TabRow.Indicator/IndicatorContainer
-> Docs and samples please!
ju...@google.com <ju...@google.com> #7
One alternative way of looking at it is that enums and various helper functions are technically all constants, as they aren't values that change over time. IIRC this is partly why FooConstants was chosen over FooDefaults.
This seems to be the opposite actually, no? The point is that these numbers are strictly tied to the spec, and so can / will change if the spec updates the internal padding for a Button for example. This allows developers to build their own components, point to these 'defaults', and always be matching the spec. For that reason, Defaults
makes more sense to me, as there's no guarantee of stability.
na...@google.com <na...@google.com> #8
Constants "won't change over time" as in "are not mutable vars changed by the program during normal execution", as defined by wikipedia:
Description
Jetpack Compose version: bom:2023.03.00
Jetpack Compose component used: BottomSheetScaffold
Android Studio Build: #AI-222.4459.24.2221.9682058 (Android Studio Flamingo | 2022.2.1 Beta 5)
Kotlin version: 1.8.10
Steps to Reproduce or Code Sample to Reproduce:
Stack trace (if applicable): Screen recordings have been attached that shows the issue. Pay close attention to the
progress
value in screenshot which is the reported progress. The progress reported is not smooth for initial swipe of bottomsheet.After updating compose-bom from 2023.01.00 (which uses 1.3.3 of compose-material) to 2023.03.00 (which uses 1.4.0 of compose-material) I started seeing issues with the bottom sheet progress callback.
My usecase: I need to calculate open-fraction of bottomsheet (i.e. when it's totally collapsed to it's peek height then open fraction is 0.0f and when it's fully expanded, open-fraction should be 1.0f).
Now based on this I need to derive few things like corner size, alpha and offsets of a few other elements. In short, if open-fraction is not reliable then UI will look shaky.
With 1.4.0 of compose-material, Bottomsheet started using
SwipeableV2State
which required quite some change for my usecase to work. Even then, the reported progress is not what I expected. After I start swipping up, the progress remains 0.0f until a certain point (I believe the point isSwipeableV2State.positionalThreshold
but not sure). After that point, reported progress goes directly from0.0
to0.075
and skips progress in between.Same thing happens when I start to collapse sheet from expanded state.