Status Update
No update yet.
Comments
il...@google.com
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
) {
...
}
}
ap...@google.com
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.
jb...@google.com
an...@google.com
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.
il...@google.com
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!
Description
Version used: 1.2.0-beta02
Devices/Android versions reproduced on: All tested
I have attached a sample project and a screen recording from the project which illustrates the issue.
The project consists of a start view with two items (one of which is rotated 180 degrees). Clicking on any item does a shared element transition into a detail view, which has two images at the bottom. When you came in via the item that was unrotated and press back, the images explode out away from the center of the shared element as desired (the first transition seen in the video). If you came in via the rotated item however, the epicenter position is calculated incorrectly and the shared elements explode away from somewhere to the lower right of the screen (second transition seen in the video).
I believe the error is caused by the use of `View#getLocationOnScreen` when calculating the epicenter for a fragment transition, in FragmentTransitionImpl:
/**
* Replacement for view.getBoundsOnScreen because that is not public. This returns a rect
* containing the bounds relative to the screen that the view is in.
*/
protected void getBoundsOnScreen(View view, Rect epicenter) {
int[] loc = new int[2];
view.getLocationOnScreen(loc);
epicenter.set(loc[0], loc[1], loc[0] + view.getWidth(), loc[1] + view.getHeight());
}
I've ran into this problem myself when calculating screen bounds - the location given by getLocationOnScreen is in fact (0,0) in the View's coordinate system transformed to that of the screen. This means if the view is rotated for example 180 degrees, this will be the lower right corner of the view as seen on screen. When you then add the width and the height, you end up to the lower right of where the view actually is on the screen - which is exactly what we see in the sample project.
The proper way to do it would be to port the code from the View implementation (which takes the matrix into account, meaning scale, rotation, pivot etc would all work).