Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Scrolling breaks on Compose 1.3.0 in non-first child only #326

Closed
felixdivo opened this issue Feb 4, 2023 · 6 comments
Closed

Bug: Scrolling breaks on Compose 1.3.0 in non-first child only #326

felixdivo opened this issue Feb 4, 2023 · 6 comments
Labels
bug Something isn't working not decompose Something unrelated to Decompose

Comments

@felixdivo
Copy link

Hey 👋

Intro & Affected Versions

I just realized that my bug that appeared when moving from Compose (JetBrains) version 1.2.2 to 1.3.0 might be due to Decompose. It existed for quite a few 1.3.0-rc* versions. I am on Decompose 1.0.0 (it already was a problem in 1.0.0-beta-04, possibly earlier). I have always set org.jetbrains.kotlin:kotlin-gradle-plugin:1.7.20 for consistency.

(A visual example and reproducer code is given at the end.)

Description

The scrollbar stops moving on the second child only (!) when on Compose 1.3.0. In the case of non-lazy scrolling, even the content stops moving. This does not happen on the first child, which is why I think this is related to Decompose more than to JB Compose.

Visuals

Click me

Expected Behavior (Compose 1.2.2)

Scrolling with the mouse wheel works too.
working

Wrong Behavior (Compose 1.3.0) variant FixedScrollable

I also tried scrolling with the mouse wheel.

fail-LazyScrollable

Wrong Behavior (Compose 1.3.0) variant LazyScrollable

I also tried scrolling with the mouse wheel.

fail-LazyScrollable

Reproducer

Click me

You can exchange occurrences of FixedScrollable and LazyScrollable, both produce similar problems.

import androidx.compose.foundation.*
import androidx.compose.foundation.layout.*
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.rememberLazyListState
import androidx.compose.material.Button
import androidx.compose.material.MaterialTheme
import androidx.compose.material.Surface
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.Window
import androidx.compose.ui.window.application
import androidx.compose.ui.window.rememberWindowState
import com.arkivanov.decompose.ComponentContext
import com.arkivanov.decompose.DefaultComponentContext
import com.arkivanov.decompose.extensions.compose.jetbrains.lifecycle.LifecycleController
import com.arkivanov.decompose.extensions.compose.jetbrains.stack.Children
import com.arkivanov.decompose.extensions.compose.jetbrains.stack.animation.fade
import com.arkivanov.decompose.extensions.compose.jetbrains.stack.animation.plus
import com.arkivanov.decompose.extensions.compose.jetbrains.stack.animation.scale
import com.arkivanov.decompose.extensions.compose.jetbrains.stack.animation.stackAnimation
import com.arkivanov.decompose.extensions.compose.jetbrains.subscribeAsState
import com.arkivanov.decompose.router.stack.ChildStack
import com.arkivanov.decompose.router.stack.StackNavigation
import com.arkivanov.decompose.router.stack.childStack
import com.arkivanov.decompose.router.stack.push
import com.arkivanov.decompose.value.MutableValue
import com.arkivanov.decompose.value.Value
import com.arkivanov.essenty.lifecycle.LifecycleRegistry
import com.arkivanov.essenty.parcelable.Parcelable
import com.arkivanov.essenty.parcelable.Parcelize
import javax.swing.SwingUtilities

/*
This examples closely follows https://arkivanov.github.io/Decompose/getting-started/quick-start/
*/


fun main() {
  val lifecycle = LifecycleRegistry()

  // Always create the root component outside Compose on the main thread
  val root =
      runOnUiThread {
          DefaultRootComponent(
              componentContext = DefaultComponentContext(lifecycle = lifecycle),
          )
      }

  application {
      val windowState = rememberWindowState()

      LifecycleController(lifecycle, windowState)

      Window(
          onCloseRequest = ::exitApplication,
          state = windowState,
          title = "My Application"
      ) {
          MaterialTheme {
              Surface {
                  RootContent(component = root, modifier = Modifier.fillMaxSize())
              }
          }
      }
  }
}

interface RootComponent {
  val stack: Value<ChildStack<*, Child>>

