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/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/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/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() + } +} 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 @@ - - - - -