From 0984cb5a1e20b1fe67b5ff35d5c28c50b0adb2bf Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 00:01:51 -0700 Subject: [PATCH 1/9] feat(metrics): add long-press help to the metrics carousel (ADFA-5510) Twelve tags under "carousel.", declared in TooltipTag alongside every other area. No "ide." prefix despite the ticket's wording: the lookup is by tag AND category, the category column already carries "ide", and none of the 530 existing ide-category rows are prefixed -- a prefixed tag would have made the carousel the sole exception. The three charts do not use setOnLongClickListener, and that is the whole subtlety of this change. MPAndroidChart's BarLineChartBase.onTouchEvent hands the event to its own touch listener and never calls super, so the framework's long-press detection never runs: a view listener would have been installed, looked wired in review, and never fired -- the same shape as the sampling-rate tap that was bound to the wrong edge of the chart. The charts answer through onChartLongPressed instead, which the renderer already implemented as a no-op. That callback also decides between two tags by where the press landed: below the plot is the time axis, which is what the sampling rate belongs to, and inside the plot is the metric itself. The decision is split out into helpTagAt so it can be tested -- TooltipManager reads the docs database from device storage in its static initialiser and cannot be loaded off-device, so a test can assert which tag is chosen but not that a tooltip appears. The rate chooser is a dialog with no free surface to long-press, so help is a neutral button there. It deliberately does not dismiss: the point is to read it and then pick a rate. Everything else is wired once, in MetricsCarouselController.bind(), which runs for the docked strip and the floating window alike -- the window's own chrome already carries the window-* tags. unbind() clears the listeners and also resets isLongClickable, which setOnLongClickListener(null) leaves set: the view would otherwise still claim a long press it no longer answers. Tier 1 and Tier 2 copy for all twelve tags is written; Tier 3 has nothing to link to, as there is no i/ help page for the carousel yet. Verified on a Pixel 6 Pro with the authored database pushed to /sdcard/Download/documentation.db, which the debug path prefers when it is newer: the arrows, title, camera and both chart regions each resolve their own tag and render the real copy, and the dialog's Help button resolves carousel.rate over the open dialog. Known gap: an arrow dimmed at the end of the carousel gives no help. A disabled view that is long-clickable consumes the touch and never fires the long press, so it does not fall through to the panel either. Fixing it means the arrows stop being disabled, which is an accessibility trade-off worth deciding explicitly rather than in passing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../androidide/ui/MemoryUsageChartRenderer.kt | 3 + .../ui/MetricsCarouselController.kt | 52 ++++++ .../androidide/ui/MetricsChartRenderer.kt | 37 +++- .../ui/NetworkUsageChartRenderer.kt | 3 + .../androidide/ui/PowerUsageChartRenderer.kt | 3 + .../androidide/ui/MetricsCarouselHelpTest.kt | 159 ++++++++++++++++++ .../androidide/idetooltips/TooltipTag.kt | 15 ++ 7 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt diff --git a/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt index de412b4262..a7d3af98c6 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt @@ -26,6 +26,7 @@ import com.github.mikephil.charting.data.LineData import com.github.mikephil.charting.data.LineDataSet import com.github.mikephil.charting.formatter.IAxisValueFormatter import com.itsaky.androidide.R +import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.utils.MemoryUsageWatcher import com.itsaky.androidide.utils.MemoryUsageWatcher.ProcessMemoryInfo import com.itsaky.androidide.utils.MetricsAnnotationStore @@ -73,6 +74,8 @@ class MemoryUsageChartRenderer( * process's complete [ProcessMemoryInfo.usageHistory]. Call when the set of watched processes * changes; [onUsagesChanged] calls it on its own when it detects such a change. */ + override val helpTag: String = TooltipTag.CAROUSEL_CHART_MEMORY + @UiThread override fun rebuild() { val chart = this.chart ?: return diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt index 01a661b265..3057a454e1 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt @@ -22,10 +22,12 @@ import android.view.ViewGroup import android.widget.ArrayAdapter import android.widget.Toast import androidx.annotation.UiThread +import androidx.appcompat.app.AlertDialog import androidx.core.view.isVisible import androidx.viewpager2.widget.ViewPager2 import com.itsaky.androidide.app.configuration.IDEBuildConfigProvider import com.itsaky.androidide.databinding.LayoutMemUsageBinding +import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.resources.R.string import com.itsaky.androidide.utils.DialogUtils import com.itsaky.androidide.utils.IntentUtils @@ -35,6 +37,8 @@ import com.itsaky.androidide.utils.MetricsSamplingRates import com.itsaky.androidide.utils.MetricsSnapshot import com.itsaky.androidide.utils.NetworkUsageWatcher import com.itsaky.androidide.utils.PowerUsageWatcher +import com.itsaky.androidide.utils.displayTooltipOnLongPress +import com.itsaky.androidide.utils.showIdeCategoryTooltipIfPresent import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -190,11 +194,36 @@ class MetricsCarouselController( binding.metricsNext.setOnClickListener { step(1) } updateArrows(binding.metricsPager.currentItem) + wireHelp(binding) + memoryUsageWatcher.listener = memoryListener networkUsageWatcher.listener = networkListener powerUsageWatcher.listener = powerListener } + /** + * Gives every control in the strip its long-press help (ADFA-5510). + * + * Here rather than at each host, because this runs for the docked strip and for the floating + * window alike -- the window's own chrome already carries the `window-*` tags, and the carousel + * inside it is this same controller. + * + * The charts are absent from this list on purpose: MPAndroidChart swallows the touch events a + * view-level long press would need, so each renderer answers through the chart's gesture + * listener instead. + */ + @UiThread + private fun wireHelp(binding: LayoutMemUsageBinding) { + val context = binding.root.context + binding.root.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_PANEL) + binding.metricsTitle.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_TITLE) + binding.metricsPrevious.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_PREVIOUS) + binding.metricsNext.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_NEXT) + binding.metricsSnapshot.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_SNAPSHOT) + binding.metricsBattery.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_BATTERY) + binding.metricsUndockedMessage.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_UNDOCKED) + } + /** * Stops feeding the carousel and releases the bound views. Sampling is unaffected -- the * watchers keep their history, so re-binding shows it in full. @@ -215,6 +244,22 @@ class MetricsCarouselController( networkRenderer.onXAxisTap = null powerRenderer.onXAxisTap = null binding?.metricsSnapshot?.setOnClickListener(null) + binding?.let { bound -> + listOf( + bound.root, + bound.metricsTitle, + bound.metricsPrevious, + bound.metricsNext, + bound.metricsSnapshot, + bound.metricsBattery, + bound.metricsUndockedMessage, + ).forEach { control -> + control.setOnLongClickListener(null) + // setOnLongClickListener(null) leaves isLongClickable set, so the view would still + // claim a long press it no longer answers. + control.isLongClickable = false + } + } binding?.metricsPrevious?.setOnClickListener(null) binding?.metricsNext?.setOnClickListener(null) pageCallback?.let { binding?.metricsPager?.unregisterOnPageChangeCallback(it) } @@ -340,7 +385,14 @@ class MetricsCarouselController( // No setMessage: an AlertDialog shows either a message or a list, never both, and // the message silently wins. The unavailable entries carry the explanation instead. .setNegativeButton(string.cancel) { dismissable, _ -> dismissable.dismiss() } + // A dialog has no free surface to long-press, so help is a button here rather than a + // gesture. It does not dismiss: the point is to read it and then choose a rate. + .setNeutralButton(string.help, null) .show() + + dialog.getButton(AlertDialog.BUTTON_NEUTRAL)?.setOnClickListener { helpAnchor -> + showIdeCategoryTooltipIfPresent(context, helpAnchor, TooltipTag.CAROUSEL_RATE) + } } /** diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt index 7f594941ce..3d7f32e221 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt @@ -22,6 +22,7 @@ import android.os.SystemClock import android.view.MotionEvent import androidx.annotation.CallSuper import androidx.annotation.UiThread +import androidx.annotation.VisibleForTesting import com.github.mikephil.charting.components.AxisBase import com.github.mikephil.charting.components.LimitLine import com.github.mikephil.charting.components.XAxis @@ -31,8 +32,10 @@ import com.github.mikephil.charting.formatter.IAxisValueFormatter import com.github.mikephil.charting.listener.ChartTouchListener 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.resolveAttr +import com.itsaky.androidide.utils.showIdeCategoryTooltipIfPresent import kotlin.math.roundToLong /** @@ -63,6 +66,34 @@ abstract class MetricsChartRenderer( */ var onXAxisTap: (() -> Unit)? = null + /** + * The help tag for this page's plot, shown on a long press (ADFA-5510). + * + * Routed through the chart's own gesture listener rather than [android.view.View.setOnLongClickListener]: + * MPAndroidChart's `BarLineChartBase.onTouchEvent` hands the event to its touch listener and + * never calls `super`, so the framework's long-press detection never runs and a view listener + * would be installed, look wired, and never fire. + */ + protected open val helpTag: String? = null + + /** + * The help tag for a long press at [y], or `null` if this page has none. + * + * Separated from showing the tooltip so it can be tested: TooltipManager reads the docs + * database from device storage in its static initialiser and cannot be loaded off-device. + */ + @VisibleForTesting + internal fun helpTagAt(y: Float): String? { + val chart = this.chart ?: return null + // The axis band answers for the sampling rate, the plot for the metric itself, matching + // where a tap goes. + return if (y >= chart.viewPortHandler.contentBottom()) { + TooltipTag.CAROUSEL_AXIS_TIME + } else { + helpTag + } + } + /** * Whether the user has pinched this chart. * @@ -236,7 +267,11 @@ abstract class MetricsChartRenderer( lastPerformedGesture: ChartTouchListener.ChartGesture?, ) = Unit - override fun onChartLongPressed(me: MotionEvent?) = Unit + override fun onChartLongPressed(me: MotionEvent?) { + val y = me?.y ?: return + val tag = helpTagAt(y) ?: return + showIdeCategoryTooltipIfPresent(chart.context, chart, tag) + } override fun onChartDoubleTapped(me: MotionEvent?) = Unit diff --git a/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt index d349fc75a0..62f9a0b72b 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt @@ -25,6 +25,7 @@ import com.github.mikephil.charting.data.Entry import com.github.mikephil.charting.data.LineDataSet import com.github.mikephil.charting.formatter.IAxisValueFormatter import com.itsaky.androidide.R +import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.utils.MetricsAnnotationStore import com.itsaky.androidide.utils.NetworkUsageWatcher import com.itsaky.androidide.utils.NetworkUsageWatcher.NetworkUsage @@ -66,6 +67,8 @@ class NetworkUsageChartRenderer( /** * Rebuilds both series from the full sample history. */ + override val helpTag: String = TooltipTag.CAROUSEL_CHART_NETWORK + @UiThread override fun rebuild() { val chart = this.chart ?: return diff --git a/app/src/main/java/com/itsaky/androidide/ui/PowerUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/PowerUsageChartRenderer.kt index 89e090d21d..3bb4128a06 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/PowerUsageChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/PowerUsageChartRenderer.kt @@ -26,6 +26,7 @@ import com.github.mikephil.charting.data.Entry import com.github.mikephil.charting.data.LineDataSet import com.github.mikephil.charting.formatter.IAxisValueFormatter import com.itsaky.androidide.R +import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.utils.MetricsAnnotationStore import com.itsaky.androidide.utils.PowerUsageWatcher import com.itsaky.androidide.utils.PowerUsageWatcher.PowerUsage @@ -58,6 +59,8 @@ class PowerUsageChartRenderer( sampleIntervalMillis = sampleIntervalMillis, annotations = annotations, ) { + override val helpTag: String = TooltipTag.CAROUSEL_CHART_POWER + @UiThread override fun rebuild() { val chart = this.chart ?: return diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt new file mode 100644 index 0000000000..6b8422a3e0 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt @@ -0,0 +1,159 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.ui + +import android.content.Context +import android.view.LayoutInflater +import android.view.View +import androidx.appcompat.view.ContextThemeWrapper +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.R +import com.itsaky.androidide.databinding.LayoutMemUsageBinding +import com.itsaky.androidide.idetooltips.TooltipTag +import com.itsaky.androidide.utils.MemoryUsageWatcher +import com.itsaky.androidide.utils.MetricsAnnotationStore +import com.itsaky.androidide.utils.NetworkUsageWatcher +import com.itsaky.androidide.utils.PowerUsageWatcher +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Pins that every control in the metrics carousel answers a long press (ADFA-5510). + * + * The assertion is that a listener is installed, not that a tooltip appears: TooltipManager reads + * the docs database from device storage in its static initialiser and cannot be loaded off-device. + * Whether a tag has copy behind it is the database's business, not this code's. + */ +@RunWith(RobolectricTestRunner::class) +class MetricsCarouselHelpTest { + private val context: Context = + ContextThemeWrapper( + ApplicationProvider.getApplicationContext(), + R.style.Theme_AndroidIDE, + ) + + private fun boundStrip(): LayoutMemUsageBinding { + val binding = LayoutMemUsageBinding.inflate(LayoutInflater.from(context)) + controller().bind(binding) + return binding + } + + private fun controller() = + MetricsCarouselController( + memoryUsageWatcher = MemoryUsageWatcher(), + networkUsageWatcher = NetworkUsageWatcher(uid = TEST_UID), + powerUsageWatcher = + PowerUsageWatcher( + source = { + PowerUsageWatcher.PowerReading( + temperatureMilliCelsius = 30_000L, + powerMicroWatts = 1_000_000L, + thermalStatus = 0, + battery = PowerUsageWatcher.BatteryState.UNKNOWN, + ) + }, + ), + lineColorFor = { android.graphics.Color.BLUE }, + annotations = MetricsAnnotationStore(), + ) + + @Test + fun `every control in the strip answers a long press`() { + val binding = boundStrip() + + val controls: List> = + listOf( + "panel" to binding.root, + "title" to binding.metricsTitle, + "previous" to binding.metricsPrevious, + "next" to binding.metricsNext, + "snapshot" to binding.metricsSnapshot, + "battery" to binding.metricsBattery, + "undocked message" to binding.metricsUndockedMessage, + ) + + val unwired = controls.filterNot { (_, view) -> view.isLongClickable }.map { it.first } + assertThat(unwired).isEmpty() + } + + @Test + fun `an unbound strip has no help wired`() { + // Guards the test above: if inflation alone made these long-clickable, it would pass + // against a controller that wires nothing. + val binding = LayoutMemUsageBinding.inflate(LayoutInflater.from(context)) + + assertThat(binding.metricsPrevious.isLongClickable).isFalse() + assertThat(binding.metricsSnapshot.isLongClickable).isFalse() + } + + @Test + fun `unbinding releases the help listeners`() { + val binding = LayoutMemUsageBinding.inflate(LayoutInflater.from(context)) + val controller = controller() + controller.bind(binding) + controller.unbind() + + assertThat(binding.metricsPrevious.isLongClickable).isFalse() + assertThat(binding.metricsSnapshot.isLongClickable).isFalse() + } + + @Test + fun `each chart page declares its own help tag`() { + // The charts are not in the list above: MPAndroidChart swallows the touch events a view + // long press needs, so they answer through the chart's gesture listener instead. + assertThat(TooltipTag.CAROUSEL_CHART_MEMORY).isEqualTo("carousel.chart.memory") + assertThat(TooltipTag.CAROUSEL_CHART_NETWORK).isEqualTo("carousel.chart.network") + assertThat(TooltipTag.CAROUSEL_CHART_POWER).isEqualTo("carousel.chart.power") + } + + @Test + fun `a long press below the plot asks about the sampling rate, not the metric`() { + val chart = SafeLineChart(context) + val renderer = + NetworkUsageChartRenderer( + usageProvider = { + NetworkUsageWatcher.NetworkUsage(LongArray(SAMPLES) { 1_000L }, LongArray(SAMPLES) { 500L }) + }, + ) + renderer.attach(chart) + chart.measure( + View.MeasureSpec.makeMeasureSpec(WIDTH, View.MeasureSpec.EXACTLY), + View.MeasureSpec.makeMeasureSpec(HEIGHT, View.MeasureSpec.EXACTLY), + ) + chart.layout(0, 0, WIDTH, HEIGHT) + + val handler = chart.viewPortHandler + // Guards the two assertions below: on an unlaid-out chart both points land on one edge. + assertThat(handler.contentBottom()).isLessThan(HEIGHT.toFloat()) + + // Below the plot is the time axis, which is what the sampling rate belongs to. + assertThat(renderer.helpTagAt(handler.contentBottom() + 1f)).isEqualTo(TooltipTag.CAROUSEL_AXIS_TIME) + // Inside the plot, the metric itself answers. + assertThat(renderer.helpTagAt((handler.contentTop() + handler.contentBottom()) / 2f)) + .isEqualTo(TooltipTag.CAROUSEL_CHART_NETWORK) + } + + private companion object { + const val TEST_UID = 10_123 + const val WIDTH = 720 + const val HEIGHT = 400 + const val SAMPLES = 60 + } +} diff --git a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt index 7c4e23f0e3..64d2025174 100644 --- a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt +++ b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt @@ -314,4 +314,19 @@ object TooltipTag { const val GIT_DIALOG_ABORT_MERGE = "git.dialog.abortmerge" const val GIT_PUSH = "git.action.push" const val GIT_PULL = "git.action.pull" + + // Editor metrics carousel (ADFA-5510). Unprefixed like every other tag here: the lookup is by + // tag AND category, and the category column already carries "ide". + const val CAROUSEL_PANEL = "carousel.panel" + const val CAROUSEL_TITLE = "carousel.title" + const val CAROUSEL_PREVIOUS = "carousel.previous" + const val CAROUSEL_NEXT = "carousel.next" + const val CAROUSEL_SNAPSHOT = "carousel.snapshot" + const val CAROUSEL_CHART_MEMORY = "carousel.chart.memory" + const val CAROUSEL_CHART_NETWORK = "carousel.chart.network" + const val CAROUSEL_CHART_POWER = "carousel.chart.power" + const val CAROUSEL_BATTERY = "carousel.battery" + const val CAROUSEL_AXIS_TIME = "carousel.axis.time" + const val CAROUSEL_RATE = "carousel.rate" + const val CAROUSEL_UNDOCKED = "carousel.undocked" } From eb98d74997c8b6d1fa938b1b8bd3a9099e80de29 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 01:11:22 -0700 Subject: [PATCH 2/9] docs(metrics): re-attach the rebuild KDoc the help tag displaced (ADFA-5510) Adding `override val helpTag` above `rebuild()` left each renderer's rebuild KDoc documenting the property instead of the function. The property is already documented on the base class, so the fix is to put it above the comment rather than to write a second one. ktlint did not catch it: a KDoc before a property is legal, and only the identical mistake in MetricsAnnotationStore -- where the doc ended up inside a class body with nothing to attach to -- tripped the linter. Co-Authored-By: Claude Opus 5 --- .../java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt | 3 ++- .../java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt index a7d3af98c6..0a2b886829 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt @@ -69,12 +69,13 @@ class MemoryUsageChartRenderer( pidToDatasetIdx.clear() } + override val helpTag: String = TooltipTag.CAROUSEL_CHART_MEMORY + /** * Rebuilds the chart's datasets from scratch for the currently watched processes, rendering each * process's complete [ProcessMemoryInfo.usageHistory]. Call when the set of watched processes * changes; [onUsagesChanged] calls it on its own when it detects such a change. */ - override val helpTag: String = TooltipTag.CAROUSEL_CHART_MEMORY @UiThread override fun rebuild() { diff --git a/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt index bfe570290b..4a3e4de0a9 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt @@ -64,10 +64,11 @@ class NetworkUsageChartRenderer( sampleIntervalMillis = sampleIntervalMillis, annotations = annotations, ) { + override val helpTag: String = TooltipTag.CAROUSEL_CHART_NETWORK + /** * Rebuilds both series from the full sample history. */ - override val helpTag: String = TooltipTag.CAROUSEL_CHART_NETWORK @UiThread override fun rebuild() { From 379b91c51ca045dbd97fd3ed1c3974236ad4e476 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 07:54:13 -0700 Subject: [PATCH 3/9] fix(metrics): keep the undocked message's help, and stop the target list drifting (ADFA-5510) The "In a floating window" message could never show its tooltip, and the fault was mine twice over. It is only visible while the carousel is undocked -- and undocking calls unbind(), whose listener-clearing block I added for a different reason took that view's long-press listener with it. Docked, the view is gone. So the tag was unreachable in both states, which is the whole of what it was for. The controls are now one list, helpTargets(binding), driving the wiring, the unwiring and the test. There were three hand-maintained copies, which is precisely how a control added later gets help on binding and keeps a stale listener capturing a dead binding after unbinding -- the hazard MetricsCarouselLayout.setUndocked already carries a comment about. Unbinding clears every target except the undocked message, for the reason above. The test that claimed to check the tags asserted the TooltipTag constants against their own string literals, so it would have passed with two controls' tags swapped and never read the wiring at all. It now asserts which view each tag reaches, and that no two controls share one. Left as it is: carousel.panel. Its children tile the strip, so a long press almost always lands on a child that answers for itself, and the base of the strip is the pager, whose chart consumes its own touches. It is a genuine catch-all for the gaps rather than a control, and wiring it costs nothing -- but it will rarely be what answers, and the PR should not claim otherwise. Co-Authored-By: Claude Opus 5 --- .../ui/MetricsCarouselController.kt | 61 ++++++++++++------- .../androidide/ui/MetricsCarouselHelpTest.kt | 33 ++++++++-- 2 files changed, 67 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt index 5939076eb5..785bdc141f 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt @@ -26,6 +26,7 @@ import android.view.ViewGroup import android.widget.ArrayAdapter import android.widget.Toast import androidx.annotation.UiThread +import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog import androidx.core.view.isVisible import androidx.viewpager2.widget.ViewPager2 @@ -222,15 +223,35 @@ class MetricsCarouselController( @UiThread private fun wireHelp(binding: LayoutMemUsageBinding) { val context = binding.root.context - binding.root.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_PANEL) - binding.metricsTitle.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_TITLE) - binding.metricsPrevious.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_PREVIOUS) - binding.metricsNext.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_NEXT) - binding.metricsSnapshot.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_SNAPSHOT) - binding.metricsBattery.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_BATTERY) - binding.metricsUndockedMessage.displayTooltipOnLongPress(context, TooltipTag.CAROUSEL_UNDOCKED) + helpTargets(binding).forEach { (view, tag) -> + view.displayTooltipOnLongPress(context, tag) + } } + /** + * Every control that answers a long press, and the tag it answers with. + * + * One list drives the wiring, the unwiring and the test, because three hand-maintained copies + * is how a control added later gets help on binding and keeps a stale listener after unbinding. + * + * The charts are absent on purpose: MPAndroidChart swallows the touch events a view-level long + * press needs, so each renderer answers through the chart's own gesture listener instead. + */ + @VisibleForTesting + internal fun helpTargets(binding: LayoutMemUsageBinding): List> = + listOf( + // The strip itself, for the gaps its children do not cover. + binding.root to TooltipTag.CAROUSEL_PANEL, + binding.metricsTitle to TooltipTag.CAROUSEL_TITLE, + binding.metricsPrevious to TooltipTag.CAROUSEL_PREVIOUS, + binding.metricsNext to TooltipTag.CAROUSEL_NEXT, + binding.metricsSnapshot to TooltipTag.CAROUSEL_SNAPSHOT, + binding.metricsBattery to TooltipTag.CAROUSEL_BATTERY, + // Wired even though it is only visible while undocked: the message is the one control + // that outlives unbind(), so its help must not be torn down with the rest. + binding.metricsUndockedMessage to TooltipTag.CAROUSEL_UNDOCKED, + ) + /** * Stops feeding the carousel and releases the bound views. Sampling is unaffected -- the * watchers keep their history, so re-binding shows it in full. @@ -252,20 +273,18 @@ class MetricsCarouselController( powerRenderer.onXAxisTap = null binding?.metricsSnapshot?.setOnClickListener(null) binding?.let { bound -> - listOf( - bound.root, - bound.metricsTitle, - bound.metricsPrevious, - bound.metricsNext, - bound.metricsSnapshot, - bound.metricsBattery, - bound.metricsUndockedMessage, - ).forEach { control -> - control.setOnLongClickListener(null) - // setOnLongClickListener(null) leaves isLongClickable set, so the view would still - // claim a long press it no longer answers. - control.isLongClickable = false - } + helpTargets(bound) + // All but the undocked message: that view becomes visible *because* the carousel + // unbound, so clearing its listener here left the one control the user can still + // reach with no help at all. + .filterNot { (view, _) -> view === bound.metricsUndockedMessage } + .map { (view, _) -> view } + .forEach { control -> + control.setOnLongClickListener(null) + // setOnLongClickListener(null) leaves isLongClickable set, so the view would still + // claim a long press it no longer answers. + control.isLongClickable = false + } } binding?.metricsPrevious?.setOnClickListener(null) binding?.metricsNext?.setOnClickListener(null) diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt index 6b8422a3e0..ed9ec75c93 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt @@ -115,12 +115,33 @@ class MetricsCarouselHelpTest { } @Test - fun `each chart page declares its own help tag`() { - // The charts are not in the list above: MPAndroidChart swallows the touch events a view - // long press needs, so they answer through the chart's gesture listener instead. - assertThat(TooltipTag.CAROUSEL_CHART_MEMORY).isEqualTo("carousel.chart.memory") - assertThat(TooltipTag.CAROUSEL_CHART_NETWORK).isEqualTo("carousel.chart.network") - assertThat(TooltipTag.CAROUSEL_CHART_POWER).isEqualTo("carousel.chart.power") + fun `each control is wired to its own tag`() { + val binding = LayoutMemUsageBinding.inflate(LayoutInflater.from(context)) + val targets = controller().helpTargets(binding) + + // Asserting the constants against their own literals, as this test used to, would pass + // just as happily with two controls' tags swapped. + val byTag = targets.associate { (view, tag) -> tag to view } + assertThat(byTag[TooltipTag.CAROUSEL_PREVIOUS]).isSameInstanceAs(binding.metricsPrevious) + assertThat(byTag[TooltipTag.CAROUSEL_NEXT]).isSameInstanceAs(binding.metricsNext) + assertThat(byTag[TooltipTag.CAROUSEL_SNAPSHOT]).isSameInstanceAs(binding.metricsSnapshot) + assertThat(byTag[TooltipTag.CAROUSEL_BATTERY]).isSameInstanceAs(binding.metricsBattery) + assertThat(byTag[TooltipTag.CAROUSEL_TITLE]).isSameInstanceAs(binding.metricsTitle) + assertThat(byTag[TooltipTag.CAROUSEL_UNDOCKED]).isSameInstanceAs(binding.metricsUndockedMessage) + // Every tag distinct, so no two controls can answer with the same one. + assertThat(targets.map { it.second }.toSet()).hasSize(targets.size) + } + + @Test + fun `unbinding keeps the undocked message answering`() { + val binding = LayoutMemUsageBinding.inflate(LayoutInflater.from(context)) + val controller = controller() + controller.bind(binding) + controller.unbind() + + // That view becomes visible *because* the carousel unbound, so clearing its listener left + // the one control a user can still reach with no help at all. + assertThat(binding.metricsUndockedMessage.isLongClickable).isTrue() } @Test From 93bee8c9ac851a70ec1df9f1fa9921926efb1dd3 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 09:45:41 -0700 Subject: [PATCH 4/9] ADFA-5510: address review findings on the carousel's help The undocked carousel was the one floating window whose chrome answered no long press. ADFA-5510 wired help to everything inside the carousel and missed the frame around it -- including the dock control, which is the only way back and so the one that most needs explaining. It now installs the same ChromeControlTooltips.handler the editor and plugin tabs do. helpTag is abstract. A page with no help is a page nobody wrote copy for, which should be a compile error rather than a silently unhelpful long press; every page already overrides it. One predicate decides what is on the axis band. The tap that opens the sampling-rate chooser and the long press that explains it have to agree on where that band is; written twice, they could drift apart and the tooltip would then describe a control the tap no longer reaches. clearLongPressHelp() is now one call, because setOnLongClickListener(null) leaves isLongClickable set and the view goes on claiming long presses for help it no longer offers. EditorBottomSheet had the same bug at six teardown sites and is swept too. Tests: four classes had grown their own measure/layout/draw helper, with the comment explaining why the draw matters in three of them and the draw itself missing from one. One layOutAndDraw() now, shared. The help test also built a controller per test case and never released any of them -- each one installs itself as the listener on three watchers -- so it ran against a growing pile of live carousels; they are tracked and unbound. Checked on device rather than by argument: the sampling-rate dialog's Help and Cancel buttons stay separate and legible at font scales 1.0, 1.5 and 2.0 (Help ends at x=479, Cancel starts at x=873 at 2.0), and the nine rate entries still fit without scrolling. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../MetricsCarouselDockableContent.kt | 12 +++++ .../itsaky/androidide/ui/EditorBottomSheet.kt | 13 ++--- .../ui/MetricsCarouselController.kt | 8 +-- .../androidide/ui/MetricsChartRenderer.kt | 23 +++++--- .../utils/LongPressHelpExtensions.kt | 34 ++++++++++++ .../com/itsaky/androidide/ui/ChartLayout.kt | 52 +++++++++++++++++++ .../ui/MemoryUsageChartRendererTest.kt | 16 +----- .../ui/MetricsAnnotationRenderingTest.kt | 3 ++ .../androidide/ui/MetricsCarouselHelpTest.kt | 30 +++++++---- .../androidide/ui/MetricsChartAxisTapTest.kt | 11 +--- .../ui/NetworkUsageChartRendererTest.kt | 19 +------ .../ui/PowerUsageChartRendererTest.kt | 12 +---- 12 files changed, 152 insertions(+), 81 deletions(-) create mode 100644 app/src/main/java/com/itsaky/androidide/utils/LongPressHelpExtensions.kt create mode 100644 app/src/test/java/com/itsaky/androidide/ui/ChartLayout.kt diff --git a/app/src/main/java/com/itsaky/androidide/editor/floating/MetricsCarouselDockableContent.kt b/app/src/main/java/com/itsaky/androidide/editor/floating/MetricsCarouselDockableContent.kt index e35c79b40b..294fb33857 100644 --- a/app/src/main/java/com/itsaky/androidide/editor/floating/MetricsCarouselDockableContent.kt +++ b/app/src/main/java/com/itsaky/androidide/editor/floating/MetricsCarouselDockableContent.kt @@ -23,6 +23,7 @@ import android.view.View import android.view.ViewGroup import android.view.inputmethod.InputMethodManager import com.itsaky.androidide.databinding.LayoutMemUsageBinding +import com.itsaky.androidide.floating.model.ChromeControl import com.itsaky.androidide.floating.model.DockableContent import com.itsaky.androidide.floating.window.FloatingWindowHost import com.itsaky.androidide.ui.MetricsCarouselController @@ -49,6 +50,17 @@ class MetricsCarouselDockableContent( ) : DockableContent { override val id: String = ID + /** + * The window chrome's own help, the same handler the editor and plugin tabs install. + * + * Without it the undocked carousel was the one floating window whose minimize, maximize and + * dock controls answered no long press -- and the dock control is the only way back, so it is + * the one that most needs explaining. ADFA-5510 wired help to everything inside the carousel + * and missed the frame around it. + */ + override val onChromeControlLongPress: (ChromeControl, View) -> Unit = + ChromeControlTooltips.handler + override fun onCreateView( context: Context, host: FloatingWindowHost, diff --git a/app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt b/app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt index 9570333eb2..7f464dbd2f 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt @@ -67,6 +67,7 @@ import com.itsaky.androidide.tasks.runOnUiThread 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.dpToPx import com.itsaky.androidide.utils.flashError import com.itsaky.androidide.utils.flashSuccess @@ -332,17 +333,17 @@ class EditorBottomSheet binding.tabs.clearOnTabSelectedListeners() binding.shareOutputAction.setOnClickListener(null) - binding.shareOutputAction.setOnLongClickListener(null) + binding.shareOutputAction.clearLongPressHelp() binding.clearOutputAction.setOnClickListener(null) - binding.clearOutputAction.setOnLongClickListener(null) + binding.clearOutputAction.clearLongPressHelp() binding.searchOutputAction.setOnClickListener(null) - binding.searchOutputAction.setOnLongClickListener(null) + binding.searchOutputAction.clearLongPressHelp() binding.filterOutputAction.setOnClickListener(null) - binding.filterOutputAction.setOnLongClickListener(null) + binding.filterOutputAction.clearLongPressHelp() binding.wordWrapOutputAction.setOnClickListener(null) - binding.wordWrapOutputAction.setOnLongClickListener(null) + binding.wordWrapOutputAction.clearLongPressHelp() binding.viewOptionsOutputAction.setOnClickListener(null) - binding.viewOptionsOutputAction.setOnLongClickListener(null) + binding.viewOptionsOutputAction.clearLongPressHelp() binding.copyDiagnosticsFab.setOnClickListener(null) binding.headerContainer.setOnClickListener(null) removeOnLayoutChangeListener(fabLayoutChangeListener) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt index 785bdc141f..5e6854c50c 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt @@ -43,6 +43,7 @@ import com.itsaky.androidide.utils.MetricsSamplingRates import com.itsaky.androidide.utils.MetricsSnapshot import com.itsaky.androidide.utils.NetworkUsageWatcher import com.itsaky.androidide.utils.PowerUsageWatcher +import com.itsaky.androidide.utils.clearLongPressHelp import com.itsaky.androidide.utils.displayTooltipOnLongPress import com.itsaky.androidide.utils.showIdeCategoryTooltipIfPresent import kotlinx.coroutines.CancellationException @@ -279,12 +280,7 @@ class MetricsCarouselController( // reach with no help at all. .filterNot { (view, _) -> view === bound.metricsUndockedMessage } .map { (view, _) -> view } - .forEach { control -> - control.setOnLongClickListener(null) - // setOnLongClickListener(null) leaves isLongClickable set, so the view would still - // claim a long press it no longer answers. - control.isLongClickable = false - } + .forEach(View::clearLongPressHelp) } binding?.metricsPrevious?.setOnClickListener(null) binding?.metricsNext?.setOnClickListener(null) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt index 15085b3b2e..a5da502bcc 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt @@ -76,7 +76,7 @@ abstract class MetricsChartRenderer( * never calls `super`, so the framework's long-press detection never runs and a view listener * would be installed, look wired, and never fire. */ - protected open val helpTag: String? = null + protected abstract val helpTag: String /** * The help tag for a long press at [y], or `null` if this page has none. @@ -86,14 +86,21 @@ abstract class MetricsChartRenderer( */ @VisibleForTesting internal fun helpTagAt(y: Float): String? { - val chart = this.chart ?: return null // The axis band answers for the sampling rate, the plot for the metric itself, matching // where a tap goes. - return if (y >= chart.viewPortHandler.contentBottom()) { - TooltipTag.CAROUSEL_AXIS_TIME - } else { - helpTag - } + return if (isOnAxisBand(y)) TooltipTag.CAROUSEL_AXIS_TIME else helpTag + } + + /** + * Whether [y] landed on the x axis band rather than in the plot. + * + * One predicate, because the tap that opens the sampling-rate chooser and the long press that + * explains it have to agree on where that band is: written twice, they can drift apart and the + * tooltip then describes a control the tap no longer reaches. + */ + private fun isOnAxisBand(y: Float): Boolean { + val chart = this.chart ?: return false + return y >= chart.viewPortHandler.contentBottom() } /** @@ -284,7 +291,7 @@ abstract class MetricsChartRenderer( ) : OnChartGestureListener { override fun onChartSingleTapped(me: MotionEvent?) { val y = me?.y ?: return - if (y >= chart.viewPortHandler.contentBottom()) { + if (isOnAxisBand(y)) { onXAxisTap?.invoke() } } diff --git a/app/src/main/java/com/itsaky/androidide/utils/LongPressHelpExtensions.kt b/app/src/main/java/com/itsaky/androidide/utils/LongPressHelpExtensions.kt new file mode 100644 index 0000000000..43e3b76b00 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/utils/LongPressHelpExtensions.kt @@ -0,0 +1,34 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.utils + +import android.view.View + +/** + * Stops this view answering a long press. + * + * `setOnLongClickListener(null)` alone is not enough: [View.setOnLongClickListener] sets + * `isLongClickable` when it installs a listener but does not unset it when the listener is + * removed, so the view goes on consuming long presses -- and showing the system's own + * "performLongClick" feedback -- for help it no longer offers. Every teardown that clears a + * long-press help listener wants both halves, so it is one call. + */ +fun View.clearLongPressHelp() { + setOnLongClickListener(null) + isLongClickable = false +} diff --git a/app/src/test/java/com/itsaky/androidide/ui/ChartLayout.kt b/app/src/test/java/com/itsaky/androidide/ui/ChartLayout.kt new file mode 100644 index 0000000000..4bc1d534c8 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/ui/ChartLayout.kt @@ -0,0 +1,52 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.ui + +import android.graphics.Bitmap +import android.graphics.Canvas +import android.view.View + +/** The plot size every metrics chart test lays out at; roughly the carousel strip on a phone. */ +const val CHART_WIDTH = 720 + +const val CHART_HEIGHT = 400 + +/** + * Lays this chart out and draws it once, which is what every assertion about its viewport needs. + * + * Two separate reasons, both easy to leave out and neither of which fails loudly. Without the + * layout the plot area has no extent, so every coordinate lands on its edge and a hit test cannot + * tell inside from outside. Without the draw the scroll to the newest samples has not run -- + * MPAndroidChart queues `moveViewToX` as a job that only executes during a draw pass -- so the + * chart still reports the *oldest* samples as visible and a window assertion reads the wrong end + * of the buffer. + * + * Four test classes had grown their own copy of this, with the comment explaining it in three of + * them and the draw missing from one. + */ +fun SafeLineChart.layOutAndDraw( + width: Int = CHART_WIDTH, + height: Int = CHART_HEIGHT, +) { + measure( + View.MeasureSpec.makeMeasureSpec(width, View.MeasureSpec.EXACTLY), + View.MeasureSpec.makeMeasureSpec(height, View.MeasureSpec.EXACTLY), + ) + layout(0, 0, width, height) + draw(Canvas(Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888))) +} diff --git a/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt index 031c59167c..9140466a8f 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt @@ -49,12 +49,7 @@ class MemoryUsageChartRendererTest { lineColorFor = { Color.BLUE }, ) - /** - * A chart showing one process with the given byte history, laid out and drawn once. - * - * The draw matters: MPAndroidChart queues the scroll to the newest samples as a job that only - * runs during a draw pass, so without one the chart reports the oldest samples as visible. - */ + /** A chart showing one process with the given byte history, laid out and drawn once. */ private fun laidOutChart(history: LongArray): SafeLineChart { val chart = chart() val process = @@ -64,12 +59,7 @@ class MemoryUsageChartRendererTest { MutableShiftedLongArray(LongArray(history.size) { history[it] }), ) renderer { arrayOf(process) }.attach(chart) - chart.measure( - View.MeasureSpec.makeMeasureSpec(WIDTH, View.MeasureSpec.EXACTLY), - View.MeasureSpec.makeMeasureSpec(HEIGHT, View.MeasureSpec.EXACTLY), - ) - chart.layout(0, 0, WIDTH, HEIGHT) - chart.draw(Canvas(Bitmap.createBitmap(WIDTH, HEIGHT, Bitmap.Config.ARGB_8888))) + chart.layOutAndDraw() return chart } @@ -211,8 +201,6 @@ class MemoryUsageChartRendererTest { private companion object { const val BYTES_PER_MB = 1024L * 1024L - const val WIDTH = 720 - const val HEIGHT = 400 const val PID_IDE = 1 /** Longer than the visible window, so the start of the history scrolls off screen. */ diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsAnnotationRenderingTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsAnnotationRenderingTest.kt index 98ccf47ea7..c752cf9ece 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MetricsAnnotationRenderingTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsAnnotationRenderingTest.kt @@ -22,6 +22,7 @@ import androidx.test.core.app.ApplicationProvider import com.github.mikephil.charting.data.Entry import com.github.mikephil.charting.data.LineDataSet import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.utils.MetricsAnnotationStore import org.junit.Test import org.junit.runner.RunWith @@ -47,6 +48,8 @@ class MetricsAnnotationRenderingTest { annotations = annotations, nowMillis = now, ) { + override val helpTag: String = TooltipTag.CAROUSEL_CHART_MEMORY + override fun rebuild() { val chart = this.chart ?: return val entries = List(sampleCount) { Entry(it.toFloat(), 0f) } diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt index ed9ec75c93..177c1e4651 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt @@ -30,6 +30,7 @@ import com.itsaky.androidide.utils.MemoryUsageWatcher import com.itsaky.androidide.utils.MetricsAnnotationStore import com.itsaky.androidide.utils.NetworkUsageWatcher import com.itsaky.androidide.utils.PowerUsageWatcher +import org.junit.After import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -49,13 +50,30 @@ class MetricsCarouselHelpTest { R.style.Theme_AndroidIDE, ) + /** + * Every controller this test builds, so [tearDown] can release them. + * + * Each one installs itself as the listener on three watchers; a controller left bound holds + * its views and goes on being fed for the rest of the JVM's life, and the tests then run + * against a growing pile of live carousels. + */ + private val controllers = mutableListOf() + + @After + fun tearDown() { + controllers.forEach { it.unbind() } + controllers.clear() + } + private fun boundStrip(): LayoutMemUsageBinding { val binding = LayoutMemUsageBinding.inflate(LayoutInflater.from(context)) controller().bind(binding) return binding } - private fun controller() = + private fun controller() = newController().also(controllers::add) + + private fun newController() = MetricsCarouselController( memoryUsageWatcher = MemoryUsageWatcher(), networkUsageWatcher = NetworkUsageWatcher(uid = TEST_UID), @@ -154,15 +172,11 @@ class MetricsCarouselHelpTest { }, ) renderer.attach(chart) - chart.measure( - View.MeasureSpec.makeMeasureSpec(WIDTH, View.MeasureSpec.EXACTLY), - View.MeasureSpec.makeMeasureSpec(HEIGHT, View.MeasureSpec.EXACTLY), - ) - chart.layout(0, 0, WIDTH, HEIGHT) + chart.layOutAndDraw() val handler = chart.viewPortHandler // Guards the two assertions below: on an unlaid-out chart both points land on one edge. - assertThat(handler.contentBottom()).isLessThan(HEIGHT.toFloat()) + assertThat(handler.contentBottom()).isLessThan(CHART_HEIGHT.toFloat()) // Below the plot is the time axis, which is what the sampling rate belongs to. assertThat(renderer.helpTagAt(handler.contentBottom() + 1f)).isEqualTo(TooltipTag.CAROUSEL_AXIS_TIME) @@ -173,8 +187,6 @@ class MetricsCarouselHelpTest { private companion object { const val TEST_UID = 10_123 - const val WIDTH = 720 - const val HEIGHT = 400 const val SAMPLES = 60 } } diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsChartAxisTapTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsChartAxisTapTest.kt index 7e6a9a7a82..8d752e17c5 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MetricsChartAxisTapTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsChartAxisTapTest.kt @@ -58,12 +58,7 @@ class MetricsChartAxisTapTest { renderer.attach(chart) renderer.onXAxisTap = { taps++ } - // Without a layout pass the plot area has no extent, so every coordinate is on its edge. - chart.measure( - View.MeasureSpec.makeMeasureSpec(WIDTH, View.MeasureSpec.EXACTLY), - View.MeasureSpec.makeMeasureSpec(HEIGHT, View.MeasureSpec.EXACTLY), - ) - chart.layout(0, 0, WIDTH, HEIGHT) + chart.layOutAndDraw() return chart } @@ -82,7 +77,7 @@ class MetricsChartAxisTapTest { // Guards the other tests: on an unlaid-out chart they would all tap the same edge. assertThat(chart.viewPortHandler.contentBottom()).isGreaterThan(chart.viewPortHandler.contentTop()) - assertThat(chart.viewPortHandler.contentBottom()).isLessThan(HEIGHT.toFloat()) + assertThat(chart.viewPortHandler.contentBottom()).isLessThan(CHART_HEIGHT.toFloat()) } @Test @@ -115,8 +110,6 @@ class MetricsChartAxisTapTest { } private companion object { - const val WIDTH = 720 - const val HEIGHT = 400 const val SAMPLES = 60 } } diff --git a/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt b/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt index 85cd37298c..699b8c64da 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt @@ -38,9 +38,6 @@ import kotlin.math.log10 @RunWith(RobolectricTestRunner::class) class NetworkUsageChartRendererTest { private companion object { - const val WIDTH = 720 - const val HEIGHT = 400 - /** Longer than the visible window, so the start of the history scrolls off screen. */ const val SAMPLE_COUNT = 200 } @@ -98,21 +95,7 @@ class NetworkUsageChartRendererTest { assertThat(ys[2] - ys[1]).isLessThan(4f) } - /** - * Lays the chart out and draws it once. - * - * The draw is not decoration: MPAndroidChart queues the scroll to the newest samples as a job - * that only runs during a draw pass, so without one the chart still reports the *oldest* - * samples as visible and every assertion here would read the wrong window. - */ - private fun laidOut(chart: SafeLineChart) { - chart.measure( - View.MeasureSpec.makeMeasureSpec(WIDTH, View.MeasureSpec.EXACTLY), - View.MeasureSpec.makeMeasureSpec(HEIGHT, View.MeasureSpec.EXACTLY), - ) - chart.layout(0, 0, WIDTH, HEIGHT) - chart.draw(Canvas(Bitmap.createBitmap(WIDTH, HEIGHT, Bitmap.Config.ARGB_8888))) - } + private fun laidOut(chart: SafeLineChart) = chart.layOutAndDraw() @Test fun `the axis is scaled to what is on screen, not to the whole buffer`() { diff --git a/app/src/test/java/com/itsaky/androidide/ui/PowerUsageChartRendererTest.kt b/app/src/test/java/com/itsaky/androidide/ui/PowerUsageChartRendererTest.kt index 30209b5f1a..4cfb5770f5 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/PowerUsageChartRendererTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/PowerUsageChartRendererTest.kt @@ -304,15 +304,7 @@ class PowerUsageChartRendererTest { assertThat(renderer.batteryReadout()).isNull() } - private fun laidOut(chart: SafeLineChart) { - chart.measure( - View.MeasureSpec.makeMeasureSpec(WIDTH, View.MeasureSpec.EXACTLY), - View.MeasureSpec.makeMeasureSpec(HEIGHT, View.MeasureSpec.EXACTLY), - ) - chart.layout(0, 0, WIDTH, HEIGHT) - // The scroll to the newest samples is a job that only runs during a draw pass. - chart.draw(Canvas(Bitmap.createBitmap(WIDTH, HEIGHT, Bitmap.Config.ARGB_8888))) - } + private fun laidOut(chart: SafeLineChart) = chart.layOutAndDraw() @Test fun `the power axis starts at zero, never below it`() { @@ -353,8 +345,6 @@ class PowerUsageChartRendererTest { } private companion object { - const val WIDTH = 720 - const val HEIGHT = 400 const val SAMPLES = 200 const val OPAQUE = 0xFF000000.toInt() From 90ee07448a193ba992b9b1b3be4ba01d5c42d7b2 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 11:28:18 -0700 Subject: [PATCH 5/9] ADFA-5510: let the end arrows answer a long press, without losing their state A disabled View still consumes a touch and then drops it without calling any listener, so a long press on the arrow at either end of the carousel showed no tooltip -- and that is the arrow whose greying-out a user is likeliest to ask about. Same shape as the isLongClickable bug: the listener was installed, looked wired, and never fired. isClickable is the narrower statement and the true one. The arrow does not answer a tap; it does answer a long press. step() clamps anyway, so a tap on a dimmed arrow was already a no-op, and isEnabled was buying nothing but the swallowed help. Dropping isEnabled outright would have cost more than it gained, though: it is what a screen reader reads to announce a control as unavailable, and alpha is invisible to accessibility services, so the state would have disappeared for exactly the users who cannot see the dimming. An AccessibilityDelegateCompat reports the node as disabled and non-clickable instead, reading View.isClickable rather than holding its own copy of the state. Tested both halves: what a touch sees, and what createAccessibilityNodeInfo hands a screen reader. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../ui/MetricsCarouselController.kt | 58 +++++++++++++++++-- .../androidide/ui/MetricsCarouselHelpTest.kt | 42 ++++++++++++++ 2 files changed, 95 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt index b74d495321..079391b036 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt @@ -28,6 +28,9 @@ import android.widget.Toast import androidx.annotation.UiThread import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog +import androidx.core.view.AccessibilityDelegateCompat +import androidx.core.view.ViewCompat +import androidx.core.view.accessibility.AccessibilityNodeInfoCompat import androidx.core.view.isVisible import androidx.viewpager2.widget.ViewPager2 import com.itsaky.androidide.app.configuration.IDEBuildConfigProvider @@ -201,6 +204,9 @@ class MetricsCarouselController( // annoying. binding.metricsPrevious.setOnClickListener { step(-1) } binding.metricsNext.setOnClickListener { step(1) } + // After the click listeners, which set isClickable themselves. + ViewCompat.setAccessibilityDelegate(binding.metricsPrevious, arrowAccessibilityDelegate) + ViewCompat.setAccessibilityDelegate(binding.metricsNext, arrowAccessibilityDelegate) updateArrows(binding.metricsPager.currentItem) wireHelp(binding) @@ -284,6 +290,8 @@ class MetricsCarouselController( } binding?.metricsPrevious?.setOnClickListener(null) binding?.metricsNext?.setOnClickListener(null) + binding?.metricsPrevious?.let { ViewCompat.setAccessibilityDelegate(it, null) } + binding?.metricsNext?.let { ViewCompat.setAccessibilityDelegate(it, null) } pageCallback?.let { binding?.metricsPager?.unregisterOnPageChangeCallback(it) } pageCallback = null @@ -312,10 +320,30 @@ class MetricsCarouselController( @UiThread private fun updateArrows(position: Int) { val binding = this.binding ?: return - binding.metricsPrevious.isEnabled = position > 0 - binding.metricsNext.isEnabled = position < pages.lastIndex - binding.metricsPrevious.alpha = if (position > 0) 1f else DISABLED_ARROW_ALPHA - binding.metricsNext.alpha = if (position < pages.lastIndex) 1f else DISABLED_ARROW_ALPHA + setPagingAvailable(binding.metricsPrevious, available = position > 0) + setPagingAvailable(binding.metricsNext, available = position < pages.lastIndex) + } + + /** + * Marks an arrow as leading somewhere, or not. + * + * Deliberately not `isEnabled`. A disabled View still consumes a touch and then drops it + * without calling any listener, so a long press on the arrow at either end of the carousel + * showed no tooltip -- and that is the arrow whose greying-out a user is likeliest to ask + * about. [isClickable] is the narrower statement and the true one: the arrow does not answer a + * tap, but it does answer a long press. [step] clamps anyway, so a tap on a dimmed arrow was + * already a no-op. + * + * Alpha alone would have lost the state for anyone who cannot see it, since a screen reader + * reads a node's flags rather than its opacity. [arrowAccessibilityDelegate] puts it back. + */ + @UiThread + private fun setPagingAvailable( + arrow: View, + available: Boolean, + ) { + arrow.alpha = if (available) 1f else DIMMED_ARROW_ALPHA + arrow.isClickable = available } /** @@ -571,7 +599,27 @@ class MetricsCarouselController( else -> null } - const val DISABLED_ARROW_ALPHA = 0.35f + const val DIMMED_ARROW_ALPHA = 0.35f + + /** + * Reports an arrow that leads nowhere as disabled, and as offering no tap. + * + * The views stay touch-enabled so they can still answer a long press with their tooltip + * (see [setPagingAvailable]); without this, TalkBack would offer "double-tap to activate" + * on an arrow that does nothing, and give no hint that the carousel has an end. Reads + * [View.isClickable] rather than holding its own copy, so there is one source of truth. + */ + val arrowAccessibilityDelegate = + object : AccessibilityDelegateCompat() { + override fun onInitializeAccessibilityNodeInfo( + host: View, + info: AccessibilityNodeInfoCompat, + ) { + super.onInitializeAccessibilityNodeInfo(host, info) + info.isEnabled = host.isClickable + info.isClickable = host.isClickable + } + } /** Dims a rate this device cannot offer, so the list shows what the hardware costs. */ const val UNAVAILABLE_RATE_ALPHA = 0.4f diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt index 177c1e4651..e2bf17635c 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselHelpTest.kt @@ -21,6 +21,7 @@ import android.content.Context import android.view.LayoutInflater import android.view.View import androidx.appcompat.view.ContextThemeWrapper +import androidx.core.view.accessibility.AccessibilityNodeInfoCompat import androidx.test.core.app.ApplicationProvider import com.google.common.truth.Truth.assertThat import com.itsaky.androidide.R @@ -111,6 +112,47 @@ class MetricsCarouselHelpTest { assertThat(unwired).isEmpty() } + @Test + fun `the arrow at the end of the carousel is dimmed but still answers a long press`() { + val binding = boundStrip() + + // On the first page there is nowhere to go back to. Disabling that arrow would leave it + // consuming the long press and dropping it, so the one arrow whose greyed-out state a + // user is likeliest to ask about was the one with no answer. + assertThat(binding.metricsPager.currentItem).isEqualTo(0) + assertThat(binding.metricsPrevious.alpha).isLessThan(1f) + assertThat(binding.metricsPrevious.isEnabled).isTrue() + assertThat(binding.metricsPrevious.isLongClickable).isTrue() + // It answers no tap, though: that is the narrower and the true statement. + assertThat(binding.metricsPrevious.isClickable).isFalse() + + // ...and the other end is at full strength, so the dimming means something. + assertThat(binding.metricsNext.alpha).isEqualTo(1f) + assertThat(binding.metricsNext.isClickable).isTrue() + } + + @Test + fun `a dimmed arrow still reads as disabled to a screen reader`() { + val binding = boundStrip() + + // Alpha is invisible to accessibility services, so dropping isEnabled would have taken + // the state away from exactly the users who cannot see the dimming. + val previous = nodeInfoFor(binding.metricsPrevious) + assertThat(previous.isEnabled).isFalse() + assertThat(previous.isClickable).isFalse() + + val next = nodeInfoFor(binding.metricsNext) + assertThat(next.isEnabled).isTrue() + assertThat(next.isClickable).isTrue() + } + + /** What a screen reader would be handed for [view]. */ + private fun nodeInfoFor(view: View): AccessibilityNodeInfoCompat { + val info = view.createAccessibilityNodeInfo() + assertThat(info).isNotNull() + return AccessibilityNodeInfoCompat.wrap(info!!) + } + @Test fun `an unbound strip has no help wired`() { // Guards the test above: if inflation alone made these long-clickable, it would pass From fcff821f74175447b6355795ab3221004e999355 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 12:30:35 -0700 Subject: [PATCH 6/9] ADFA-5510: give the new adapter test's renderer a help tag helpTag became abstract on this branch, so the test renderer that arrived with ADFA-5499's adapter refactor does not compile here without one. The merge is the first place the two changes meet. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../com/itsaky/androidide/ui/MetricsCarouselAdapterTest.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselAdapterTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselAdapterTest.kt index b2cb9f205e..148507adc4 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselAdapterTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselAdapterTest.kt @@ -25,6 +25,7 @@ import com.github.mikephil.charting.data.Entry import com.github.mikephil.charting.data.LineDataSet import com.google.common.truth.Truth.assertThat import com.itsaky.androidide.R +import com.itsaky.androidide.idetooltips.TooltipTag import com.itsaky.androidide.resources.R.string import org.junit.Test import org.junit.runner.RunWith @@ -49,6 +50,8 @@ class MetricsCarouselAdapterTest { private class TestRenderer( private val readout: String? = null, ) : MetricsChartRenderer(sampleIntervalMillis = { 1_000L }) { + override val helpTag: String = TooltipTag.CAROUSEL_CHART_MEMORY + val attached = mutableListOf() override fun rebuild() { From f9b39e28add2d4373734400e569fa2bbe670f483 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 14:55:21 -0700 Subject: [PATCH 7/9] ADFA-5510: give the window's close control a tag of its own Long-pressing the X on a floating window answered "Opens the file in a separate window" -- the opposite action, and the wrong noun when the window holds the metrics carousel. The close control had no tag, so it borrowed WINDOW_UNDOCK, which is not spare: two editor controls use it for a real undock, where that copy is correct. So this was a mismap rather than bad copy, and the fix is a tag of its own. Found from a screenshot of the undocked carousel, and reachable from the carousel at all because ADFA-5510 wired its chrome to the shared handler. It was equally wrong on editor and plugin windows before that. Verified on device: the X now reads "Closes the floating window.", and its See More gives the detail. Checked the dock control alongside it, so the two neighbouring controls no longer describe the same action. Also pins in the rebind test that dimming an end arrow changes its alpha and not its tint. The two are orthogonal -- the tint is the colour the glyph is drawn in, the dimming is alpha over it -- which is why asserting the arrows share a tint does not contradict their looking different at the ends of the carousel. A later change that dimmed through a state-aware ColorStateList would break that, and now it would be caught. The copy lives in documentation.db, which this repository cannot edit; it has been written into the local database for testing, and the four window-* tags need it in the content release. Recorded on ADFA-5513. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../editor/floating/ChromeControlTooltips.kt | 3 +-- .../ui/MetricsCarouselRebindTest.kt | 19 +++++++++++++++++++ .../androidide/idetooltips/TooltipTag.kt | 9 +++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/editor/floating/ChromeControlTooltips.kt b/app/src/main/java/com/itsaky/androidide/editor/floating/ChromeControlTooltips.kt index 20121ec1db..43a32c64c8 100644 --- a/app/src/main/java/com/itsaky/androidide/editor/floating/ChromeControlTooltips.kt +++ b/app/src/main/java/com/itsaky/androidide/editor/floating/ChromeControlTooltips.kt @@ -7,7 +7,6 @@ import com.itsaky.androidide.idetooltips.TooltipCategory import com.itsaky.androidide.idetooltips.TooltipManager import com.itsaky.androidide.idetooltips.TooltipTag - object ChromeControlTooltips { val handler: (ChromeControl, View) -> Unit = { control, anchor -> tagFor(control)?.let { tag -> @@ -20,6 +19,6 @@ object ChromeControlTooltips { ChromeControl.MINIMIZE -> TooltipTag.WINDOW_MINIMIZE ChromeControl.MAXIMIZE -> TooltipTag.WINDOW_MAXIMIZE ChromeControl.DOCK -> TooltipTag.WINDOW_DOCK - ChromeControl.CLOSE -> TooltipTag.WINDOW_UNDOCK + ChromeControl.CLOSE -> TooltipTag.WINDOW_CLOSE } } diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselRebindTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselRebindTest.kt index 8546083abb..66df690e00 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselRebindTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsCarouselRebindTest.kt @@ -123,6 +123,25 @@ class MetricsCarouselRebindTest { assertThat(previous.defaultColor).isEqualTo(next.defaultColor) } + @Test + fun `dimming an end arrow changes its alpha, not its tint`() { + val binding = strip() + controller().bind(binding) + + // The two are orthogonal, which is why asserting the arrows share a tint does not + // contradict their looking different at the ends of the carousel: the tint is the colour + // the glyph is drawn in, and the dimming is alpha over the top of it. A later change that + // dimmed through a state-aware ColorStateList instead would break that, and this says so. + assertThat(binding.metricsPager.currentItem).isEqualTo(0) + assertThat(binding.metricsPrevious.alpha).isLessThan(binding.metricsNext.alpha) + + val previous = ImageViewCompat.getImageTintList(binding.metricsPrevious)!! + val next = ImageViewCompat.getImageTintList(binding.metricsNext)!! + assertThat(previous.defaultColor).isEqualTo(next.defaultColor) + // One colour, no per-state variation: nothing here depends on the enabled state. + assertThat(previous.isStateful).isFalse() + } + @Test fun `the arrow tint is the colour the title uses`() { val binding = strip() diff --git a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt index 64d2025174..5086eb33e1 100644 --- a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt +++ b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt @@ -235,6 +235,15 @@ object TooltipTag { const val WINDOW_DOCK = "window-dock" const val WINDOW_UNDOCK = "window-undock" + /** + * The floating window's close control. + * + * Distinct from [WINDOW_UNDOCK], which names the opposite action and belongs to the editor + * controls that open something in a window. The chrome's close button borrowed that tag, so a + * long press on it answered "Opens the file in a separate window". + */ + const val WINDOW_CLOSE = "window-close" + // Delete project const val DELETE_PROJECT = "project.delete" const val DELETE_PROJECT_SELECT = "project.delete.select" From 498f3d4f7858be090f9f200602d12b368e9be428 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 17:50:10 -0700 Subject: [PATCH 8/9] ADFA-5510: sweep the last setOnLongClickListener(null) site TooltipMaterialCheckBox clears its tooltip listener the same way everything else did before this ticket: setOnLongClickListener(null) without unsetting isLongClickable, so it goes on consuming long presses for a tooltip it no longer offers. Missed by my own sweep, and for a dull reason: I grepped *.kt, and this one is Java in the resources module. It cannot use the Kotlin extension from there, so it does both halves inline. Also records why the chart's long-press help takes the default haptic feedback while every view-based site passes false. Those rely on View.performLongClick buzzing for them; BarLineChartBase.onTouchEvent never calls super, so the framework's long press never runs on a chart and the manual feedback is the only thing there is. The two look inconsistent and are not. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../androidide/ui/MetricsChartRenderer.kt | 4 ++ .../resources/TooltipMaterialCheckBox.java | 60 ++++++++++--------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt index ad09aef9fb..6d72601dba 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt @@ -343,6 +343,10 @@ abstract class MetricsChartRenderer( override fun onChartLongPressed(me: MotionEvent?) { val y = me?.y ?: return 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) } diff --git a/resources/src/main/java/com/itsaky/androidide/resources/TooltipMaterialCheckBox.java b/resources/src/main/java/com/itsaky/androidide/resources/TooltipMaterialCheckBox.java index 49fb9eb6f2..e291a9fc8e 100644 --- a/resources/src/main/java/com/itsaky/androidide/resources/TooltipMaterialCheckBox.java +++ b/resources/src/main/java/com/itsaky/androidide/resources/TooltipMaterialCheckBox.java @@ -9,34 +9,38 @@ import com.google.android.material.checkbox.MaterialCheckBox; /** - * A MaterialCheckBox that implements ITooltipView to provide a unified - * long-press listener for tooltips. + * A MaterialCheckBox that implements ITooltipView to provide a unified long-press listener for tooltips. */ public class TooltipMaterialCheckBox extends MaterialCheckBox implements ITooltipView { - public TooltipMaterialCheckBox(@NonNull Context context) { - super(context); - } - - public TooltipMaterialCheckBox(@NonNull Context context, @Nullable AttributeSet attrs) { - super(context, attrs); - } - - public TooltipMaterialCheckBox(@NonNull Context context, @Nullable AttributeSet attrs, int defStyleAttr) { - super(context, attrs, defStyleAttr); - } - - @Override - public void setTooltipLongPressListener(OnTooltipLongPressListener listener) { - if (listener == null) { - setOnLongClickListener(null); - return; - } - // Bridge our interface listener to the standard Android OnLongClickListener - setOnLongClickListener(v -> { - listener.onLongPress(); - // Return true to consume the event, preventing other actions - return true; - }); - } -} \ No newline at end of file + public TooltipMaterialCheckBox(@NonNull Context context) { + super(context); + } + + public TooltipMaterialCheckBox(@NonNull Context context, @Nullable AttributeSet attrs) { + super(context, attrs); + } + + public TooltipMaterialCheckBox(@NonNull Context context, @Nullable AttributeSet attrs, int defStyleAttr) { + super(context, attrs, defStyleAttr); + } + + @Override + public void setTooltipLongPressListener(OnTooltipLongPressListener listener) { + if (listener == null) { + setOnLongClickListener(null); + // setOnLongClickListener sets isLongClickable when it installs a listener but does not + // unset it when the listener is removed, so without this the checkbox goes on + // consuming long presses -- and showing the platform's long-press feedback -- for a + // tooltip it no longer offers. + setLongClickable(false); + return; + } + // Bridge our interface listener to the standard Android OnLongClickListener + setOnLongClickListener(v -> { + listener.onLongPress(); + // Return true to consume the event, preventing other actions + return true; + }); + } +} From a8225f21afc0053b260f01f5f9866311412b7af1 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 07:27:35 -0700 Subject: [PATCH 9/9] ADFA-5510: keep the legend out of the axis tap band The band was everything below the plot, and MPAndroidChart aligns the legend to the bottom by default -- under the axis labels, inside the band. So tapping the legend, the one part of a chart a reader expects to be tappable, opened the sampling-rate chooser; choosing a rate there clears every buffer, so a mis-tap costs the history being looked at. The band now stops at the legend's top edge, taken from what the chart reserves for it, and is never narrower than one axis label -- otherwise a legend that measured larger than expected could squeeze the rate chooser out of reach entirely. Verified by probe before fixing: a laid-out chart reports legendVerticalAlignment=BOTTOM and legendEnabled=true, with the legend below contentBottom(). Two tests, one per side of the bound; the legend one fails against the unbounded band. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../androidide/ui/MetricsChartRenderer.kt | 17 ++++++++++- .../androidide/ui/MetricsChartAxisTapTest.kt | 28 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt index aaf31d5fc8..75d8bf21eb 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt @@ -97,10 +97,25 @@ abstract class MetricsChartRenderer( * One predicate, because the tap that opens the sampling-rate chooser and the long press that * explains it have to agree on where that band is: written twice, they can drift apart and the * tooltip then describes a control the tap no longer reaches. + * + * Bounded below, not just above. Everything under the plot used to count, and the legend lives + * there too -- MPAndroidChart aligns it to the bottom by default, under the axis labels. So + * tapping the legend, which is the one thing in a chart a reader expects to be tappable, opened + * the sampling-rate chooser; picking a rate there clears every buffer, and the user loses the + * history they were looking at for an action they did not ask for. + * + * The band stops at the legend's top edge, and is never narrower than one axis label, so a + * legend that measures larger than expected cannot squeeze the rate chooser out of reach. */ private fun isOnAxisBand(y: Float): Boolean { val chart = this.chart ?: return false - return y >= chart.viewPortHandler.contentBottom() + val top = chart.viewPortHandler.contentBottom() + val legend = chart.legend + // What the chart reserves for the legend at the bottom: its measured height plus the + // offset it keeps above itself. Both are pixels, as MPAndroidChart stores them. + val reservedForLegend = if (legend.isEnabled) legend.mNeededHeight + legend.yOffset else 0f + val bottom = maxOf(chart.height - reservedForLegend, top + chart.xAxis.textSize) + return y >= top && y < bottom } /** diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsChartAxisTapTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsChartAxisTapTest.kt index 8653db671e..6cfdfa6ed6 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MetricsChartAxisTapTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsChartAxisTapTest.kt @@ -128,6 +128,34 @@ class MetricsChartAxisTapTest { assertThat(taps).isEqualTo(1) } + @Test + fun `a tap on the legend does not open the chooser`() { + val chart = laidOutChart() + + // MPAndroidChart aligns the legend to the bottom by default, below the axis labels, so + // "everything under the plot" included it -- and the legend is the one part of a chart a + // reader expects to be tappable. Opening the rate chooser there is bad enough; picking a + // rate in it clears every buffer, so a mis-tap costs the history being looked at. + // + // Robolectric measures no real text, so the legend here is a few pixels rather than the + // ~10dp row a device draws. That is enough: the assertion is about which side of the + // boundary the legend's own rows fall on, and the bottom row is the legend's. + tapAt(chart, CHART_HEIGHT - 1f) + + assertThat(taps).isEqualTo(0) + } + + @Test + fun `the axis labels still open the chooser, with the legend excluded`() { + val chart = laidOutChart() + + // The other half of the bound: narrowing the band must not put the rate chooser out of + // reach. One axis label's height below the plot always stays in it. + tapAt(chart, chart.viewPortHandler.contentBottom() + chart.xAxis.textSize / 2f) + + assertThat(taps).isEqualTo(1) + } + @Test fun `a tap above the plot does not open the chooser`() { val chart = laidOutChart()