codelab-android-compose icon indicating copy to clipboard operation
codelab-android-compose copied to clipboard

[AdvancedStateAndSideEffectsCodelab] injected Modifier in CraneTabBar not used

Open pitoszud opened this issue 3 years ago • 3 comments

You are injecting modifier - Row(modifier), and then you call Modifier.width(8.dp), Modifier.padding(8.dp) etc... instead of modifier.width(8.dp), modifier.padding(8.dp) etc. Is it a typo, or it was intentional? If so, what's the benefit and why injecting Modifier at all (in this case)?

@Composable
fun CraneTabBar(
    modifier: Modifier = Modifier,
    onMenuClicked: () -> Unit,
    children: @Composable (Modifier) -> Unit
) {
    Row(modifier) {
        // Separate Row as the children shouldn't have the padding
        Row(Modifier.padding(top = 8.dp)) {
            Image(
                modifier = Modifier
                    .padding(top = 8.dp)
                    .clickable(onClick = onMenuClicked),
                painter = painterResource(id = R.drawable.ic_menu),
                contentDescription = stringResource(id = R.string.cd_menu)
            )
            Spacer(Modifier.width(8.dp))
            Image(
                painter = painterResource(id = R.drawable.ic_crane_logo),
                contentDescription = null
            )
        }
        children(
            Modifier
                .weight(1f)
                .align(Alignment.CenterVertically)
        )
    }
}

pitoszud avatar Oct 07 '21 12:10 pitoszud

The modifier is used in the outer Row Row(modifier) { ...

Benefit is that u can modify the Row itself without translating this modification to the element itself. As an example: u could set a fillMaxSize() to the outer Row without modifying the Elements inside it.

TheRealDarkEagle avatar Nov 10 '21 13:11 TheRealDarkEagle

In which part of the outer row it is being used?

pitoszud avatar Nov 11 '21 11:11 pitoszud

@Composable fun CraneTabBar( modifier: Modifier = Modifier, onMenuClicked: () -> Unit, children: @Composable (Modifier) -> Unit ) { Row(modifier) { <- here // Separate Row as the children shouldn't have the padding

TheRealDarkEagle avatar Nov 15 '21 16:11 TheRealDarkEagle