Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ androidxUiAutomator = "2.3.0"
composeStabilityAnalyzer = "0.10.0"
kotlinxSerializationJson = "1.11.0"
jetbrains-compose = "1.11.1"
# Native runtime for desktop Compose UI tests. Must match the skiko version that
# jetbrains-compose pulls transitively (org.jetbrains.skiko:skiko).
skiko = "0.144.6"
glide = "5.0.7"
fresco = "3.7.0"
coil = "2.7.0"
Expand Down Expand Up @@ -90,6 +93,8 @@ kotlinx-serialization-json = { group = "org.jetbrains.kotlinx", name = "kotlinx-
jetbrains-compose-runtime = { group = "org.jetbrains.compose.runtime", name = "runtime", version.ref = "jetbrains-compose" }
compose-runtime-annotation = { group = "androidx.compose.runtime", name = "runtime-annotation", version = "1.11.3" }
jetbrains-compose-ui = { group = "org.jetbrains.compose.ui", name = "ui", version.ref = "jetbrains-compose" }
jetbrains-compose-ui-test = { group = "org.jetbrains.compose.ui", name = "ui-test", version.ref = "jetbrains-compose" }
jetbrains-compose-ui-test-junit4 = { group = "org.jetbrains.compose.ui", name = "ui-test-junit4", version.ref = "jetbrains-compose" }
jetbrains-compose-foundation = { group = "org.jetbrains.compose.foundation", name = "foundation", version.ref = "jetbrains-compose" }
jetbrains-compose-animation = { group = "org.jetbrains.compose.animation", name = "animation", version.ref = "jetbrains-compose" }

Expand Down
30 changes: 29 additions & 1 deletion landscapist-zoomable/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@ import com.github.skydoves.landscapist.Configuration
import org.jetbrains.kotlin.gradle.dsl.JvmTarget
import org.jetbrains.kotlin.gradle.tasks.KotlinJvmCompile

/** Resolves the skiko native runtime classifier (e.g. "macos-arm64") for the running host. */
fun skikoHostTarget(): String {
val os = System.getProperty("os.name").lowercase()
val arch = System.getProperty("os.arch").lowercase()
val osPart = when {
os.contains("mac") || os.contains("darwin") -> "macos"
os.contains("windows") -> "windows"
else -> "linux"
}
val archPart = if (arch.contains("aarch64") || arch.contains("arm64")) "arm64" else "x64"
return "$osPart-$archPart"
}

plugins {
id("landscapist.library.compose.multiplatformWasm")
id("landscapist.spotless")
Expand Down Expand Up @@ -59,6 +72,18 @@ kotlin {
implementation(libs.androidx.core.ktx)
}
}

val desktopTest by getting {
dependencies {
implementation(kotlin("test"))
implementation(libs.jetbrains.compose.ui.test.junit4)
implementation(libs.jetbrains.compose.foundation)
// Skiko native runtime for the host OS, required to render off-screen during the
// desktop runComposeUiTest runs. The classifier is resolved from the running host so the
// tests also run on CI (e.g. linux-x64).
runtimeOnly("org.jetbrains.skiko:skiko-awt-runtime-${skikoHostTarget()}:${libs.versions.skiko.get()}")
}
}
}

