Make ExpanderTab "expand" properly (#9522)

* Make ExpanderTab "expand" properly

* Make ExpanderTab "expand" properly - patch1

* Make ExpanderTab "expand" properly - new signature

* Make ExpanderTab "expand" properly - enable dynamic content

* Minor WorldScreenMusicPopup visual tweaks

* Make ExpanderTab "expand" properly - tweaks

* Make ExpanderTab "expand" properly - Kdoc and types review

* Post-merge fixes

core/src/com/unciv/ui/components/ExpanderTab.kt

@@ -1,31 +1,41 @@
 package com.unciv.ui.components
 
 import com.badlogic.gdx.graphics.Color
-import com.badlogic.gdx.math.Interpolation
 import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.actions.FloatAction
+import com.badlogic.gdx.scenes.scene2d.ui.Cell
+import com.badlogic.gdx.scenes.scene2d.ui.Container
+import com.badlogic.gdx.scenes.scene2d.ui.Image
 import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.scenes.scene2d.ui.WidgetGroup
+import com.badlogic.gdx.scenes.scene2d.utils.Layout
 import com.badlogic.gdx.utils.Align
 import com.unciv.Constants
 import com.unciv.UncivGame
-import com.unciv.ui.images.ImageGetter
+import com.unciv.models.metadata.GameSettings
-import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.toLabel
+import com.unciv.ui.components.input.onClick
+import com.unciv.ui.images.IconCircleGroup
+import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.screens.basescreen.BaseScreen
+import kotlin.math.abs
 
 /**
  * A widget with a header that when clicked shows/hides a sub-Table.
  *
  * @param title The header text, automatically translated.
  * @param fontSize Size applied to header text (only)
- * @param icon Optional icon - please use [Image][com.badlogic.gdx.scenes.scene2d.ui.Image] or [IconCircleGroup]
+ * @param icon Optional icon - please use [Image] or [IconCircleGroup] and make sure size is set
+ * @param startsOutOpened Default initial "open" state if no [persistenceID] set or no persistes state found
  * @param defaultPad Padding between content and wrapper.
  * @param headerPad Default padding for the header Table.
- * @param expanderWidth If set initializes header width
+ * @param headerAlign How the header content aligns - use [Align] constants.
+ * @param expanderWidth If set initializes cell minWidth and wrapper width
  * @param persistenceID If specified, the ExpanderTab will remember its open/closed state for the duration of one app run
- * @param onChange If specified, this will be called after the visual change for a change in [isOpen] completes (e.g. to react to changed size)
+ * @param animated Controls whether opening/closing is animated, defaults to the [continuousRendering][GameSettings.continuousRendering] setting.
- * @param initContent Optional lambda with [innerTable] as parameter, to help initialize content.
+ * @param content An [Actor] supporting [Layout] with the content to display in expanded state. Will be `pack()`ed!
+ * @param onChange If specified, this will be called on any visual change: repeatedly during animation if enabled, otherwise once after each change to [isOpen]. (e.g. to react to changed size)
  */
 class ExpanderTab(
     title: String,
@@ -34,27 +44,64 @@ class ExpanderTab(
     startsOutOpened: Boolean = true,
     defaultPad: Float = 10f,
     headerPad: Float = 10f,
-    expanderWidth: Float = 0f,
+    headerAlign: Int = Align.center,
+    private val expanderWidth: Float = 0f,
     private val persistenceID: String? = null,
-    private val onChange: (() -> Unit)? = null,
+    animated: Boolean? = null,
-    initContent: ((Table) -> Unit)? = null
+    private val content: WidgetGroup,
-): Table(BaseScreen.skin) {
+    private val onChange: (() -> Unit)? = null
-    private companion object {
+) : Table(BaseScreen.skin) {
-        const val arrowSize = 18f
+    /** Alternate builder-style constructor for an [ExpanderTab]
-        const val arrowImage = "OtherIcons/BackArrow"
+     *
-        val arrowColor = Color(1f,0.96f,0.75f,1f)
+     * @param initContent A lambda with the future [content] as parameter, to help initialize. Will be `pack()`ed when done!
-        const val animationDuration = 0.2f
+     */
+    constructor(
+        title: String,
+        fontSize: Int = Constants.headingFontSize,
+        icon: Actor? = null,
+        startsOutOpened: Boolean = true,
+        defaultPad: Float = 10f,
+        headerPad: Float = 10f,
+        headerAlign: Int = Align.center,
+        expanderWidth: Float = 0f,
+        persistenceID: String? = null,
+        animated: Boolean? = null,
+        onChange: (() -> Unit)? = null,
+        initContent: ((Table) -> Unit)
+    ) : this (
+        title, fontSize, icon, startsOutOpened, defaultPad,
+        headerPad, headerAlign, expanderWidth, persistenceID, animated,
+        Table(BaseScreen.skin).apply {
+            defaults().growX()
+            initContent(this)
+        },
+        onChange
+    )
 
-        val persistedStates = HashMap<String, Boolean>()
+    companion object {
+        private const val arrowSize = 18f
+        private const val arrowImage = "OtherIcons/BackArrow"
+        private val arrowColor = Color(1f,0.96f,0.75f,1f)
+        private const val animationDurationForStageHeight = 0.5f  // also serves as maximum
+
+        private val persistedStates = HashMap<String, Boolean>()
     }
 
-    val header = Table(skin)  // Header with label and icon, touchable to show/hide
+    // _Please_ don't make header, wrapper or content public. Makes tweaking this widget harder.
+    // If more control is needed and the parameter count gets too high, consider using a Style class
+    // or open class / protected fun createHeader() or dedicated setters instead.
+    private val header = Table(skin)  // Header with label and icon, touchable to show/hide
     private val headerLabel = title.toLabel(fontSize = fontSize)
-    private val headerIcon = ImageGetter.getImage(arrowImage)
+    private val arrowIcon = ImageGetter.getImage(arrowImage)
-    private val contentWrapper = Table()  // Wrapper for innerTable, this is what will be shown/hidden
+    private val headerCell: Cell<Table>
 
-    /** The container where the client should add the content to toggle */
+    private val wrapper: Container<WidgetGroup>
-    val innerTable = Table()
+    private val wrapperCell: Cell<Container<WidgetGroup>>
+    private var wrapperWidth: Float = 0f
+    private var wrapperHeight: Float = 0f
+
+    private var currentPercent = 0f
+    private val noAnimation = !(animated ?: UncivGame.Current.settings.continuousRendering)
 
     /** Indicates whether the contents are currently shown, changing this will animate the widget */
     // This works because a HashMap _could_ store an entry for the null key but we cannot actually store one when declaring as HashMap<String, Boolean>
@@ -66,11 +113,14 @@ class ExpanderTab(
         }
 
     init {
+        setLayoutEnabled(false)
+
+        header.align(headerAlign)
         header.defaults().pad(headerPad)
-        headerIcon.setSize(arrowSize, arrowSize)
+        arrowIcon.setSize(arrowSize, arrowSize)
-        headerIcon.setOrigin(Align.center)
+        arrowIcon.setOrigin(Align.center)
-        headerIcon.rotation = 180f
+        arrowIcon.rotation = 180f
-        headerIcon.color = arrowColor
+        arrowIcon.color = arrowColor
         header.background(
             BaseScreen.skinStrings.getUiBackground(
                 "General/ExpanderTab",
@@ -79,48 +129,78 @@ class ExpanderTab(
         )
         if (icon != null) header.add(icon)
         header.add(headerLabel)
-        header.add(headerIcon).size(arrowSize).align(Align.center)
+        header.add(arrowIcon).size(arrowSize).align(Align.center)
         header.touchable= Touchable.enabled
         header.onClick { toggle() }
-        if (expanderWidth != 0f)
+
-            defaults().minWidth(expanderWidth)
+        content.pack()
+        measureContent()
+
+        wrapper = Container(content).apply {
+                setRound(false)
+                bottom()  // controls what is seen first on opening!
+                setSize(wrapperWidth, 0f)
+            }
+
         defaults().growX()
-        contentWrapper.defaults().growX().pad(defaultPad)
+        headerCell = add(header).minWidth(wrapperWidth)
-        innerTable.defaults().growX()
+        row()
-        add(header).fillY().row()
+        wrapperCell = add(wrapper).size(wrapperWidth, 0f).pad(defaultPad)
-        add(contentWrapper)
+
-        contentWrapper.add(innerTable)      // update will revert this
+        setLayoutEnabled(true)
-        initContent?.invoke(innerTable)
+        update(fromInit = true)
-        if (expanderWidth == 0f) {
-            // Measure content width incl. pad, set header to same width
-            if (innerTable.needsLayout()) contentWrapper.pack()
-            getCell(header).minWidth(contentWrapper.width)
-        }
-        update(noAnimation = true)
     }
 
-    private fun update(noAnimation: Boolean = false) {
+    override fun getPrefHeight() = header.prefHeight + wrapperHeight * currentPercent
+
+    override fun layout() {
+        // Critical magic here! Key to allow dynamic content.
+        // However, I can't explain why an invalidated header also needs to trigger it. Without, the
+        // WorldScreenMusicPopup's expanders, which are width-controlled by their outer cell's fillX/expandX,
+        // start aligned and same width, but will slightly misalign by some 10f on opening/closing some of them.
+        if (content.needsLayout() || header.needsLayout())
+            contentHasChanged()
+        super.layout()
+    }
+
+    private fun contentHasChanged() {
+        val oldWidth = wrapperWidth
+        val oldHeight = wrapperHeight
+        content.pack()
+        measureContent()
+        if (wrapperWidth == oldWidth && wrapperHeight == oldHeight) return
+        headerCell.minWidth(wrapperWidth)
+        currentPercent *= oldHeight / wrapperHeight  // to animate smoothly to new height, >1f should work too
+        update()
+    }
+
+    private fun measureContent() {
+        wrapperWidth = if (expanderWidth > 0f) expanderWidth else content.width
+        wrapperHeight = content.height
+    }
+
+    private fun update(fromInit: Boolean = false) {
         if (persistenceID != null)
             persistedStates[persistenceID] = isOpen
-        if (noAnimation || !UncivGame.Current.settings.continuousRendering) {
+
-            contentWrapper.clear()
+        if (noAnimation || fromInit) {
-            if (isOpen) contentWrapper.add(innerTable)
+            updateContentVisibility(if (isOpen) 1f else 0f)
-            headerIcon.rotation = if (isOpen) 90f else 180f
+            wrapper.isVisible = isOpen
-            if (!noAnimation) onChange?.invoke()
+            if (!fromInit) onChange?.invoke()
             return
         }
-        val action = object: FloatAction ( 90f, 180f, animationDuration, Interpolation.linear) {
+
-            override fun update(percent: Float) {
+        clearActions()
-                super.update(percent)
+        addAction(ExpandAction())
-                headerIcon.rotation = this.value
+    }
-                if (this.isComplete) {
+
-                    contentWrapper.clear()
+    private fun updateContentVisibility(percent: Float) {
-                    if (isOpen) contentWrapper.add(innerTable)
+        currentPercent = percent
-                    onChange?.invoke()
+        val height = percent * wrapperHeight
-                }
+        wrapperCell.size(wrapperWidth, height)  // needed for layout
-            }
+        wrapper.setSize(wrapperWidth, height)   // needed for clipping
-        }.apply { isReverse = isOpen }
+        arrowIcon.rotation = 90f * (2f - percent)
-        addAction(action)
+        invalidateHierarchy()
     }
 
     /** Toggle [isOpen], animated */
@@ -128,8 +208,38 @@ class ExpanderTab(
         isOpen = !isOpen
     }
 
-    /** Change header label text after initialization */
+    /** Change header label text after initialization - **no** auto-translation! */
     fun setText(text: String) {
         headerLabel.setText(text)
     }
+
+    private inner class ExpandAction : FloatAction() {
+        init {
+            start = currentPercent  // start from wherever we were if turned around midway
+            end = if (isOpen) 1f else 0f
+            // Duration: shorter if less content height...
+            val heightFactor = stage?.run { wrapperHeight.coerceAtMost(height) / height } ?: 0.5f
+            // ... and shorter if turned around midway
+            val distanceFactor = abs(end - currentPercent)
+            duration = (animationDurationForStageHeight * heightFactor)
+                .coerceAtLeast(0.15f) * distanceFactor
+        }
+
+        override fun begin() {
+            super.begin()
+            wrapper.clip(true)
+            wrapper.isVisible = true
+        }
+
+        override fun update(percent: Float) {
+            super.update(percent)
+            updateContentVisibility(value)
+            onChange?.invoke()
+        }
+
+        override fun end() {
+            wrapper.clip(false)
+            wrapper.isVisible = isOpen   // allows turning clip off in closed state
+        }
+    }
 }

core/src/com/unciv/ui/popups/options/ModCheckTab.kt

@@ -123,7 +123,7 @@ class ModCheckTab(
                         .apply { color = Color.BLACK }
                         .surroundWithCircle(30f, color = iconColor)
 
-                    val expanderTab = ExpanderTab(mod.name, icon = icon, startsOutOpened = false) {
+                    val expanderTab = ExpanderTab(mod.name, icon = icon, startsOutOpened = false, headerAlign = Align.left) {
                         it.defaults().align(Align.left)
                         if (!noProblem && mod.folderLocation != null) {
                             val replaceableUniques = getDeprecatedReplaceableUniques(mod)
@@ -143,7 +143,6 @@ class ModCheckTab(
                                     .joinToString("\n") { line -> line.text }
                             }).row()
                     }
-                    expanderTab.header.left()
 
                     val loadingLabel = modCheckResultTable.children.last()
                     modCheckResultTable.removeActor(loadingLabel)

core/src/com/unciv/ui/screens/cityscreen/CitizenManagementTable.kt

@@ -83,16 +83,15 @@ class CitizenManagementTable(val cityScreen: CityScreen) : Table(BaseScreen.skin
     }
 
     fun asExpander(onChange: (() -> Unit)?): ExpanderTab {
+        update()
         return ExpanderTab(
             title = "{Citizen Management}",
             fontSize = Constants.defaultFontSize,
             persistenceID = "CityStatsTable.CitizenManagement",
             startsOutOpened = false,
+            content = this,
             onChange = onChange
-        ) {
+        )
-            it.add(this)
-            update()
-        }
     }
 
 }

core/src/com/unciv/ui/screens/cityscreen/CityReligionInfoTable.kt

@@ -96,6 +96,7 @@ class CityReligionInfoTable(
 
     fun asExpander(onChange: (()->Unit)?): ExpanderTab {
         val (icon, label) = getIconAndLabel(religionManager.getMajorityReligion())
+        defaults().center().pad(5f)
         return ExpanderTab(
                 title = "Majority Religion: [$label]",
                 fontSize = Constants.defaultFontSize,
@@ -103,10 +104,8 @@ class CityReligionInfoTable(
                 defaultPad = 0f,
                 persistenceID = "CityStatsTable.Religion",
                 startsOutOpened = false,
+                content = this,
                 onChange = onChange
-            ) {
+            )
-                defaults().center().pad(5f)
-                it.add(this)
-            }
     }
 }

core/src/com/unciv/ui/screens/cityscreen/CityStatsTable.kt

@@ -232,7 +232,6 @@ class CityStatsTable(private val cityScreen: CityScreen): Table() {
         otherBuildings.sortBy { it.name }
 
         val totalTable = Table()
-        lowerTable.addCategory("Buildings", totalTable, false)
 
         if (specialistBuildings.isNotEmpty()) {
             val specialistBuildingsTable = Table()
@@ -261,6 +260,8 @@ class CityStatsTable(private val cityScreen: CityScreen): Table() {
             for (building in otherBuildings) addBuildingButton(building, regularBuildingsTable)
             totalTable.add(regularBuildingsTable).growX().right().row()
         }
+
+        lowerTable.addCategory("Buildings", totalTable, false)
     }
 
     private fun addBuildingButton(building: Building, destinationTable: Table) {
@@ -312,17 +313,15 @@ class CityStatsTable(private val cityScreen: CityScreen): Table() {
         destinationTable.add(button).pad(1f).padBottom(2f).padTop(2f).expandX().right().row()
     }
 
-    private fun Table.addCategory(category: String, showHideTable: Table, startsOpened: Boolean = true, innerPadding: Float = 10f) : ExpanderTab {
+    private fun Table.addCategory(category: String, showHideTable: Table, startsOpened: Boolean = true) : ExpanderTab {
         val expanderTab = ExpanderTab(
             title = category,
             fontSize = Constants.defaultFontSize,
             persistenceID = "CityInfo.$category",
             startsOutOpened = startsOpened,
-            defaultPad = innerPadding,
+            content = showHideTable,
             onChange = { onContentResize() }
-        ) {
+        )
-            it.add(showHideTable).fillX().right()
-        }
         add(expanderTab).growX().row()
         return expanderTab
     }

core/src/com/unciv/ui/screens/cityscreen/SpecialistAllocationTable.kt

@@ -136,16 +136,15 @@ class SpecialistAllocationTable(private val cityScreen: CityScreen) : Table(Base
 
 
     fun asExpander(onChange: (() -> Unit)?): ExpanderTab {
+        update()
         return ExpanderTab(
             title = "{Specialists}:",
             fontSize = Constants.defaultFontSize,
             persistenceID = "CityStatsTable.Specialists",
             startsOutOpened = true,
+            content = this,
             onChange = onChange
-        ) {
+        )
-            it.add(this)
-            update()
-        }
     }
 
 }

core/src/com/unciv/ui/screens/diplomacyscreen/OffersListScroll.kt

@@ -23,6 +23,7 @@ import com.unciv.models.ruleset.tile.ResourceSupplyList
 import com.unciv.models.translations.tr
 import com.unciv.ui.components.ExpanderTab
 import com.unciv.ui.components.extensions.disable
+import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.input.onClick
 import com.unciv.ui.images.IconTextButton
 import com.unciv.ui.images.ImageGetter
@@ -41,8 +42,11 @@ class OffersListScroll(
 ) : ScrollPane(null) {
     val table = Table(BaseScreen.skin).apply { defaults().pad(5f) }
 
-
+    private data class ExpanderData(
-    private val expanderTabs = HashMap<TradeType, ExpanderTab>()
+        val label: String,
+        val content: Table = Table().apply { defaults().pad(5f) }
+    )
+    private val expanderContents = HashMap<TradeType, ExpanderData>()
 
     /**
      * @param offersToDisplay The offers which should be displayed as buttons
@@ -55,10 +59,10 @@ class OffersListScroll(
         untradableOffers: ResourceSupplyList = ResourceSupplyList.emptyList
     ) {
         table.clear()
-        expanderTabs.clear()
+        expanderContents.clear()
 
         for (offerType in values()) {
-            val labelName = when(offerType){
+            val labelName = when(offerType) {
                 Gold, Gold_Per_Turn, Treaty, Agreement, Introduction -> ""
                 Luxury_Resource -> "Luxury resources"
                 Strategic_Resource -> "Strategic resources"
@@ -68,11 +72,12 @@ class OffersListScroll(
             }
             val offersOfType = offersToDisplay.filter { it.type == offerType }
             if (labelName.isNotEmpty() && offersOfType.any()) {
-                expanderTabs[offerType] = ExpanderTab(labelName, persistenceID = "Trade.$persistenceID.$offerType") {
+                expanderContents[offerType] = ExpanderData(labelName)
-                    it.defaults().pad(5f)
-                }
             }
         }
+        val expanderWidth = (expanderContents.values.maxByOrNull { it.label.length }
+            ?.run { label.toLabel(fontSize = Constants.headingFontSize).prefWidth }
+            ?: 0f) + 50f  // 50 for Expander header pad and arrow
 
         for (offerType in values()) {
             val offersOfType = offersToDisplay.filter { it.type == offerType }
@@ -81,11 +86,6 @@ class OffersListScroll(
                     { if (it.type==City) it.getOfferText() else it.name.tr() }
                 ))
 
-            if (expanderTabs.containsKey(offerType)) {
-                expanderTabs[offerType]!!.innerTable.clear()
-                table.add(expanderTabs[offerType]!!).row()
-            }
-
             for (offer in offersOfType) {
                 val tradeLabel = offer.getOfferText(untradableOffers.sumBy(offer.name))
                 val tradeIcon = when (offer.type) {
@@ -122,11 +122,18 @@ class OffersListScroll(
                 else tradeButton.disable()  // for instance we have negative gold
 
 
-                if (expanderTabs.containsKey(offerType))
+                if (expanderContents.containsKey(offerType))
-                    expanderTabs[offerType]!!.innerTable.add(tradeButton).row()
+                    expanderContents[offerType]!!.content.add(tradeButton).row()
                 else
                     table.add(tradeButton).row()
             }
+
+            expanderContents[offerType]?.run {
+                table.add(
+                    ExpanderTab(label, expanderWidth = expanderWidth,
+                        persistenceID = "Trade.$persistenceID.$offerType", content = content)
+                ).row()
+            }
         }
         actor = table
     }

core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorViewTab.kt

@@ -120,12 +120,11 @@ class MapEditorViewTab(
                 "{Natural Wonders} (${naturalWonders.size})",
                 fontSize = 21,
                 startsOutOpened = false,
-                headerPad = 5f
+                headerPad = 5f,
-            ) {
+                content = MarkupRenderer.render(lines, iconDisplay = IconDisplay.NoLink) {
-                it.add(MarkupRenderer.render(lines, iconDisplay = IconDisplay.NoLink) { name->
+                    scrollToWonder(it)
-                    scrollToWonder(name)
+                }
-                })
+            )).row()
-            }).row()
         }
 
         // Starting locations not cached like natural wonders - storage is already compact
@@ -136,12 +135,11 @@ class MapEditorViewTab(
                 "{Starting locations} (${tileMap.startingLocationsByNation.size})",
                 fontSize = 21,
                 startsOutOpened = false,
-                headerPad = 5f
+                headerPad = 5f,
-            ) {
+                content = MarkupRenderer.render(lines.asIterable(), iconDisplay = IconDisplay.NoLink) {
-                it.add(MarkupRenderer.render(lines.asIterable(), iconDisplay = IconDisplay.NoLink) { name ->
+                    scrollToStartOfNation(it)
-                    scrollToStartOfNation(name)
+                }
-                })
+            )).row()
-            }).row()
         }
 
         addSeparator()

core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt

@@ -244,23 +244,20 @@ class NewGameScreen(
     private fun initPortrait() {
         scrollPane.setScrollingDisabled(false,false)
 
-        topTable.add(ExpanderTab("Game Options") {
+        topTable.add(ExpanderTab("Game Options", content = newGameOptionsTable))
-            it.add(newGameOptionsTable).row()
+            .expandX().fillX().row()
-        }).expandX().fillX().row()
         topTable.addSeparator(Color.DARK_GRAY, height = 1f)
 
         topTable.add(newGameOptionsTable.modCheckboxes).expandX().fillX().row()
         topTable.addSeparator(Color.DARK_GRAY, height = 1f)
 
-        topTable.add(ExpanderTab("Map Options") {
+        topTable.add(ExpanderTab("Map Options", content = mapOptionsTable))
-            it.add(mapOptionsTable).row()
+            .expandX().fillX().row()
-        }).expandX().fillX().row()
         topTable.addSeparator(Color.DARK_GRAY, height = 1f)
 
         (playerPickerTable.playerListTable.parent as ScrollPane).setScrollingDisabled(true,true)
-        topTable.add(ExpanderTab("Civilizations") {
+        topTable.add(ExpanderTab("Civilizations", content = playerPickerTable))
-            it.add(playerPickerTable).row()
+            .expandX().fillX().row()
-        }).expandX().fillX().row()
     }
 
     private fun checkConnectionToMultiplayerServer(): Boolean {

core/src/com/unciv/ui/screens/pickerscreens/ModManagementScreen.kt

@@ -158,21 +158,16 @@ class ModManagementScreen(
 
         topTable.add(optionsManager.expander).top().growX().row()
 
-        installedExpanderTab = ExpanderTab(optionsManager.getInstalledHeader(), expanderWidth = stage.width) {
+        installedExpanderTab = ExpanderTab(optionsManager.getInstalledHeader(), expanderWidth = stage.width, content = scrollInstalledMods)
-            it.add(scrollInstalledMods).growX()
-        }
         topTable.add(installedExpanderTab).top().growX().row()
 
-        onlineExpanderTab = ExpanderTab(optionsManager.getOnlineHeader(), expanderWidth = stage.width) {
+        onlineExpanderTab = ExpanderTab(optionsManager.getOnlineHeader(), expanderWidth = stage.width, content = scrollOnlineMods)
-            it.add(scrollOnlineMods).growX()
-        }
         topTable.add(onlineExpanderTab).top().padTop(10f).growX().row()
 
         topTable.add().expandY().row() // helps with top() being ignored
 
-        topTable.add(ExpanderTab("Mod info and options", expanderWidth = stage.width) {
+        topTable.add(ExpanderTab("Mod info and options", expanderWidth = stage.width, content = modActionTable))
-            it.add(modActionTable).growX()
+            .bottom().padTop(10f).growX().row()
-        }).bottom().padTop(10f).growX().row()
     }
 
     private fun initLandscape() {

core/src/com/unciv/ui/screens/worldscreen/mainmenu/WorldScreenMusicPopup.kt

@@ -34,7 +34,7 @@ class WorldScreenMusicPopup(
 
     private val musicController = UncivGame.Current.musicController
     private val trackStyle: TextButton.TextButtonStyle
-    private val historyExpander: ExpanderTab
+    private val historyTable = Table()
     private val visualMods = worldScreen.game.settings.visualMods
     private val mods = worldScreen.gameInfo.gameParameters.mods
 
@@ -58,13 +58,19 @@ class WorldScreenMusicPopup(
         trackStyle.disabledFontColor = Color.LIGHT_GRAY
 
         addMusicMods(settings)
-        historyExpander = addHistory()
+        addHistory()
         addMusicControls(bottomTable, settings, musicController)
-        addCloseButton().colspan(2)
+        addCloseButton().padTop(10f).padBottom(0f).colspan(2)
+
+        getScrollPane()?.run {
+            fadeScrollBars = false
+            if (bottomTable.prefWidth < prefWidth)
+                bottomTable.width = prefWidth
+        }
 
         musicController.onChange {
-            historyExpander.innerTable.clear()
+            historyTable.clear()
-            historyExpander.innerTable.updateTrackList(musicController.getHistory())
+            historyTable.updateTrackList(musicController.getHistory())
         }
     }
 
@@ -88,24 +94,24 @@ class WorldScreenMusicPopup(
         }
     }
 
-    private fun addHistory() = addTrackList("—History—", musicController.getHistory())
+    private fun addHistory() = addTrackList("—History—", musicController.getHistory(), historyTable)
 
-    private fun addTrackList(title: String, tracks: Sequence<MusicController.MusicTrackInfo>): ExpanderTab {
+    private fun addTrackList(title: String, tracks: Sequence<MusicController.MusicTrackInfo>, table: Table? = null) {
         // Note title is either a mod name or something that cannot be a mod name (thanks to the em-dashes)
         val icon = when (title) {
             in mods -> "OtherIcons/Mods"
             in visualMods -> "UnitPromotionIcons/Scouting"
             else -> null
         }?.let { ImageGetter.getImage(it).apply { setSize(18f) } }
+        val content = table ?: Table()
+        content.defaults().growX()
+        content.updateTrackList(tracks)
         val expander = ExpanderTab(title, Constants.defaultFontSize, icon,
             startsOutOpened = false, defaultPad = 0f, headerPad = 5f,
             persistenceID = "MusicPopup.$title",
-        ) {
+            content = content
-            it.updateTrackList(tracks)
+        )
-        }
         add(expander).colspan(2).growX().row()
-
-        return expander
     }
 
     private fun Table.updateTrackList(tracks: Sequence<MusicController.MusicTrackInfo>) {