  // Defines all possible child components
  sealed class Child {
      class ListChild(val component: ListComponent) : Child()
      class DetailsChild(val component: DetailsComponent) : Child()
  }
}

class DefaultRootComponent(
  componentContext: ComponentContext,
) : RootComponent, ComponentContext by componentContext {

  private val navigation = StackNavigation<Config>()

  private val _stack =
      childStack(
          source = navigation,
          initialConfiguration = Config.List, // The initial child component is List
          handleBackButton = true, // Automatically pop from the stack on back button presses
          childFactory = ::child,
      )

  override val stack: Value<ChildStack<*, RootComponent.Child>> = _stack

  private fun child(config: Config, componentContext: ComponentContext): RootComponent.Child =
      when (config) {
          is Config.List -> RootComponent.Child.ListChild(listComponent(componentContext))
          is Config.Details -> RootComponent.Child.DetailsChild(detailsComponent(componentContext, config))
      }

  private fun listComponent(componentContext: ComponentContext): ListComponent =
      DefaultListComponent(
          componentContext = componentContext,
          onItemSelected = { item: String -> // Supply dependencies and callbacks
              navigation.push(Config.Details(item = item)) // Push the details component
          },
      )

  private fun detailsComponent(componentContext: ComponentContext, config: Config.Details): DetailsComponent =
      DefaultDetailsComponent(
          componentContext = componentContext,
      )

  @Parcelize // The `kotlin-parcelize` plugin must be applied if you are targeting Android
  private sealed interface Config : Parcelable {
      object List : Config
      data class Details(val item: String) : Config
  }
}


interface ListComponent {
  val model: Value<Model>

  fun onItemClicked(item: String)

  data class Model(
      val items: List<String>,
  )
}

class DefaultListComponent(
  componentContext: ComponentContext,
  private val onItemSelected: (item: String) -> Unit,
) : ListComponent {
  override val model: Value<ListComponent.Model> =
      MutableValue(ListComponent.Model(items = List(3) { "Item $it" }))

  override fun onItemClicked(item: String) {
      onItemSelected(item)
  }
}

interface DetailsComponent {
  val model: Value<Model>

  data class Model(
      val items: List<String>,
  )
}

class DefaultDetailsComponent(
  componentContext: ComponentContext,
) : DetailsComponent {
  override val model: Value<DetailsComponent.Model> =
      MutableValue(DetailsComponent.Model(items = List(100) { "Detail Item $it" }))
}


@Composable
fun ListContent(component: ListComponent, modifier: Modifier = Modifier) {
  val model by component.model.subscribeAsState()

  Column {
      Button(onClick = { component.onItemClicked("Heyo") }) {
          Text(text = "Item 1")
      }

      // Here, it is still working
      FixedScrollable()
      // LazyScrollable()
  }
}

@Composable
fun DetailsContent(component: DetailsComponent, modifier: Modifier = Modifier) {
  // Here, the scrollbar breaks
  FixedScrollable()
  // LazyScrollable()
}


@Composable
fun RootContent(component: RootComponent, modifier: Modifier = Modifier) {
  Children(
      stack = component.stack,
      modifier = modifier,
      animation = stackAnimation(fade() + scale()),
  ) {
      when (val child = it.instance) {
          is RootComponent.Child.ListChild -> ListContent(component = child.component)
          is RootComponent.Child.DetailsChild -> DetailsContent(component = child.component)
      }
  }
}


private fun <T> runOnUiThread(block: () -> T): T {
  if (SwingUtilities.isEventDispatchThread()) {
      return block()
  }

  var error: Throwable? = null
  var result: T? = null

  SwingUtilities.invokeAndWait {
      try {
          result = block()
      } catch (e: Throwable) {
          error = e
      }
  }

  error?.also { throw it }

  @Suppress("UNCHECKED_CAST")
  return result as T
}

