Skip to content
Closed
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
2 changes: 1 addition & 1 deletion REVIEW.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ Help in CoGo is reached by **long-press**, anywhere: a progressive three-tier ex
- **Wire up help on new interactive elements.** Anything tappable — buttons, icon controls, menu items, list rows, toolbar actions — gets long-press help. A new actionable view with no tooltip is as incomplete as a missing `contentDescription`.
- **Cover new screens and panels too.** Even where pixels aren't interactive, a new screen/panel/dialog needs a top-level help entry so help is always reachable.
- **The affordance is the requirement, not finished copy.** Tooltip content may still be in authoring — fine — but the long-press must be wired and routed into the tier system. Don't ship UI that can never surface help.
- **Reuse the system.** Wire help through `idetooltips` — today the `View.displayTooltipOnLongPress(context, anchorView, category, tag)` extension (`setOnLongClickListener` → `TooltipManager.showTooltip`) — not a one-off popup.
- **Reuse the system.** Wire help through `idetooltips` — today the `View.displayTooltipOnLongPress(context, tooltipTag, tooltipCategory, holdMillis)` extension (a long-click listener for the framework's own gesture, plus a touch listener that times the longer hold ADFA-5554 asks for, both reaching `TooltipManager.showTooltip`) — not a one-off popup.
- **Compose has no native entry point yet** (tracked by **ADFA-4381**). The helper is View-based (needs an `anchorView`), so until `idetooltips` grows a Compose API, a composable wires help via `AndroidView` interop. Flag it in review rather than skipping help, and build the reusable `Modifier`/wrapper once instead of copy-pasting interop.

## 10. Architecture alignment
Expand Down
25 changes: 7 additions & 18 deletions app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import com.itsaky.androidide.utils.DiagnosticsFormatter
import com.itsaky.androidide.utils.IntentUtils.shareFile
import com.itsaky.androidide.utils.Symbols.forFile
import com.itsaky.androidide.utils.clearLongPressHelp
import com.itsaky.androidide.utils.displayTooltipOnLongPress
import com.itsaky.androidide.utils.dpToPx
import com.itsaky.androidide.utils.flashError
import com.itsaky.androidide.utils.flashSuccess
Expand Down Expand Up @@ -252,7 +253,7 @@ class EditorBottomSheet
}
}
}
binding.shareOutputAction.setOnLongClickListener(generateTooltipListener(TooltipTag.OUTPUT_SHARE_EXTERNAL))
binding.shareOutputAction.displayTooltipOnLongPress(context, TooltipTag.OUTPUT_SHARE_EXTERNAL)

binding.clearOutputAction.setOnClickListener {
val fragment =
Expand All @@ -263,7 +264,7 @@ class EditorBottomSheet
}
(fragment as ShareableOutputFragment).clearOutput()
}
binding.clearOutputAction.setOnLongClickListener(generateTooltipListener(TooltipTag.OUTPUT_CLEAR))
binding.clearOutputAction.displayTooltipOnLongPress(context, TooltipTag.OUTPUT_CLEAR)

binding.copyDiagnosticsFab.setOnClickListener {
copyDiagnosticsToClipboard()
Expand All @@ -279,7 +280,7 @@ class EditorBottomSheet
viewModel.setSheetState(sheetState = BottomSheetBehavior.STATE_EXPANDED)
fragment.beginSearch()
}
binding.searchOutputAction.setOnLongClickListener(generateTooltipListener(TooltipTag.OUTPUT_SEARCH))
binding.searchOutputAction.displayTooltipOnLongPress(context, TooltipTag.OUTPUT_SEARCH)

binding.filterOutputAction.setOnClickListener {
val fragment = pagerAdapter.getFragmentAtIndex<Fragment>(binding.tabs.selectedTabPosition)
Expand All @@ -290,23 +291,23 @@ class EditorBottomSheet
viewModel.setSheetState(sheetState = BottomSheetBehavior.STATE_EXPANDED)
fragment.toggleFilterBar()
}
binding.filterOutputAction.setOnLongClickListener(generateTooltipListener(TooltipTag.OUTPUT_FILTER))
binding.filterOutputAction.displayTooltipOnLongPress(context, TooltipTag.OUTPUT_FILTER)

updateWordWrapButtonState(EditorPreferences.outputWordWrap)
binding.wordWrapOutputAction.setOnClickListener {
val newState = !EditorPreferences.outputWordWrap
EditorPreferences.outputWordWrap = newState
updateWordWrapButtonState(newState)
}
binding.wordWrapOutputAction.setOnLongClickListener(generateTooltipListener(TooltipTag.OUTPUT_WORD_WRAP))
binding.wordWrapOutputAction.displayTooltipOnLongPress(context, TooltipTag.OUTPUT_WORD_WRAP)

