Should Modifier#transformable() offer listeners for gesture start and end? [266829800]

Assigned

Bug

Status Update

No update yet.

Description

sa...@squareup.com

created issue #1

Jan 27, 2023 06:13PM

I'm investigating if Modifier#transformable() can be used for a photo viewer screen. When a gesture ends, I'd like to reset the transformations if needed. For example, if the content is zoomed below < 1f, it'll be scaled back 1f.

I'm currently doing this by adding another pointerInput(Unit) modifier after transformable():

pointerInput(Unit) {
  awaitEachGesture {
    awaitFirstDown(requireUnconsumed = false)
    awaitAllPointersUp()

    // Reset transformations.
  }
}

Do you think this looks good enough? Or should transformable() offer start & end hooks if it offers any benefit?

Jetpack Compose version: 1.4.0-alpha05
Jetpack Compose component(s) used: Modifier#transformable()
Android Studio Build: Flamingo Canary 10
Kotlin version: 1.8.0

Comments

ma...@google.com <ma...@google.com> #2Jan 27, 2023 06:24PM

Status: Won't Fix (Intended Behavior)

Project: platform/frameworks/support
Branch: androidx-main

commit cb1afd67dacbf14cd22206c6f5b543dbf18cec1f
Author: Tony Mak <tonymak@google.com>
Date: Thu Jan 06 14:52:58 2022

Deprecate all the APIs in textclassifier

Bug: 210509084

Test: Check api/current.txt and ensure all classes are deprecated

Relnote: "Deprecate all the APIs in the textclassifier module"

Change-Id: Idc18063196531c0e926162fefeadf2dc5f559da1

M textclassifier/textclassifier/src/main/java/androidx/textclassifier/TextSelection.java
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/TextLinks.java
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/TextClassificationSessionId.java
M textclassifier/textclassifier/api/public_plus_experimental_current.txt
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/TextClassification.java
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/TextClassificationManager.java
M textclassifier/textclassifier/api/current.txt
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/TextClassificationContext.java
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/TextLinksParams.java
M textclassifier/textclassifier/api/restricted_current.txt
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/ConversationAction.java
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/TextClassifier.java
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/ConversationActions.java
M textclassifier/textclassifier/src/main/java/androidx/textclassifier/ExtrasUtils.java

https://android-review.googlesource.com/1938773

sa...@squareup.com <sa...@squareup.com> #3Jan 27, 2023 06:29PM

Is there any update when the alpha04 will be available? We need it to target api 31 because of the PendingIntent mutability flags missing in alpha03.

sa...@squareup.com <sa...@squareup.com> #4Jan 27, 2023 09:00PM

I'll schedule it for the next release. Note that alpha04 is a complete turn-down of the library where all APIs are marked as Deprecated, so you'll also want to start removing your usages of the library.

ma...@google.com <ma...@google.com> #5Jan 30, 2023 10:45AM

Status: Assigned (reopened)

Project: platform/frameworks/support
Branch: androidx-main

commit 19ed7e69823a09f3c63f8c8cdf339af8ca1645ab
Author: Alan Viverette <alanv@google.com>
Date: Mon Mar 14 15:18:16 2022

Bump textclassifier for final release and turn-down

Bug: 210509084
Change-Id: Ib8b2386903bb49baa3211f7de7fe1587fdda1483
Test: ./gradlew checkApi

M libraryversions.toml

https://android-review.googlesource.com/2024823

lp...@google.com <lp...@google.com> #6Jan 31, 2023 02:43PM

Bugjuggler: wait until 2022-03-23

sa...@squareup.com <sa...@squareup.com> #7Mar 1, 2023 05:31PM

Hi. I've received your bug and will wait until 2022-03-23 00:00 -0400 EDT and then assign the bug to alanv@google.com.

lp...@google.com <lp...@google.com> #8Mar 1, 2023 05:43PM

Is there a replacement or should we just abandon TextClassifier for android older than API Level 26 ?

sa...@saket.me <sa...@saket.me> #9Mar 1, 2023 06:05PM

Abandon. The owning team had no plans to release a replacement library.

sa...@saket.me <sa...@saket.me> #10Mar 1, 2023 06:10PM

On a second thought, a callback alone wouldn't be sufficient for enabling nested scrolling. Modifier.transformable() consumes all events by default even if its content can't pan any further, preventing scrollable parents from showing overscroll effects. I can file a separate issue for that if you want.

lp...@google.com <lp...@google.com> #11Mar 1, 2023 06:19PM

Ah I see, yes please file a separate bug. Probably transformable should just implement nested scrolling directly - if it did do that, would you still have a need to figure out the fling velocity on end?

sa...@squareup.com <sa...@squareup.com> #12Mar 1, 2023 07:49PM

If transformable can support nested scrolling, that'd be the best!

sa...@squareup.com <sa...@squareup.com> #13May 12, 2023 05:31PM

Modifier.transformable() consumes all events by default even if its content can't pan any further, preventing scrollable parents from showing overscroll effects

Am I understanding it correctly that https://android-review.googlesource.com/c/platform/frameworks/support/+/2547591 fixes this?

sa...@squareup.com <sa...@squareup.com> #14May 12, 2023 05:47PM

On a second thought, the new canPan parameter isn't sufficient. It needs to also provide the pan delta so that consumers can decide if content can be panned in the gesture direction.

lp...@google.com <lp...@google.com> May 15, 2023 01:19PM

Reassigned to ma...@google.com.

ma...@google.com <ma...@google.com> #15May 15, 2023 05:51PM

Main usecase we covered with this API is the ability to disable pan based on some external state (e.g. cumulative scale or rotataion or something else). It is not clear why you would want to decide on wether to pan or not based on the direction of the pan delta.

I understand your usecase, but this API is not intended to substitute nested scroll-like abilities

sa...@saket.me <sa...@saket.me> #16May 15, 2023 06:07PM

If my content in a pager is zoomed in, but is also panned to the edge, is there a better way to return false from canPan? The documentation of canPan recommends using scale > 1f but that may not be something that all apps use.

While I understand that this is leaking into nested scrolling, nested scrolling can't be used for 2d scrolling yet.

ma...@google.com <ma...@google.com> #17May 17, 2023 01:45PM

Hmm, I see. The thing is that I don't think it will be possible to control the consumption of the events once you started the panning, e.g. if you are at the bound of the zoomed picture and the pointer goes down - providing delta will cover this case, but not when you reach the bound with the finger already down.

The only real way to make this change correctly is the nested scroll dispatch.

sa...@saket.me <sa...@saket.me> #18May 17, 2023 06:33PM

e.g. if you are at the bound of the zoomed picture and the pointer goes down - providing delta will cover this case, but not when you reach the bound with the finger already down.

That sounds perfectly fine! Can I ask you to try out this experience using telephoto's sample on your phone? https://github.com/saket/telephoto/releases/download/0.3.0/telephoto_0.3.0_sample.apk

ma...@google.com <ma...@google.com> #19May 19, 2023 02:36PM

Accepted by ma...@google.com.

I see what you mean, that does make sense from the UX perspective. I see that in general it can be beneficial to have this info anyway, I will add Offset to canPan to help with that.

sa...@squareup.com <sa...@squareup.com> #20May 26, 2023 02:13PM

Yay thank you!

ap...@google.com <ap...@google.com> #21Jun 19, 2023 01:45PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 6bcc8c148ecd1cb13f48ea2fab0582731f44daf6
Author: malkov <malkov@google.com>
Date: Fri May 19 15:37:10 2023

Add pan change Offset to canPan parameter in transformable to help make informed decision on pan start.

Since the addition of canPan parameter in transformable, it made sense to add pan delta to this callback to help making informated decisions, e.g. when zoomed image is panned to the bound it is important to understand the direction of the pan. Providing pan delta helps to deternime this.

Relnote: Modifier.transformable now provides pan delta in canPan parameter to help determine the direction of the pan to allow or disallow it.

Test: added new
Fixes: 266829800
Change-Id: I692aa5bf43084658ca6a20175869d9d593986f2b

M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/TransformableTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/Transformable.kt

https://android-review.googlesource.com/2597810

sa...@squareup.com <sa...@squareup.com> #22Jun 19, 2023 03:44PM

Hey folks, can this issue be reopened again? https://android-review.googlesource.com/2597810 is unrelated to the original feature request.

ma...@google.com <ma...@google.com> #23Jun 19, 2023 03:52PM