google / accompanist

A collection of extension libraries for Jetpack Compose
https://google.github.io/accompanist
Apache License 2.0
7.43k stars 598 forks source link

[Placeholder] Placeholders overlapping #1571

Closed massipetro closed 1 year ago

massipetro commented 1 year ago

Describe the bug

I've noticed that, after updating accompanist placeholder from version 0.23.1 to version 0.30.0, sometimes two placeholders would overlap.

For example: given two Text components in a Row, both with Modifier.placeholder(state.isLoading) and a state based text value (respectively state.text1 and state.text2); when state changes twice from isLoading = false to isLoading = true and with a text1 value shorter than the previous one, then the two placeholders overlap.

I've noticed this behaviour happening after the commit 9c4a319. This problem is present from the version 0.24.10-beta.

To Reproduce

Run the following code and tap on the button 3 times.

import androidx.compose.foundation.background
import androidx.compose.foundation.layout.*
import androidx.compose.material.Button
import androidx.compose.material.Text
import androidx.compose.runtime.*
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import com.google.accompanist.placeholder.material.placeholder

data class State(
    val isLoading: Boolean,
    val text1: String,
    val text2: String
)

val stateList = listOf(
    State(
        isLoading = false,
        text1 = "Hello",
        text2 = "world"
    ),
    State(
        isLoading = true,
        text1 = "#",
        text2 = "#"
    )
)

@Preview(showSystemUi = true)
@Composable
fun PlaceholderSnippet() {
    var index by remember { mutableStateOf(0) }
    val state = stateList[index]

    Column(
        horizontalAlignment = Alignment.CenterHorizontally,
        verticalArrangement = Arrangement.Center,
        modifier = Modifier
            .fillMaxSize()
            .background(color = Color.Cyan)
    ) {
        Text(text = "State index: $index")
        Spacer(Modifier.width(4.dp))
        Row {
            Text(text = state.text1, modifier = Modifier.placeholder(state.isLoading))
            Spacer(Modifier.width(4.dp))
            Text(text = state.text2, modifier = Modifier.placeholder(state.isLoading))
        }
        Spacer(Modifier.width(4.dp))
        Button(
            onClick = { index = (index + 1) % stateList.size }
        ) {
            Text(text = "Change state")
        }
    }
}

Expected behavior

I expect the placeholders on the elements not to overlap.

Video

placeholder_bug.webm

Environment:

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

massipetro commented 1 year ago

up

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

massipetro commented 1 year ago

up