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

Revert Hoverable to AOSP state #1773

Merged
merged 1 commit into from
Jan 13, 2025
Merged

Conversation

igordmn
Copy link
Collaborator

@igordmn igordmn commented Jan 13, 2025

Reverted to https://github.com/androidx/androidx/blob/6c04a79c5c3a5fdb6d5221eb344fe1ed98227957/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Hoverable.kt#L89

The changes were added to avoid input lag during scrolling on hoverable items: #188

But after that we introduced another change - flush all effects before each frame (#1260):

BaseComposeScene {
  override fun render(canvas: Canvas, nanoTime: Long) {
    recomposer.performScheduledEffects()
    ...
  }
}

This makes the first fix not needed.

Testing

import androidx.compose.foundation.background
import androidx.compose.foundation.hoverable
import androidx.compose.foundation.interaction.HoverInteraction
import androidx.compose.foundation.interaction.MutableInteractionSource
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.items
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.singleWindowApplication

fun main() {
    singleWindowApplication {
        Test()
    }
}

@Composable
private fun Test() {
    LazyColumn {
        items((0..200).toList()) {
            val interactionSource = remember { MutableInteractionSource() }
            var hovered by remember { mutableStateOf(false) }
            LaunchedEffect(interactionSource) {
                interactionSource.interactions.collect { interaction ->
                    when (interaction) {
                        is HoverInteraction.Enter -> hovered = true
                        is HoverInteraction.Exit -> hovered = false
                    }
                }
            }

            Text(
                it.toString(),
                Modifier
                    .background(if (hovered) Color.Blue else Color.White)
                    .hoverable(interactionSource)
                    .height(20.dp)
                    .fillMaxWidth()
            )
        }
    }
}

Scroll without moving - see that latency of hovers didn't change compared to the jb-main state

Release Notes

N/A

The changes were added to avoid input lag during scrolling on hoverable items:
#188

But after that we introduced another change - flush all effects before each frame (#1260):
```
BaseComposeScene {
  override fun render(canvas: Canvas, nanoTime: Long) {
    recomposer.performScheduledEffects()
    ...
  }
}
```
This makes the first fix not needed.

## Testing
```
import androidx.compose.foundation.background
import androidx.compose.foundation.hoverable
import androidx.compose.foundation.interaction.HoverInteraction
import androidx.compose.foundation.interaction.MutableInteractionSource
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.items
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.singleWindowApplication

fun main() {
    singleWindowApplication {
        Test()
    }
}

@composable
private fun Test() {
    LazyColumn {
        items((0..200).toList()) {
            val interactionSource = remember { MutableInteractionSource() }
            var hovered by remember { mutableStateOf(false) }
            LaunchedEffect(interactionSource) {
                interactionSource.interactions.collect { interaction ->
                    when (interaction) {
                        is HoverInteraction.Enter -> hovered = true
                        is HoverInteraction.Exit -> hovered = false
                    }
                }
            }

            Text(
                it.toString(),
                Modifier
                    .background(if (hovered) Color.Blue else Color.White)
                    .hoverable(interactionSource)
                    .height(20.dp)
                    .fillMaxWidth()
            )
        }
    }
}
```

Scroll without moving - see that latency of hovers didn't change compared to the jb-main state
@igordmn igordmn requested a review from m-sasha January 13, 2025 12:51
@igordmn igordmn merged commit c07d660 into jb-main Jan 13, 2025
7 checks passed
@igordmn igordmn deleted the igor.demin/restore-hoverable branch January 13, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants