-
Notifications
You must be signed in to change notification settings - Fork 99
Added basic double-tap, based off view click by cleverchuk. Addresses #1642 and #1576 #1681
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
Open
DavidGrath
wants to merge
8
commits into
open-telemetry:main
Choose a base branch
from
DavidGrath:issue-1642-double-tap
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
3b43281
Added basic double-tap, based off view click
DavidGrath a051d4a
Made fixes to correct failures to checks
DavidGrath ae74e2a
Ran apiDump
DavidGrath 838d198
Merge branch 'main' into issue-1642-double-tap
DavidGrath fc26fa0
New hardware attributes: pointer, clicks, and button. Single tap and …
DavidGrath 7c58c18
Make detekt and apiDump happy
DavidGrath 75c1407
Removed primary button exclusivity from double-tap
DavidGrath 7c2d59c
Apply suggestions from code review
DavidGrath File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| public final class io/opentelemetry/android/doubletap/DoubleTapInstrumentation : io/opentelemetry/android/instrumentation/AndroidInstrumentation { | ||
| public fun <init> ()V | ||
| public fun getName ()Ljava/lang/String; | ||
| public fun install (Lio/opentelemetry/android/instrumentation/InstallationContext;)V | ||
| } | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| plugins { | ||
| id("otel.android-library-conventions") | ||
| id("otel.publish-conventions") | ||
| } | ||
|
|
||
| description = "OpenTelemetry Android Double Tap gesture library instrumentation" | ||
|
|
||
| android { | ||
| namespace = "io.opentelemetry.android.instrumentation.doubletap" | ||
|
|
||
| defaultConfig { | ||
| consumerProguardFiles("consumer-rules.pro") | ||
| } | ||
| } | ||
|
|
||
| dependencies { | ||
| api(platform(libs.opentelemetry.platform.alpha)) // Required for sonatype publishing | ||
| implementation(project(":services")) | ||
| implementation(project(":agent-api")) | ||
| implementation(project(":instrumentation:android-instrumentation")) | ||
|
|
||
| implementation(libs.opentelemetry.instrumentation.apiSemconv) | ||
| implementation(libs.opentelemetry.semconv.incubating) | ||
|
|
||
| testImplementation(project(":test-common")) | ||
| testImplementation(project(":session")) | ||
| testImplementation(libs.robolectric) | ||
| testImplementation(libs.androidx.test.core) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # used in reflection to check if compose is available at runtime | ||
| -keepnames class androidx.compose.ui.platform.ComposeView |
4 changes: 4 additions & 0 deletions
4
instrumentation/double-tap/src/main/kotlin/internal/ViewUtils.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| package internal | ||
|
|
||
| internal const val APP_SCREEN_DOUBLE_TAP_EVENT_NAME = "app.screen.doubletap" | ||
| internal const val VIEW_DOUBLE_TAP_EVENT_NAME = "app.widget.doubletap" | ||
19 changes: 19 additions & 0 deletions
19
...ouble-tap/src/main/kotlin/io/opentelemetry/android/doubletap/DoubleTapActivityCallback.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package io.opentelemetry.android.doubletap | ||
|
|
||
| import android.app.Activity | ||
| import io.opentelemetry.android.internal.services.visiblescreen.activities.DefaultingActivityLifecycleCallbacks | ||
|
|
||
| internal class DoubleTapActivityCallback( | ||
| private val doubleTapEventGenerator: DoubleTapEventGenerator | ||
| ): DefaultingActivityLifecycleCallbacks { | ||
|
|
||
| override fun onActivityResumed(activity: Activity){ | ||
| super.onActivityResumed(activity) | ||
| doubleTapEventGenerator.startTracking(activity.window) | ||
| } | ||
|
|
||
| override fun onActivityPaused(activity: Activity) { | ||
| super.onActivityPaused(activity) | ||
| doubleTapEventGenerator.stopTracking() | ||
| } | ||
| } |
159 changes: 159 additions & 0 deletions
159
.../double-tap/src/main/kotlin/io/opentelemetry/android/doubletap/DoubleTapEventGenerator.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| package io.opentelemetry.android.doubletap | ||
|
|
||
| import android.view.GestureDetector | ||
| import android.view.MotionEvent | ||
| import android.view.View | ||
| import android.view.ViewGroup | ||
| import android.view.Window | ||
| import internal.APP_SCREEN_DOUBLE_TAP_EVENT_NAME | ||
| import internal.VIEW_DOUBLE_TAP_EVENT_NAME | ||
| import io.opentelemetry.api.common.Attributes | ||
| import io.opentelemetry.api.logs.LogRecordBuilder | ||
| import io.opentelemetry.api.logs.Logger | ||
| import io.opentelemetry.semconv.incubating.AppIncubatingAttributes.APP_SCREEN_COORDINATE_X | ||
| import io.opentelemetry.semconv.incubating.AppIncubatingAttributes.APP_SCREEN_COORDINATE_Y | ||
| import io.opentelemetry.semconv.incubating.AppIncubatingAttributes.APP_WIDGET_ID | ||
| import io.opentelemetry.semconv.incubating.AppIncubatingAttributes.APP_WIDGET_NAME | ||
| import java.lang.ref.WeakReference | ||
| import java.util.LinkedList | ||
|
|
||
| internal class DoubleTapEventGenerator( | ||
| private val eventLogger: Logger | ||
| ) { | ||
|
|
||
| private var windowRef: WeakReference<Window>? = null | ||
|
|
||
| private val gestureListener = object : GestureDetector.SimpleOnGestureListener() { | ||
|
|
||
|
|
||
| override fun onDoubleTap(motionEvent: MotionEvent): Boolean { | ||
| windowRef?.get()?.let { window -> | ||
|
|
||
| createEvent(APP_SCREEN_DOUBLE_TAP_EVENT_NAME) | ||
| .setAttribute(APP_SCREEN_COORDINATE_Y, motionEvent.y.toLong()) | ||
| .setAttribute(APP_SCREEN_COORDINATE_X, motionEvent.x.toLong()) | ||
| .emit() | ||
|
|
||
| findTargetForTap(window.decorView, motionEvent.x, motionEvent.y)?.let { view -> | ||
| createEvent(VIEW_DOUBLE_TAP_EVENT_NAME) | ||
| .setAllAttributes(createViewAttributes(view)) | ||
| .emit() | ||
| } | ||
|
|
||
| } | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| private val viewCoordinates = IntArray(2) | ||
|
|
||
|
|
||
| val gestureDetector = GestureDetector(null, gestureListener) | ||
|
|
||
| fun startTracking(window: Window) { | ||
| windowRef = WeakReference(window) | ||
| val currentCallback = window.callback | ||
| window.callback = DoubleTapWindowCallbackWrapper(currentCallback, this) | ||
|
|
||
| } | ||
|
|
||
| fun generateDoubleTap(motionEvent: MotionEvent?) { | ||
| if(motionEvent != null) { | ||
| gestureDetector.onTouchEvent(motionEvent) | ||
| } | ||
| } | ||
|
|
||
| fun stopTracking() { | ||
| windowRef?.get()?.run { | ||
| if (callback is DoubleTapWindowCallbackWrapper) { | ||
| callback = (callback as DoubleTapWindowCallbackWrapper).unwrap() | ||
| } | ||
| } | ||
| windowRef = null | ||
| } | ||
|
|
||
| private fun createEvent(name: String): LogRecordBuilder = | ||
| eventLogger | ||
| .logRecordBuilder() | ||
| .setEventName(name) | ||
|
|
||
| private fun createViewAttributes(view: View): Attributes { | ||
| val builder = Attributes.builder() | ||
| builder.put(APP_WIDGET_NAME, viewToName(view)) | ||
| builder.put(APP_WIDGET_ID, view.id.toString()) | ||
|
|
||
| builder.put(APP_SCREEN_COORDINATE_X, view.x.toLong()) | ||
| builder.put(APP_SCREEN_COORDINATE_Y, view.y.toLong()) | ||
| return builder.build() | ||
| } | ||
|
|
||
| private fun viewToName(view: View): String = | ||
| try { | ||
| view.resources?.getResourceEntryName(view.id) ?: view.id.toString() | ||
| } catch (throwable: Throwable) { | ||
| view.id.toString() | ||
| } | ||
|
|
||
| private fun findTargetForTap( | ||
| decorView: View, | ||
| x: Float, | ||
| y: Float, | ||
| ): View? { | ||
| val queue = LinkedList<View>() | ||
| queue.addFirst(decorView) | ||
| var target: View? = null | ||
|
|
||
| while (queue.isNotEmpty()) { | ||
| val view = queue.removeFirst() | ||
| if (isJetpackComposeView(view)) { | ||
| return null | ||
| } | ||
|
|
||
| if (isValidClickTarget(view)) { | ||
| target = view | ||
| } | ||
|
|
||
| if (view is ViewGroup) { | ||
| handleViewGroup(view, x, y, queue) | ||
| } | ||
| } | ||
| return target | ||
| } | ||
|
|
||
| private fun isValidClickTarget(view: View): Boolean = view.isClickable && view.isVisible | ||
|
|
||
| private fun handleViewGroup( | ||
| view: ViewGroup, | ||
| x: Float, | ||
| y: Float, | ||
| stack: LinkedList<View>, | ||
| ) { | ||
| if (!view.isVisible) return | ||
|
|
||
| for (i in 0 until view.childCount) { | ||
| val child = view.getChildAt(i) | ||
| if (hitTest(child, x, y) && !isJetpackComposeView(child)) { | ||
| stack.add(child) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun hitTest( | ||
| view: View, | ||
| x: Float, | ||
| y: Float, | ||
| ): Boolean { | ||
| view.getLocationInWindow(viewCoordinates) | ||
| val vx = viewCoordinates[0] | ||
| val vy = viewCoordinates[1] | ||
|
|
||
| val w = view.width | ||
| val h = view.height | ||
| return !(x < vx || x > vx + w || y < vy || y > vy + h) | ||
| } | ||
|
|
||
| private fun isJetpackComposeView(view: View): Boolean = view::class.java.name.startsWith("androidx.compose.ui.platform.ComposeView") | ||
|
|
||
| private val View.isVisible: Boolean | ||
| get() = visibility == View.VISIBLE | ||
| } |
29 changes: 29 additions & 0 deletions
29
...double-tap/src/main/kotlin/io/opentelemetry/android/doubletap/DoubleTapInstrumentation.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package io.opentelemetry.android.doubletap | ||
|
|
||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import com.google.auto.service.AutoService | ||
| import io.opentelemetry.android.instrumentation.AndroidInstrumentation | ||
| import io.opentelemetry.android.instrumentation.InstallationContext | ||
|
|
||
| @AutoService(AndroidInstrumentation::class) | ||
| class DoubleTapInstrumentation: AndroidInstrumentation { | ||
| override val name: String = "doubletap" | ||
|
|
||
| override fun install(ctx: InstallationContext) { | ||
| ctx.application?.registerActivityLifecycleCallbacks( | ||
| DoubleTapActivityCallback( | ||
| DoubleTapEventGenerator( | ||
| ctx.openTelemetry | ||
| .logsBridge | ||
| .loggerBuilder("io.opentelemetry.android.instrumentation.doubletap") | ||
| .build(), | ||
| ), | ||
| ), | ||
| ) | ||
| } | ||
|
|
||
| } |
44 changes: 44 additions & 0 deletions
44
...-tap/src/main/kotlin/io/opentelemetry/android/doubletap/DoubleTapWindowCallbackWrapper.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package io.opentelemetry.android.doubletap | ||
|
|
||
| import android.os.Build.VERSION_CODES | ||
| import android.view.ActionMode | ||
| import android.view.KeyboardShortcutGroup | ||
| import android.view.Menu | ||
| import android.view.MotionEvent | ||
| import android.view.SearchEvent | ||
| import android.view.Window.Callback | ||
| import androidx.annotation.RequiresApi | ||
|
|
||
| internal class DoubleTapWindowCallbackWrapper( | ||
| private val callback: Callback, | ||
| private val doubleTapEventGenerator: DoubleTapEventGenerator, | ||
| ): Callback by callback { | ||
|
|
||
| override fun dispatchTouchEvent(event: MotionEvent?): Boolean { | ||
| doubleTapEventGenerator.generateDoubleTap(event) | ||
| return callback.dispatchTouchEvent(event) | ||
| } | ||
|
|
||
| @RequiresApi(api = VERSION_CODES.O) | ||
| override fun onPointerCaptureChanged(hasCapture: Boolean) { | ||
| callback.onPointerCaptureChanged(hasCapture) | ||
| } | ||
|
|
||
| @RequiresApi(api = VERSION_CODES.N) | ||
| override fun onProvideKeyboardShortcuts( | ||
| data: List<KeyboardShortcutGroup?>?, | ||
| menu: Menu?, | ||
| deviceId: Int, | ||
| ) { | ||
| callback.onProvideKeyboardShortcuts(data, menu, deviceId) | ||
| } | ||
|
|
||
| override fun onSearchRequested(searchEvent: SearchEvent?): Boolean = callback.onSearchRequested(searchEvent) | ||
|
|
||
| override fun onWindowStartingActionMode( | ||
| callback: ActionMode.Callback?, | ||
| type: Int, | ||
| ): ActionMode? = this.callback.onWindowStartingActionMode(callback, type) | ||
|
|
||
| fun unwrap(): Callback = callback | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we not use a generic click/tap event name and use attribute to ie distinguish between the type ie single/double click
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review.
The idea is to eventually expand to other gestures including scroll and pinch, that's the reasoning behind me leaving single tap distinct from double tap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no problem with scroll & pinch having dedicated name as they would also have different attributes. However in the case of clicks (single vs double) they would have the same attributes and the number of definitions triples when you consider that it could also be a left vs right click when using a mouse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I see it working out like this:
InputDevice.getSources()source as 2 attributesMotionEvent.getToolType()as an attributeMotionEvent.getButtonState()as an attribute if it's greater than 0I'd be translating the returned integers into strings
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and also add
click.typefor either single or doubleThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that as otherwise we would have had so many different event names.
The question is attribute names. I am thinking:
hw.pointer.typefor the tool type as it is describing the hardware that was usedhw.pointer.buttonfor the button state with stylus_primary shortened to primary etc.hw.pointer.clicksfor how many clicks ie 1 or 2There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I assume we're only focusing on
SOURCE_CLASS_POINTERsince that's where gestures typically come from. I think it's good, although I'd prefer hardware in full instead of shortened.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try and get back to this on or before the 18th
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries, note
hw.*is a defined namespace in semconv. 😉There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Thanks for pointing that out. Noted