android / codelab-android-compose

Apache License 2.0
1.42k stars 646 forks source link

[AdvancedStateAndSideEffectsCodelab] injected Modifier in CraneTabBar not used #204

Open pitoszud opened 2 years ago

pitoszud commented 2 years ago

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)
        )
    }
}
TheRealDarkEagle commented 2 years ago

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.

pitoszud commented 2 years ago

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

TheRealDarkEagle commented 2 years ago

@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