@Composable
fun LazyScrollable() {
  Box(
      modifier = Modifier
          .fillMaxSize()
          .background(color = Color.White)
  ) {
      val state = rememberLazyListState()

      LazyColumn(
          modifier = Modifier.fillMaxSize(),
          state = state,
      ) {
          items(10) { x ->
              TextBox("Item #$x", x)
          }
      }
      VerticalScrollbar(
          modifier = Modifier.align(Alignment.CenterEnd).fillMaxHeight(),
          adapter = rememberScrollbarAdapter(
              scrollState = state,
          )
      )
  }
}

@Composable
fun FixedScrollable() {
  Box(
      modifier = Modifier
          .fillMaxSize()
          .background(color = Color.White)
  ) {
      val state = rememberScrollState(0)
      Column(modifier = Modifier.fillMaxSize().verticalScroll(state)) {
          (0..10).forEach { x ->
              TextBox("Item #$x", x)
          }
      }
      VerticalScrollbar(
          modifier = Modifier.align(Alignment.CenterEnd).fillMaxHeight(),
          adapter = rememberScrollbarAdapter(
              scrollState = state,
          )
      )
  }
}


@Composable
fun TextBox(text: String = "Item", index: Int) {
  Box(
      modifier = Modifier
          .height(130.dp)
          .fillMaxWidth()
          .background(
              color = Color(
                  red = (index * 50).mod(255),
                  green = (index * 100).mod(255),
                  blue = (index * 150).mod(255),
                  alpha = 128
              )
          ),
      contentAlignment = Alignment.CenterStart
  ) {
      Text(text = text)
  }
}
@arkivanov
Copy link
Owner

Hey! Thanks for such a detail explanation and the reproducer! This appears to be a bug in Compose. Specifically, the issue happens when movableContentOf is used. Children function uses movableContentOf for switching.

Here is a simple reproducer.

@Composable
fun Foo() {
    val child1: @Composable (Modifier) -> Unit = remember { movableContentOf { modifier -> FixedScrollable(modifier) } }
    val child2: @Composable (Modifier) -> Unit = remember { movableContentOf { modifier -> FixedScrollable(modifier) } }
    var isHorizontal by remember { mutableStateOf(true) }

    Column(modifier = Modifier.fillMaxSize()) {
        val modifier = Modifier.fillMaxWidth().weight(1F)
        if (isHorizontal) {
            Row(modifier = modifier) {
                child1(Modifier.weight(1F))
                child2(Modifier.weight(1F))
            }
        } else {
            Column(modifier = modifier) {
                child1(Modifier.weight(1F))
                child2(Modifier.weight(1F))
            }
        }

        Button(onClick = { isHorizontal = !isHorizontal }) {
            Text("Switch")
        }
    }
}

I have filed JetBrains/compose-multiplatform#2696. I will also try to find a workaround.

@arkivanov arkivanov added the bug Something isn't working label Feb 4, 2023
@arkivanov
Copy link
Owner

arkivanov commented Feb 4, 2023

It doesn't look possible to fix it on Decompose side, as there seems no way to avoid movableContentOf without breaking the behaviour. The only workaround I could find is to downgrade Compose to either 1.2.2 or 1.3.0-alpha01-dev831. The issue is reproducible on desktop since 1.3.0-alpha01-dev849, and is not reproducible on Android.

@felixdivo
Copy link
Author

Thank you @arkivanov for your swift reaction! I'll stay on 1.2.2 for the time being and will have a close eye on the other issue. 🙂 I think I cannot help you in this issue since I'm not that deep into Compose. 🙃

@arkivanov
Copy link
Owner

Another possible workaround is to not use stack animations.

@arkivanov
Copy link
Owner

I believe there is even better workaround:

  1. Update Kotlin to 1.8.0
  2. Use Compose version 1.2.2 or 1.3.0-alpha01-dev831
  3. Either downgrade Compose plugin to a version compatible with Kotlin 1.8.0, or ignore the compatibility check (see docs)

@arkivanov arkivanov added the not decompose Something unrelated to Decompose label Feb 24, 2023
@arkivanov
Copy link
Owner

Looks like the issue is fixed in JB Compose version 1.3.1-rc01. Closing this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working not decompose Something unrelated to Decompose
Projects
None yet
Development

No branches or pull requests

2 participants