diff --git a/actions/src/main/java/com/itsaky/androidide/actions/SidebarSlotExceededException.kt b/actions/src/main/java/com/itsaky/androidide/actions/SidebarSlotExceededException.kt deleted file mode 100644 index 26cdc3fa32..0000000000 --- a/actions/src/main/java/com/itsaky/androidide/actions/SidebarSlotExceededException.kt +++ /dev/null @@ -1,20 +0,0 @@ - - -package com.itsaky.androidide.actions - -class SidebarSlotExceededException( - requestedSlots: Int, - availableSlots: Int, - pluginId: String? = null -) : RuntimeException( - buildMessage(requestedSlots, availableSlots, pluginId) -) { - companion object { - private fun buildMessage(requested: Int, available: Int, pluginId: String?): String { - val pluginInfo = pluginId?.let { " Plugin '$it'" } ?: "" - return "Sidebar slot limit exceeded.$pluginInfo declared $requested sidebar item(s), " + - "but only $available slot(s) available. " + - "IdeNavigationRailView supports a maximum of ${SidebarSlotManager.MAX_NAVIGATION_RAIL_ITEMS} items." - } - } -} diff --git a/actions/src/main/java/com/itsaky/androidide/actions/SidebarSlotManager.kt b/actions/src/main/java/com/itsaky/androidide/actions/SidebarSlotManager.kt index ab2f6bc468..c4837525ee 100644 --- a/actions/src/main/java/com/itsaky/androidide/actions/SidebarSlotManager.kt +++ b/actions/src/main/java/com/itsaky/androidide/actions/SidebarSlotManager.kt @@ -5,50 +5,36 @@ import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicInteger object SidebarSlotManager { + private val builtInItemCount = AtomicInteger(0) + private val reservedPluginSlots = ConcurrentHashMap() - const val MAX_NAVIGATION_RAIL_ITEMS = 12 + fun setBuiltInItemCount(count: Int) { + require(count >= 0) { "Built-in item count must not be negative" } + builtInItemCount.set(count) + } - private val builtInItemCount = AtomicInteger(0) - private val reservedPluginSlots = ConcurrentHashMap() + fun getBuiltInItemCount(): Int = builtInItemCount.get() - fun setBuiltInItemCount(count: Int) { - require(count in 0..MAX_NAVIGATION_RAIL_ITEMS) { - "Built-in item count must be between 0 and $MAX_NAVIGATION_RAIL_ITEMS" - } - builtInItemCount.set(count) - } + fun getReservedPluginSlotCount(): Int = reservedPluginSlots.values.sum() - fun getBuiltInItemCount(): Int = builtInItemCount.get() + fun getTotalItemCount(): Int = builtInItemCount.get() + getReservedPluginSlotCount() - fun getReservedPluginSlotCount(): Int = reservedPluginSlots.values.sum() + fun getDeclaredSlots(pluginId: String): Int = reservedPluginSlots[pluginId] ?: 0 - fun getTotalItemCount(): Int = builtInItemCount.get() + getReservedPluginSlotCount() + fun reservePluginSlots( + pluginId: String, + count: Int, + ) { + if (count <= 0) return + reservedPluginSlots[pluginId] = count + } - fun getAvailableSlotsForPlugins(): Int = - (MAX_NAVIGATION_RAIL_ITEMS - builtInItemCount.get() - getReservedPluginSlotCount()) - .coerceAtLeast(0) + fun releasePluginSlots(pluginId: String) { + reservedPluginSlots.remove(pluginId) + } - fun canAddPluginItems(count: Int): Boolean = count <= getAvailableSlotsForPlugins() - - fun getDeclaredSlots(pluginId: String): Int = reservedPluginSlots[pluginId] ?: 0 - - @Throws(SidebarSlotExceededException::class) - fun reservePluginSlots(pluginId: String, count: Int) { - if (count <= 0) return - - val available = getAvailableSlotsForPlugins() - if (count > available) { - throw SidebarSlotExceededException(count, available, pluginId) - } - reservedPluginSlots[pluginId] = count - } - - fun releasePluginSlots(pluginId: String) { - reservedPluginSlots.remove(pluginId) - } - - fun reset() { - builtInItemCount.set(0) - reservedPluginSlots.clear() - } + fun reset() { + builtInItemCount.set(0) + reservedPluginSlots.clear() + } } diff --git a/actions/src/test/java/com/itsaky/androidide/actions/SidebarSlotManagerTest.kt b/actions/src/test/java/com/itsaky/androidide/actions/SidebarSlotManagerTest.kt new file mode 100644 index 0000000000..b8c151d309 --- /dev/null +++ b/actions/src/test/java/com/itsaky/androidide/actions/SidebarSlotManagerTest.kt @@ -0,0 +1,20 @@ +package com.itsaky.androidide.actions + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Test + +class SidebarSlotManagerTest { + @After + fun tearDown() = SidebarSlotManager.reset() + + @Test + fun `a plugin can declare more sidebar items than fit beside the built-in ones`() { + SidebarSlotManager.setBuiltInItemCount(7) + + SidebarSlotManager.reservePluginSlots("plugin.a", 14) + + assertThat(SidebarSlotManager.getDeclaredSlots("plugin.a")).isEqualTo(14) + assertThat(SidebarSlotManager.getTotalItemCount()).isEqualTo(21) + } +} diff --git a/app/src/main/java/com/itsaky/androidide/ui/IdeNavigationRailView.kt b/app/src/main/java/com/itsaky/androidide/ui/IdeNavigationRailView.kt index 2e5042d3a8..f10450b885 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/IdeNavigationRailView.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/IdeNavigationRailView.kt @@ -2,56 +2,98 @@ package com.itsaky.androidide.ui import android.content.Context import android.util.AttributeSet +import androidx.core.view.isGone +import androidx.core.view.marginTop import androidx.core.widget.NestedScrollView import com.google.android.material.navigation.NavigationBarMenuView import com.google.android.material.navigationrail.NavigationRailView +import com.itsaky.androidide.R -class IdeNavigationRailView @JvmOverloads constructor( - context: Context, - attrs: AttributeSet? = null, - defStyleAttr: Int = com.google.android.material.R.attr.navigationRailStyle -) : NavigationRailView(context, attrs, defStyleAttr) { - - companion object { - const val MAX_ITEM_COUNT = 12 - } - - override fun getMaxItemCount(): Int = MAX_ITEM_COUNT - - override fun onAttachedToWindow() { - super.onAttachedToWindow() - enableMenuScrolling() - } - - private fun enableMenuScrolling() { - post { - val menuView = (0 until childCount) - .map { getChildAt(it) } - .firstOrNull { it is NavigationBarMenuView } - ?: return@post - - if (menuView.parent is NestedScrollView) return@post - - removeView(menuView) - - val scroll = NestedScrollView(context).apply { - isVerticalScrollBarEnabled = false - addView( - menuView, - LayoutParams( - LayoutParams.WRAP_CONTENT, - LayoutParams.WRAP_CONTENT - ) - ) - } - - addView( - scroll, - LayoutParams( - LayoutParams.WRAP_CONTENT, - LayoutParams.MATCH_PARENT - ) - ) - } - } -} +class IdeNavigationRailView + @JvmOverloads + constructor( + context: Context, + attrs: AttributeSet? = null, + defStyleAttr: Int = com.google.android.material.R.attr.navigationRailStyle, + ) : NavigationRailView(context, attrs, defStyleAttr) { + private val menuMarginTop = resources.getDimensionPixelSize(R.dimen.sidebar_rail_menu_margin_top) + private var menuScroll: NestedScrollView? = null + + override fun getMaxItemCount(): Int = Int.MAX_VALUE + + override fun onAttachedToWindow() { + super.onAttachedToWindow() + enableMenuScrolling() + } + + override fun onMeasure( + widthMeasureSpec: Int, + heightMeasureSpec: Int, + ) { + super.onMeasure(widthMeasureSpec, heightMeasureSpec) + val scroll = menuScroll ?: return + + val menuTop = menuTop() + (scroll.layoutParams as LayoutParams).topMargin = menuTop + scroll.forceLayout() + scroll.measure( + MeasureSpec.makeMeasureSpec(scroll.measuredWidth, MeasureSpec.EXACTLY), + MeasureSpec.makeMeasureSpec( + (measuredHeight - paddingTop - paddingBottom - menuTop).coerceAtLeast(0), + MeasureSpec.EXACTLY, + ), + ) + } + + override fun onLayout( + changed: Boolean, + left: Int, + top: Int, + right: Int, + bottom: Int, + ) { + super.onLayout(changed, left, top, right, bottom) + val menu = menuScroll?.getChildAt(0) ?: return + menu.offsetTopAndBottom(-menu.top) + } + + private fun menuTop(): Int { + val header = headerView?.takeUnless { it.isGone } ?: return menuMarginTop + return header.marginTop + header.measuredHeight + menuMarginTop + } + + private fun enableMenuScrolling() { + post { + val menuView = + (0 until childCount) + .map { getChildAt(it) } + .firstOrNull { it is NavigationBarMenuView } + ?: return@post + + if (menuView.parent is NestedScrollView) return@post + + removeView(menuView) + + val scroll = + NestedScrollView(context).apply { + isVerticalScrollBarEnabled = false + addView( + menuView, + LayoutParams( + LayoutParams.WRAP_CONTENT, + LayoutParams.WRAP_CONTENT, + ), + ) + } + + addView( + scroll, + LayoutParams( + LayoutParams.WRAP_CONTENT, + LayoutParams.MATCH_PARENT, + ), + ) + menuScroll = scroll + } + } + } diff --git a/app/src/main/res/values/dimens.xml b/app/src/main/res/values/dimens.xml index 70c48d7d9b..f0ee3d5ea8 100644 --- a/app/src/main/res/values/dimens.xml +++ b/app/src/main/res/values/dimens.xml @@ -29,4 +29,5 @@ 44dp 64dp 6dp + 8dp diff --git a/app/src/test/java/com/itsaky/androidide/ui/IdeNavigationRailViewTest.kt b/app/src/test/java/com/itsaky/androidide/ui/IdeNavigationRailViewTest.kt new file mode 100644 index 0000000000..42645cd04b --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/ui/IdeNavigationRailViewTest.kt @@ -0,0 +1,83 @@ +package com.itsaky.androidide.ui + +import android.app.Activity +import android.app.Application +import android.os.Looper +import android.view.ContextThemeWrapper +import android.view.View +import android.view.ViewGroup +import android.widget.FrameLayout +import androidx.core.view.children +import androidx.core.widget.NestedScrollView +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(application = Application::class) +class IdeNavigationRailViewTest { + @Test + fun `holds more items than a material navigation rail allows`() { + val context = + ContextThemeWrapper( + ApplicationProvider.getApplicationContext(), + com.google.android.material.R.style.Theme_Material3_DayNight, + ) + val rail = IdeNavigationRailView(context) + + repeat(ITEM_COUNT) { rail.menu.add(0, it + 1, it, "Item $it") } + + assertThat(rail.menu.size()).isEqualTo(ITEM_COUNT) + } + + @Test + fun `scrolls to the last item when the rail has a header`() = assertScrollsToLastItem(withHeader = true) + + @Test + fun `scrolls to the last item when the rail has no header`() = assertScrollsToLastItem(withHeader = false) + + private fun assertScrollsToLastItem(withHeader: Boolean) { + val activity = Robolectric.buildActivity(Activity::class.java).setup().get() + val rail = + IdeNavigationRailView( + ContextThemeWrapper(activity, com.google.android.material.R.style.Theme_Material3_DayNight), + ) + if (withHeader) { + rail.addHeaderView( + FrameLayout(rail.context).apply { + addView(View(context), FrameLayout.LayoutParams(1, HEADER_HEIGHT)) + }, + ) + } + repeat(ITEM_COUNT) { rail.menu.add(0, it + 1, it, "Item $it") } + activity.setContentView(rail, ViewGroup.LayoutParams(RAIL_WIDTH, RAIL_HEIGHT)) + shadowOf(Looper.getMainLooper()).idle() + rail.measure( + View.MeasureSpec.makeMeasureSpec(RAIL_WIDTH, View.MeasureSpec.EXACTLY), + View.MeasureSpec.makeMeasureSpec(RAIL_HEIGHT, View.MeasureSpec.EXACTLY), + ) + rail.layout(0, 0, RAIL_WIDTH, RAIL_HEIGHT) + + val scroll = rail.children.filterIsInstance().single() + val menu = scroll.getChildAt(0) as ViewGroup + assertThat(scroll.top).isAtLeast(rail.headerView?.bottom ?: 0) + assertThat(scroll.canScrollVertically(1)).isTrue() + + scroll.scrollTo(0, menu.height) + + val lastItem = menu.getChildAt(menu.childCount - 1) + assertThat(menu.top + lastItem.bottom - scroll.scrollY).isAtMost(scroll.height) + } + + private companion object { + const val ITEM_COUNT = 21 + const val HEADER_HEIGHT = 120 + const val RAIL_WIDTH = 80 + const val RAIL_HEIGHT = 600 + } +} diff --git a/docs/PLUGIN_API_CHANGELOG.md b/docs/PLUGIN_API_CHANGELOG.md index d5b77cb544..07d22a8b28 100644 --- a/docs/PLUGIN_API_CHANGELOG.md +++ b/docs/PLUGIN_API_CHANGELOG.md @@ -36,6 +36,15 @@ milestone. **[verified]** = read from the checked-in ABI dump. **[reconstructed] = diffed from `plugin-api/src` history (predates the dump; symbol-accurate). ### 26.41 — unreleased +- **added — No cap on sidebar items** _(ADFA-4977)_ + The sidebar held 12 items: the IDE's seven plus the slots plugins declared with + `plugin.sidebar_items`. A plugin declaring more than the free slots failed to load, and + plugins loaded before the editor counted its own items could overfill the sidebar and + crash the IDE at launch. The sidebar now scrolls, so every declared item is shown. + `IdeSidebarService.getMaxSidebarItems()` and `getAvailableSidebarSlots()` return + `Int.MAX_VALUE`, and `canAddSidebarItems()` returns `true`. A plugin still returns no + more items than it declares. Floor `plugin.min_ide_version` at `26.41` if the plugin + declares more than 5 items, the slots an older IDE leaves free. - **added — Tool-source groups and health, backend model names, and change listeners** _(ADFA-6278)_ **[verified]** A consumer such as the agent's chat screen could not tell which tools the agent has, whether they work, or which model will answer, and was never told when any of that changed. diff --git a/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt b/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt index aedff8ed0b..1fc1f3d164 100644 --- a/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt +++ b/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt @@ -4,7 +4,6 @@ package com.itsaky.androidide.plugins.manager.core import android.app.Activity import android.content.Context -import com.itsaky.androidide.actions.SidebarSlotExceededException import com.itsaky.androidide.actions.SidebarSlotManager import com.itsaky.androidide.plugins.IPlugin import com.itsaky.androidide.plugins.PluginContext @@ -552,14 +551,7 @@ class PluginManager private constructor( return Result.failure(SecurityException("plugin failed security validation: ${manifest.id}")) } - // Validate sidebar slots BEFORE loading plugin code if (manifest.sidebarItems > 0) { - val available = SidebarSlotManager.getAvailableSlotsForPlugins() - if (manifest.sidebarItems > available) { - return Result.failure( - SidebarSlotExceededException(manifest.sidebarItems, available, manifest.id), - ) - } SidebarSlotManager.reservePluginSlots(manifest.id, manifest.sidebarItems) reservedSlotsPluginId = manifest.id } diff --git a/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeSidebarServiceImpl.kt b/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeSidebarServiceImpl.kt index 601006a55d..d3e7186925 100644 --- a/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeSidebarServiceImpl.kt +++ b/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeSidebarServiceImpl.kt @@ -6,16 +6,15 @@ import com.itsaky.androidide.actions.SidebarSlotManager import com.itsaky.androidide.plugins.services.IdeSidebarService class IdeSidebarServiceImpl( - private val pluginId: String + private val pluginId: String, ) : IdeSidebarService { + override fun getAvailableSidebarSlots(): Int = Int.MAX_VALUE - override fun getAvailableSidebarSlots(): Int = SidebarSlotManager.getAvailableSlotsForPlugins() + override fun canAddSidebarItems(count: Int): Boolean = true - override fun canAddSidebarItems(count: Int): Boolean = SidebarSlotManager.canAddPluginItems(count) + override fun getMaxSidebarItems(): Int = Int.MAX_VALUE - override fun getMaxSidebarItems(): Int = SidebarSlotManager.MAX_NAVIGATION_RAIL_ITEMS + override fun getCurrentSidebarItemCount(): Int = SidebarSlotManager.getTotalItemCount() - override fun getCurrentSidebarItemCount(): Int = SidebarSlotManager.getTotalItemCount() - - override fun getDeclaredSidebarSlots(): Int = SidebarSlotManager.getDeclaredSlots(pluginId) + override fun getDeclaredSidebarSlots(): Int = SidebarSlotManager.getDeclaredSlots(pluginId) } diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeSidebarServiceImplTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeSidebarServiceImplTest.kt new file mode 100644 index 0000000000..9ad883ea8d --- /dev/null +++ b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeSidebarServiceImplTest.kt @@ -0,0 +1,23 @@ +package com.itsaky.androidide.plugins.manager.services + +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.actions.SidebarSlotManager +import org.junit.After +import org.junit.Test + +class IdeSidebarServiceImplTest { + @After + fun tearDown() = SidebarSlotManager.reset() + + @Test + fun `a plugin can add sidebar items however many are already in the sidebar`() { + SidebarSlotManager.setBuiltInItemCount(7) + SidebarSlotManager.reservePluginSlots("plugin.a", 5) + + val service = IdeSidebarServiceImpl("plugin.b") + + assertThat(service.canAddSidebarItems(20)).isTrue() + assertThat(service.getAvailableSidebarSlots()).isEqualTo(Int.MAX_VALUE) + assertThat(service.getMaxSidebarItems()).isEqualTo(Int.MAX_VALUE) + } +}