From d564988089db54a57f2b0c732a710b6e9da7731b Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Fri, 2 Oct 2026 10:58:26 -0700 Subject: [PATCH 1/2] ADFA-6312: show the Run button's stop icon while Quick Build runs The BUILDING tone drew ic_quick_build_building, a layer-list of an animated-rotate ring and a stop square. On the Samsung A56 the toolbar slot rendered blank for the whole build: the button was there, enabled and announced "Cancel build", but drew no pixels. Bryan's call: drop the spinner and reuse the static ring-and-square the Run button already shows (ic_stop_daemons), so both buttons present a running build the same way. The three building drawables have no other users and are deleted. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01N1o4KqT54Vp2zpakJ54sbU --- .../androidide/actions/build/QuickBuildAction.kt | 7 ++----- .../build/QuickBuildActionPresentationTest.kt | 11 ++++------- quickbuild/docs/manual-qa.md | 2 +- .../main/res/drawable/ic_quick_build_building.xml | 15 --------------- .../res/drawable/ic_quick_build_building_arc.xml | 15 --------------- .../res/drawable/ic_quick_build_building_stop.xml | 13 ------------- 6 files changed, 7 insertions(+), 56 deletions(-) delete mode 100644 resources/src/main/res/drawable/ic_quick_build_building.xml delete mode 100644 resources/src/main/res/drawable/ic_quick_build_building_arc.xml delete mode 100644 resources/src/main/res/drawable/ic_quick_build_building_stop.xml diff --git a/app/src/main/java/com/itsaky/androidide/actions/build/QuickBuildAction.kt b/app/src/main/java/com/itsaky/androidide/actions/build/QuickBuildAction.kt index c0801faf8d..6641537593 100644 --- a/app/src/main/java/com/itsaky/androidide/actions/build/QuickBuildAction.kt +++ b/app/src/main/java/com/itsaky/androidide/actions/build/QuickBuildAction.kt @@ -227,11 +227,8 @@ class QuickBuildAction( when (tone) { QuickBuildTone.READY -> R.drawable.ic_quick_build - // Behaviour 1: a running build shows the STANDARD build's stop button, not a - // variant of the bolt, which reads as "a build is running" to someone who does - // not already know the feature. The stop square spins inside a ring rather than - // sitting still, so the ~90 s a proxy app build takes does not read as a hang. - QuickBuildTone.BUILDING -> R.drawable.ic_quick_build_building + // Behaviour 1: the Run button's own stop icon, so both buttons say "a build is running" the same way. + QuickBuildTone.BUILDING -> R.drawable.ic_stop_daemons // The hollow bolt: still plainly the Quick Build button, but not the filled // "ready and fast" one. A full build during ordinary editing is normal work, diff --git a/app/src/test/java/com/itsaky/androidide/actions/build/QuickBuildActionPresentationTest.kt b/app/src/test/java/com/itsaky/androidide/actions/build/QuickBuildActionPresentationTest.kt index 1f5732e729..f7c2084928 100644 --- a/app/src/test/java/com/itsaky/androidide/actions/build/QuickBuildActionPresentationTest.kt +++ b/app/src/test/java/com/itsaky/androidide/actions/build/QuickBuildActionPresentationTest.kt @@ -12,14 +12,11 @@ import org.junit.Test */ class QuickBuildActionPresentationTest { @Test - fun `a running build shows a spinning stop icon, not a bolt variant`() { - // The stop square AbstractCancellableRunAction swaps in, inside a spinning ring: the - // two buttons still look like they stop the same kind of thing, and the ring answers - // the manual-QA reading of a static icon as a hung app. Any bolt variant here (the - // previous ic_quick_build_outline) fails the spec, because it did not communicate - // "a build is running" to anyone. + fun `a running build shows the Run button's stop icon, not a bolt variant`() { + // The same static ring and square AbstractCancellableRunAction swaps in, so the two buttons + // present a running build identically; any bolt variant fails the spec. assertThat(QuickBuildAction.iconResFor(QuickBuildTone.BUILDING)) - .isEqualTo(R.drawable.ic_quick_build_building) + .isEqualTo(R.drawable.ic_stop_daemons) } @Test diff --git a/quickbuild/docs/manual-qa.md b/quickbuild/docs/manual-qa.md index 00ce9e8db0..14b72f7369 100644 --- a/quickbuild/docs/manual-qa.md +++ b/quickbuild/docs/manual-qa.md @@ -41,7 +41,7 @@ The button is a split button and the session's status display. Every tone has it | Icon | Tone | Session is | | ---------------------------------- | ------------ | ------------------------------------------------------------ | | Solid bolt | READY | No session, or sitting on a successful build | -| Stop square spinning inside a ring | BUILDING | Provisioning, or a build running now. Tapping stops it | +| Stop square inside a ring (the Run button's stop icon) | BUILDING | Provisioning, or a build running now. Tapping stops it | | Hollow bolt | SLOW | The next build cannot take the fast path and will be a full one. Not a failure | | Sync arrows | RECONNECTING | The compile daemon is being respawned. Transient, resolves itself | | Bolt with an exclamation mark | ERROR | A failure to act on - a failed build, or a daemon respawn that did not come back | diff --git a/resources/src/main/res/drawable/ic_quick_build_building.xml b/resources/src/main/res/drawable/ic_quick_build_building.xml deleted file mode 100644 index f14eb3b869..0000000000 --- a/resources/src/main/res/drawable/ic_quick_build_building.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - - diff --git a/resources/src/main/res/drawable/ic_quick_build_building_arc.xml b/resources/src/main/res/drawable/ic_quick_build_building_arc.xml deleted file mode 100644 index 4825d03be9..0000000000 --- a/resources/src/main/res/drawable/ic_quick_build_building_arc.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - - diff --git a/resources/src/main/res/drawable/ic_quick_build_building_stop.xml b/resources/src/main/res/drawable/ic_quick_build_building_stop.xml deleted file mode 100644 index 2b3aac2d7e..0000000000 --- a/resources/src/main/res/drawable/ic_quick_build_building_stop.xml +++ /dev/null @@ -1,13 +0,0 @@ - - - - - From 4fa62a240584765f028da2cdc05233d24f793ba3 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Fri, 2 Oct 2026 23:21:51 -0700 Subject: [PATCH 2/2] ADFA-6312: tint toolbar icons after mutate() so layered icons keep their colour The editor toolbar set the colour filter on the action's icon and only then called mutate(). LayerDrawable.mutate() rebuilds its layers from their constant state, which drops a filter set beforehand, so a layer-list icon drew its raw paint (white on the light toolbar). Tinting before mutate() also wrote onto drawable state shared with other users of the same resource. Mutate first, then tint and dim once, in a small toolbarIcon() helper so the order is pinned by a Robolectric test. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01N1o4KqT54Vp2zpakJ54sbU --- .../editor/EditorHandlerActivity.kt | 12 +-- .../activities/editor/ToolbarIcon.kt | 20 +++++ .../activities/editor/ToolbarIconTest.kt | 78 +++++++++++++++++++ 3 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 app/src/main/java/com/itsaky/androidide/activities/editor/ToolbarIcon.kt create mode 100644 app/src/test/java/com/itsaky/androidide/activities/editor/ToolbarIconTest.kt diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt index 42aecc8a1e..14aec60546 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt @@ -772,16 +772,10 @@ open class EditorHandlerActivity : // when not applicable, instead of the legacy grey-out used by built-in actions. if (action.honorVisibility && !action.visible) return@forEachIndexed - action.icon?.apply { - colorFilter = action.createColorFilter(data) - alpha = if (action.enabled) 255 else 76 - } - content.projectActionsToolbar.addMenuItem( - // This custom toolbar bypasses DefaultActionsRegistry's menu path, so its - // disabled-icon dim (alpha 76 there) must be mirrored here or a disabled - // action renders at full strength while refusing the tap. - icon = action.icon?.mutate()?.apply { alpha = if (action.enabled) 255 else 76 }, + // This toolbar bypasses DefaultActionsRegistry's menu path, so it mirrors that + // path's tint and disabled dim here. + icon = toolbarIcon(action.icon, action.createColorFilter(data), action.enabled), hint = getToolbarContentDescription(action, data), onClick = { if (action.enabled) registry.executeAction(action, data) }, onLongClick = { diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/ToolbarIcon.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/ToolbarIcon.kt new file mode 100644 index 0000000000..cf7e6636c4 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/ToolbarIcon.kt @@ -0,0 +1,20 @@ +package com.itsaky.androidide.activities.editor + +import android.graphics.ColorFilter +import android.graphics.drawable.Drawable + +/** + * Tints and dims a toolbar action's icon, mutating it first. + * + * Order matters: `LayerDrawable.mutate()` rebuilds its layers and drops a filter set before it, + * and tinting an unmutated icon writes onto state shared with every other use of the resource. + */ +internal fun toolbarIcon( + icon: Drawable?, + colorFilter: ColorFilter?, + enabled: Boolean, +): Drawable? = + icon?.mutate()?.apply { + this.colorFilter = colorFilter + alpha = if (enabled) 255 else 76 + } diff --git a/app/src/test/java/com/itsaky/androidide/activities/editor/ToolbarIconTest.kt b/app/src/test/java/com/itsaky/androidide/activities/editor/ToolbarIconTest.kt new file mode 100644 index 0000000000..767327db23 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/activities/editor/ToolbarIconTest.kt @@ -0,0 +1,78 @@ +package com.itsaky.androidide.activities.editor + +import android.graphics.Canvas +import android.graphics.Color +import android.graphics.ColorFilter +import android.graphics.PixelFormat +import android.graphics.PorterDuff +import android.graphics.PorterDuffColorFilter +import android.graphics.drawable.Drawable +import android.graphics.drawable.GradientDrawable +import android.graphics.drawable.LayerDrawable +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * That a toolbar icon is drawn with the theme tint and the disabled dim. + * + * ADFA-6312: a layer-list icon lost its tint because it was mutated after tinting, which rebuilt + * its layers, so it drew its raw white paint on the light toolbar. + */ +@RunWith(RobolectricTestRunner::class) +class ToolbarIconTest { + private val filter = PorterDuffColorFilter(Color.RED, PorterDuff.Mode.SRC_ATOP) + + @Test + fun `every layer of a layered icon keeps the colour filter`() { + val icon = LayerDrawable(arrayOf(FilterLayer(), FilterLayer())) + + val drawn = toolbarIcon(icon, filter, enabled = true) as LayerDrawable + + for (i in 0 until drawn.numberOfLayers) { + assertThat(drawn.getDrawable(i).colorFilter).isSameInstanceAs(filter) + } + } + + /** + * A layer that, like framework drawables, keeps its filter per instance and is rebuilt from + * shared state; Robolectric's legacy Paint never reports a filter back, so ColorDrawable can't. + */ + private class FilterLayer : Drawable() { + private var filter: ColorFilter? = null + + override fun draw(canvas: Canvas) {} + + override fun setAlpha(alpha: Int) {} + + override fun setColorFilter(colorFilter: ColorFilter?) { + filter = colorFilter + } + + override fun getColorFilter(): ColorFilter? = filter + + @Deprecated("Deprecated in Java") + override fun getOpacity(): Int = PixelFormat.TRANSLUCENT + + override fun getConstantState(): ConstantState = State + + private object State : ConstantState() { + override fun newDrawable(): Drawable = FilterLayer() + + override fun getChangingConfigurations(): Int = 0 + } + } + + @Test + fun `a disabled icon is dimmed and an enabled one is not`() { + // GradientDrawable reports alpha as set; ColorDrawable rounds 76 down to 75. + assertThat(toolbarIcon(GradientDrawable(), filter, enabled = false)!!.alpha).isEqualTo(76) + assertThat(toolbarIcon(GradientDrawable(), filter, enabled = true)!!.alpha).isEqualTo(255) + } + + @Test + fun `an action without an icon gets none`() { + assertThat(toolbarIcon(null, filter, enabled = true)).isNull() + } +}