targets.configureEach {
Expand Down Expand Up @@ -92,8 +117,11 @@ baselineProfile {
tasks.withType<KotlinJvmCompile>().configureEach {
compilerOptions {
jvmTarget.set(JvmTarget.JVM_17)
// Only apply explicit API mode to main sources, not tests
if (!name.contains("Test")) {
freeCompilerArgs.add("-Xexplicit-api=strict")
}
freeCompilerArgs.addAll(
"-Xexplicit-api=strict",
"-opt-in=kotlin.RequiresOptIn",
"-opt-in=kotlinx.coroutines.ExperimentalCoroutinesApi",
"-opt-in=com.skydoves.landscapist.InternalLandscapistApi",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clipToBounds
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.graphics.graphicsLayer
import androidx.compose.ui.layout.onSizeChanged
import com.skydoves.landscapist.LocalImageSourceFile
Expand All @@ -51,6 +52,7 @@ internal actual fun ZoomableContent(
zoomableState: ZoomableState,
config: ZoomableConfig,
enabled: Boolean,
onTap: ((Offset) -> Unit)?,
content: @Composable () -> Unit,
) {
// First try to get decoder from LocalImageRegionDecoder (explicitly provided)
Expand Down Expand Up @@ -109,6 +111,7 @@ internal actual fun ZoomableContent(
zoomableState = zoomableState,
config = config,
enabled = enabled,
onTap = onTap,
content = content,
)
} else {
Expand All @@ -117,6 +120,7 @@ internal actual fun ZoomableContent(
zoomableState = zoomableState,
config = config,
enabled = enabled,
onTap = onTap,
content = content,
)
}
Expand All @@ -131,6 +135,7 @@ private fun SubSamplingImageWithPlaceholder(
zoomableState: ZoomableState,
config: ZoomableConfig,
enabled: Boolean,
onTap: ((Offset) -> Unit)?,
content: @Composable () -> Unit,
) {
val isBaseLoaded = subSamplingState.isBaseLoaded
Expand All @@ -142,6 +147,7 @@ private fun SubSamplingImageWithPlaceholder(
zoomableState = zoomableState,
config = config,
enabled = enabled,
onTap = onTap,
)

// Show original content as placeholder on top until base tile is loaded
Expand All @@ -164,6 +170,7 @@ internal fun StandardZoomableContent(
zoomableState: ZoomableState,
config: ZoomableConfig,
enabled: Boolean,
onTap: ((Offset) -> Unit)? = null,
content: @Composable () -> Unit,
) {
val transformation = zoomableState.transformation
Expand All @@ -179,6 +186,7 @@ internal fun StandardZoomableContent(
Modifier.zoomGestures(
state = zoomableState,
config = config,
onTap = onTap,
)
} else {
Modifier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package com.skydoves.landscapist.zoomable

import androidx.compose.runtime.Composable
import androidx.compose.runtime.Immutable
import androidx.compose.ui.geometry.Offset
import com.skydoves.landscapist.plugins.ImagePlugin
import com.skydoves.landscapist.zoomable.internal.ZoomableContent

Expand All @@ -38,18 +39,18 @@ import com.skydoves.landscapist.zoomable.internal.ZoomableContent
*
* **With custom configuration:**
* ```kotlin
* val zoomableState = rememberZoomableState()
* // Configuration is supplied through the state.
* val zoomableState = rememberZoomableState(
* config = ZoomableConfig(
* maxZoom = 8f,
* doubleTapZoom = 3f,
* ),
* )
*
* GlideImage(
* imageModel = { imageUrl },
* component = rememberImageComponent {
* +ZoomablePlugin(
* state = zoomableState,
* config = ZoomableConfig(
* maxZoom = 8f,
* doubleTapZoom = 3f,
* ),
* )
* +ZoomablePlugin(state = zoomableState)
* },
* )
*
Expand All @@ -59,6 +60,18 @@ import com.skydoves.landscapist.zoomable.internal.ZoomableContent
* }
* ```
*
* **Handling taps:** because the zoom gesture detector consumes the pointer down, a parent
* `Modifier.clickable` will not receive taps while zoom gestures are enabled. Use [onTap] to react
* to single taps instead:
Comment on lines +63 to +65

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Narrow the tap-consumption wording.

Line 64 and Line 93 say parent clickable is blocked whenever zoom gestures are enabled, but zoomGestures now skips the tap detector when both enableDoubleTapZoom and onTap are disabled. Please document this as “while tap handling is active” to avoid contradicting the pass-through behavior.

📝 Proposed wording adjustment
- * **Handling taps:** because the zoom gesture detector consumes the pointer down, a parent
- * `Modifier.clickable` will not receive taps while zoom gestures are enabled. Use [onTap] to react
- * to single taps instead:
+ * **Handling taps:** when tap handling is active (for example, [onTap] is set or
+ * double-tap zoom is enabled), the zoom gesture detector consumes the pointer down. A parent
+ * `Modifier.clickable` will not receive those taps. Use [onTap] to react to single taps instead:
...
- *   on the image, since the zoom gesture detector consumes the pointer down and a parent
- *   `Modifier.clickable` therefore does not receive the tap while zoom gestures are enabled.
+ *   on the image while tap handling is active, since the zoom gesture detector consumes the
+ *   pointer down and a parent `Modifier.clickable` therefore does not receive the tap.

Also applies to: 91-93

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@landscapist-zoomable/src/commonMain/kotlin/com/skydoves/landscapist/zoomable/ZoomablePlugin.kt`
around lines 63 - 65, The tap-consumption note in ZoomablePlugin is too broad
because zoomGestures now bypasses the tap detector when both enableDoubleTapZoom
and onTap are disabled. Update the KDoc around ZoomablePlugin and the related
wording near zoomGestures to say parent Modifier.clickable is blocked only while
tap handling is active, and keep the guidance aligned with onTap being the
replacement for single-tap handling.

* ```kotlin
* GlideImage(
* imageModel = { imageUrl },
* component = rememberImageComponent {
* +ZoomablePlugin(onTap = { position -> openFullScreen() })
* },
* )
* ```
*
* **Reset zoom when image changes:**
* ```kotlin
* val zoomableState = rememberZoomableState(resetKey = imageUrl)
Expand All @@ -75,12 +88,16 @@ import com.skydoves.landscapist.zoomable.internal.ZoomableContent
* If null, a new state will be created internally.
* @property enabled Whether zoom gestures are enabled.
* @property onTransformChanged Callback invoked when the transformation changes.
* @property onTap Callback invoked with the tap position on a single tap. Use this to handle taps
* on the image, since the zoom gesture detector consumes the pointer down and a parent
* `Modifier.clickable` therefore does not receive the tap while zoom gestures are enabled.
*/
@Immutable
public data class ZoomablePlugin(
public val state: ZoomableState? = null,
public val enabled: Boolean = true,
public val onTransformChanged: ((ContentTransformation) -> Unit)? = null,
public val onTap: ((Offset) -> Unit)? = null,
) : ImagePlugin.ComposablePlugin {

/**
Expand All @@ -104,6 +121,7 @@ public data class ZoomablePlugin(
zoomableState = zoomableState,
config = zoomableState.config,
enabled = enabled,
onTap = onTap,
content = content,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,38 @@ import kotlinx.coroutines.launch
*
* @param state The [ZoomableState] to update based on gestures.
* @param config The [ZoomableConfig] for gesture behavior configuration.
* @param onTap Optional callback invoked with the tap position on a single tap. Provide this to
* receive tap events, since the tap detector consumes the pointer down and a parent
* `Modifier.clickable` therefore does not fire while tap handling is active.
*/
internal fun Modifier.zoomGestures(
state: ZoomableState,
config: ZoomableConfig,
onTap: ((Offset) -> Unit)? = null,
): Modifier = composed {
val scope = rememberCoroutineScope()

this
// Handle double-tap to zoom FIRST
// Handle double-tap to zoom and single-tap callbacks FIRST.
// Install this detector only when it has something to do, so that with both double-tap zoom and
// onTap disabled a single tap stays unconsumed and can reach a parent gesture handler.
.then(
if (config.enableDoubleTapZoom) {
Modifier.pointerInput(state, config) {
if (config.enableDoubleTapZoom || onTap != null) {
Modifier.pointerInput(state, config, onTap) {
detectTapGestures(
onTap = { /* Allow single tap to pass through */ },
onDoubleTap = { offset ->
scope.launch {
if (state.isZoomed) {
state.resetZoom()
} else {
state.zoomTo(config.doubleTapZoom, offset)
onTap = { offset -> onTap?.invoke(offset) },
onDoubleTap = if (config.enableDoubleTapZoom) {
{ offset ->
scope.launch {
if (state.isZoomed) {
state.resetZoom()
} else {
state.zoomTo(config.doubleTapZoom, offset)
}
}
}
} else {
null
},
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.skydoves.landscapist.zoomable.internal

import androidx.compose.runtime.Composable
import androidx.compose.ui.geometry.Offset
import com.skydoves.landscapist.zoomable.ZoomableConfig
import com.skydoves.landscapist.zoomable.ZoomableState

Expand All @@ -30,12 +31,14 @@ import com.skydoves.landscapist.zoomable.ZoomableState
* @param zoomableState The [ZoomableState] managing zoom transformations.
* @param config The [ZoomableConfig] for zoom behavior.
* @param enabled Whether zoom gestures are enabled.
* @param onTap Optional callback invoked with the tap position on a single tap.
* @param content The image content to display.
*/
@Composable
internal expect fun ZoomableContent(
zoomableState: ZoomableState,
config: ZoomableConfig,
enabled: Boolean,
onTap: ((Offset) -> Unit)?,
content: @Composable () -> Unit,
)
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import kotlinx.coroutines.flow.debounce
* @param enabled Whether zoom gestures are enabled.
* @param modifier The modifier to apply to the composable.
* @param contentDescription The content description for accessibility.
* @param onTap Optional callback invoked with the tap position on a single tap.
*/
@Composable
public fun SubSamplingImage(
Expand All @@ -62,6 +63,7 @@ public fun SubSamplingImage(
enabled: Boolean = true,
modifier: Modifier = Modifier,
contentDescription: String? = null,
onTap: ((Offset) -> Unit)? = null,
) {
var viewportSize by remember { mutableStateOf(IntSize.Zero) }
var currentFitScale by remember { mutableFloatStateOf(1f) }
Expand Down Expand Up @@ -124,6 +126,7 @@ public fun SubSamplingImage(
Modifier.zoomGestures(
state = zoomableState,
config = config,
onTap = onTap,
)
} else {
Modifier
Expand Down
Loading
Loading