binding.viewOptionsOutputAction.setOnClickListener {
val fragment = pagerAdapter.getFragmentAtIndex<Fragment>(binding.tabs.selectedTabPosition)
if (fragment is ViewOptionsOutputFragment) {
fragment.showViewOptions(it)
}
}
binding.viewOptionsOutputAction.setOnLongClickListener(generateTooltipListener(TooltipTag.OUTPUT_VIEW_OPTIONS))
binding.viewOptionsOutputAction.displayTooltipOnLongPress(context, TooltipTag.OUTPUT_VIEW_OPTIONS)

binding.headerContainer.setOnClickListener {
viewModel.setSheetState(sheetState = BottomSheetBehavior.STATE_EXPANDED)
Expand Down Expand Up @@ -388,18 +389,6 @@ class EditorBottomSheet
}
}

private fun generateTooltipListener(tooltipTag: String): OnLongClickListener =
OnLongClickListener { view: View ->
TooltipManager.showIdeCategoryTooltip(
context = context,
anchorView = view,
tag = tooltipTag,
)

// A long-click listener must return true to indicate it has consumed the event.
true
}

fun setCurrentTab(
@BottomSheetViewModel.TabDef tabIndex: Int,
) {
Expand Down
182 changes: 173 additions & 9 deletions app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package com.itsaky.androidide.ui

import android.content.Context
import android.graphics.Bitmap
import android.os.Handler
import android.os.Looper
import android.os.SystemClock
import android.util.TypedValue
import android.view.MotionEvent
Expand All @@ -38,6 +40,7 @@ import com.github.mikephil.charting.listener.OnChartGestureListener
import com.itsaky.androidide.R
import com.itsaky.androidide.idetooltips.TooltipTag
import com.itsaky.androidide.utils.MetricsAnnotationStore
import com.itsaky.androidide.utils.longPressHelpTimeoutMillis
import com.itsaky.androidide.utils.resolveAttr
import com.itsaky.androidide.utils.showIdeCategoryTooltipIfPresent
import kotlin.math.ceil
Expand Down Expand Up @@ -149,6 +152,23 @@ abstract class MetricsChartRenderer(
*/
private var appliedTextScale = Float.NaN

/**
* The gesture listener installed on the attached chart, kept so [detach] can reach its
* pending hold. Nothing else can: it lives on the chart, and a rebind installs a new one.
*/
private var axisTapListener: XAxisTapListener? = null

/**
* How the chart's hold shows its help.
*
* A seam, not a setting: `TooltipManager` reads the docs database from device storage in its
* static initialiser and cannot be loaded off-device, so without this the whole deferred-help
* path -- when it fires, when it is given up -- could not be tested at all.
*/
@VisibleForTesting
internal var showHelp: (Context, SafeLineChart, String) -> Unit =
{ context, anchor, tag -> showIdeCategoryTooltipIfPresent(context, anchor, tag) }

/**
* The top inset last reserved, so an unchanged value costs nothing.
*
Expand Down Expand Up @@ -215,7 +235,21 @@ abstract class MetricsChartRenderer(
// oldest-samples symptom this ticket was filed for -- reachable only after a pan, which is
// why the resume paths reproduce it and a fresh chart never does. Subclasses clear their own
// per-chart state through the same override.
this.chart?.let { outgoing -> if (outgoing !== chart) detach() }
//
// Unconditionally, including when the same chart is handed back. Skipping the teardown
// there let [configure] install a second gesture listener while the first stayed queued on
// the main thread with a hold nothing could reach, and added a second layout listener that
// one removeOnLayoutChangeListener cannot undo.
//
// The one thing that must survive it is the user's own viewport. detach() clears
// userHasZoomed, which is what turns the auto-follow window back on, so a rebind of an
// already-bound holder would snap a chart the user had panned back to the newest samples.
val sameChart = this.chart === chart
val hadZoomed = userHasZoomed
detach()
if (sameChart) {
userHasZoomed = hadZoomed
}
this.chart = chart
configure(chart)
chart.addOnLayoutChangeListener(newestWindowOnLayout)
Expand All @@ -230,6 +264,17 @@ abstract class MetricsChartRenderer(
open fun detach() {
userHasZoomed = false
appliedTextScale = Float.NaN
// A hold counting down survives the chart it was started on: the timer is on the main
// thread's queue. Left running it shows the outgoing page's help over whatever replaced
// it, and the replacement's listener -- a new object with its own null pendingHelp --
// could never have cancelled it.
axisTapListener?.cancelPendingHelp()
axisTapListener = null
// The chart holds the listener, and the listener is an inner class holding this renderer,
// so a detached chart left with it keeps the whole renderer alive -- and answers a later
// press through a listener whose own chart reference is now null.
chart?.onChartGestureListener = null
chart?.onSecondPointerDown = null
chart?.removeOnLayoutChangeListener(newestWindowOnLayout)
chart = null
}
Expand Down Expand Up @@ -325,7 +370,11 @@ abstract class MetricsChartRenderer(
// while the form is a circle, and load-bearing only if anyone chooses LINE.
legend.formLineWidth = 1f

onChartGestureListener = XAxisTapListener(this)
onChartGestureListener = XAxisTapListener(this).also { axisTapListener = it }
// A two-finger tap is the carousel's undock gesture, and it starts as a press like any
// other. Without this the stand-in tap fired for it and opened the sampling-rate
// chooser -- so one gesture both undocked the strip and cleared every buffer.
onSecondPointerDown = { axisTapListener?.abandonGesture() }

xAxis.valueFormatter = ElapsedTimeFormatter(sampleIntervalMillis)
// One label per 15 samples keeps the window readable without crowding.
Expand Down Expand Up @@ -430,6 +479,31 @@ abstract class MetricsChartRenderer(
private inner class XAxisTapListener(
private val chart: SafeLineChart,
) : OnChartGestureListener {
// An explicit handler, not View.postDelayed, which parks work on an unattached view's
// HandlerActionQueue until it attaches. The chart that receives a long press is attached,
// so that would happen to work -- but only by accident, and it puts the hold out of reach
// of a test. The same handler [performOnHold] uses, for the same reason.
private val handler = Handler(Looper.getMainLooper())

/** The deferred half of a long press, waiting out the rest of the hold. */
private var pendingHelp: Runnable? = null

/**
* The stand-in tap waiting for the next turn of the looper.
*
* Held for the same reason [performOnHold] holds its click: posted rather than run inline,
* it outlives the dispatch that queued it, so a [detach] landing in between would otherwise
* still open the sampling-rate chooser for a chart the renderer no longer has -- and that
* chooser clears every sample buffer.
*/
private var pendingTap: Runnable? = null

/** Whether this gesture already showed help, so its lift must not also count as a tap. */
private var helpShown = false

/** Whether the press that became a long press had started on the axis band. */
private var pendingTapOnAxis = false

override fun onChartSingleTapped(me: MotionEvent?) {
val y = me?.y ?: return
if (isOnAxisBand(y)) {
Expand All @@ -445,16 +519,91 @@ abstract class MetricsChartRenderer(
override fun onChartGestureEnd(
me: MotionEvent?,
lastPerformedGesture: ChartTouchListener.ChartGesture?,
) = Unit
) {
cancelPendingHelp()
// Lifted before the hold completed: the detector ate the tap, so stand in for it.
//
// Only for a gesture that was still a long press when it ended. A press that became a
// pan or a pinch is not a tap by any reading, and standing in for one there opened the
// sampling-rate chooser from a drag -- which clears every sample buffer, the exact
// history loss [isOnAxisBand] was narrowed to prevent. [onChartTranslate] and
// [onChartScale] give up the stand-in as the gesture escalates; this is the check for
// an escalation neither of them reports.
// A cancel is not a lift. ChartTouchListener.endAction runs for ACTION_CANCEL as well
// as ACTION_UP -- case 3 and case 1 of the same tableswitch, both reaching it with the
// original event -- and it reports mLastGesture untouched, because startAction never
// resets it. So a press an ancestor steals mid-gesture (the reveal layout, the bottom
// sheet, the pager) arrived here looking exactly like a finger lifted early, and stood
// in for a tap the user never completed. The chooser it opens clears every buffer.
val lifted = me?.actionMasked != MotionEvent.ACTION_CANCEL
if (lifted &&
!helpShown &&
pendingTapOnAxis &&
lastPerformedGesture == ChartTouchListener.ChartGesture.LONG_PRESS
) {
// Posted, not called here. This runs inside the chart's onTouchEvent, and the tap
// opens a dialog; showing one mid-dispatch leaves the chart's touch state and its
// velocity tracker part-way through a gesture. performOnHold posts its click for
// the same reason, and the two paths should not disagree.
val tap =
Runnable {
pendingTap = null
onXAxisTap?.invoke()
}
pendingTap = tap
handler.post(tap)
}
helpShown = false
pendingTapOnAxis = false
}

/**
* Drops a hold that has not fired and the tap it was standing in for.
*
* For the end of a gesture, for a gesture that turns into something else, and for
* [detach], which is the one caller outside the touch stream.
*/
fun cancelPendingHelp() {
pendingHelp?.let(handler::removeCallbacks)
pendingHelp = null
pendingTap?.let(handler::removeCallbacks)
pendingTap = null
}

override fun onChartLongPressed(me: MotionEvent?) {
val y = me?.y ?: return
val event = me ?: return
val y = event.y
val tag = helpTagAt(y) ?: return
// Haptic feedback left at its default, unlike every view-based help site, which
// passes false. Those rely on View.performLongClick buzzing for them;
// BarLineChartBase.onTouchEvent never calls super, so the framework's long press --
// and its feedback -- never runs here and this is the only thing that provides it.
showIdeCategoryTooltipIfPresent(chart.context, chart, tag)

// This arrives at the platform's own timeout -- 400ms by default, a brisk tap -- and
// help at that speed is what ADFA-5554 is about. Wait out the rest of the hold and
// show it only if the finger is still down; [onChartGestureEnd] cancels otherwise.
cancelPendingHelp()
val onAxisBand = isOnAxisBand(y)
// From the event's own downTime, not by subtracting the platform timeout from the hold.
// GestureDetector does not report a long press exactly getLongPressTimeout() after the
// finger landed: below Android Q it adds TAP_TIMEOUT, and it caches LONGPRESS_TIMEOUT
// in a static read once at class-load, so a user who lengthens the accessibility
// touch-and-hold delay moves the buttons' hold and not this one. minSdk here is 28.
val elapsed = SystemClock.uptimeMillis() - event.downTime
val remaining = (longPressHelpTimeoutMillis() - elapsed).coerceAtLeast(0L)
pendingHelp =
Runnable {
pendingHelp = null
helpShown = true
// Haptic feedback left at its default, unlike every view-based help site,
// which passes false. Those rely on View.performLongClick buzzing for them;
// BarLineChartBase.onTouchEvent never calls super, so the framework's long
// press -- and its feedback -- never runs here and this is the only thing
// that provides it.
showHelp(chart.context, chart, tag)
}.also { handler.postDelayed(it, remaining) }

// GestureDetector has already decided this gesture is a long press, so it will not
// report the tap that would have opened the sampling-rate chooser. Remember whether
// this one was headed there, so a finger lifted before the hold completes still gets
// the tap it asked for rather than nothing at all.
pendingTapOnAxis = onAxisBand
}

override fun onChartDoubleTapped(me: MotionEvent?) = Unit
Expand All @@ -472,6 +621,7 @@ abstract class MetricsChartRenderer(
scaleY: Float,
) {
userHasZoomed = true
abandonGesture()
}

override fun onChartTranslate(
Expand All @@ -483,6 +633,20 @@ abstract class MetricsChartRenderer(
// showNewestWindow dragged them back to the newest samples on the next tick -- once a
// second -- so panning a zoomed chart appeared not to work at all.
userHasZoomed = true
abandonGesture()
}

/**
* Gives up the deferred help and the stand-in tap, because this gesture has become
* something neither is meant for.
*
* A drag or a pinch can begin from a press the detector already called a long press, and
* the finger is then still down: the hold would go on to open a tooltip over a chart the
* user is in the middle of panning, and the lift would open the sampling-rate chooser.
*/
fun abandonGesture() {
cancelPendingHelp()
pendingTapOnAxis = false
}
}

Expand Down
19 changes: 19 additions & 0 deletions app/src/main/java/com/itsaky/androidide/ui/SafeLineChart.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import android.content.Context
import android.graphics.Canvas
import android.graphics.Paint
import android.util.AttributeSet
import android.view.MotionEvent
import com.github.mikephil.charting.charts.LineChart
import com.github.mikephil.charting.components.YAxis
import org.slf4j.LoggerFactory
Expand Down Expand Up @@ -80,6 +81,24 @@ class SafeLineChart : LineChart {
val color: Int,
)

/**
* Called when a second finger lands, which ends whatever one-finger gesture was in progress.
*
* MPAndroidChart's gesture listener cannot report this. `ChartTouchListener` assigns its
* `mLastGesture` only from a drag, a zoom, a long press, a tap or a fling -- never from
* ACTION_POINTER_DOWN -- so a second finger that lands and lifts without moving leaves the
* gesture still labelled LONG_PRESS, and the listener cannot tell that from a finger simply
* being lifted (ADFA-5554).
*/
var onSecondPointerDown: (() -> Unit)? = null

override fun onTouchEvent(event: MotionEvent): Boolean {
if (event.actionMasked == MotionEvent.ACTION_POINTER_DOWN) {
onSecondPointerDown?.invoke()
}
return super.onTouchEvent(event)
}

/**
* Draws the spans immediately after the grid background, which is an opaque fill of the plot: a
* span painted before [onDraw] delegates upwards is covered by it and never reaches the screen.
Expand Down
Loading
Loading