Replace select with buttons in VictoryScreen.kt charts

[WhoIsJohannes]

I find the select difficult to use.

1. It needs a lot of clicks.
2. The click targets are too small.

Can we just use buttons like on the other side? I tried implementing it but failed to make it work. Here's my WIP.

Index: core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCharts.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCharts.kt b/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCharts.kt
--- a/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCharts.kt	(revision cafcbbad4b6c1c17d106121fa2f88d0f915e2f88)
+++ b/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCharts.kt	(date 1682190305899)
@@ -9,12 +9,11 @@
 import com.unciv.ui.components.AutoScrollPane
 import com.unciv.ui.components.LineChart
 import com.unciv.ui.components.TabbedPager
-import com.unciv.ui.components.extensions.onChange
 import com.unciv.ui.components.extensions.onClick
 import com.unciv.ui.components.extensions.packIfNeeded
+import com.unciv.ui.images.IconTextButton
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.screens.basescreen.BaseScreen
-import com.unciv.ui.screens.newgamescreen.TranslatedSelectBox
 import com.unciv.ui.screens.victoryscreen.VictoryScreenCivGroup.DefeatedPlayerStyle
 import com.unciv.ui.screens.worldscreen.WorldScreen
 
@@ -27,10 +26,10 @@
     private var selectedCiv = worldScreen.selectedCiv
     private val viewingCiv = worldScreen.viewingCiv
 
-    private val rankingTypeSelect = TranslatedSelectBox(RankingType.values().map { it.label }, rankingType.name, skin)
+    private val scoreTypeTable = Table()
+    private val scoreTypeScroll = AutoScrollPane(scoreTypeTable)
     private val civButtonsTable = Table()
     private val civButtonsScroll = AutoScrollPane(civButtonsTable)
-    private val controlsColumn = Table()
     private val markerIcon = ImageGetter.getImage("OtherIcons/Star").apply {
         color = Color.GOLD
         align = Align.center
@@ -39,28 +38,40 @@
     private val chartHolder = Container<LineChart?>(null)
 
     init {
+        scoreTypeScroll.setScrollingDisabled(true, false)
+        scoreTypeTable.defaults().space(40f).fillX()
+
         civButtonsScroll.setScrollingDisabled(true, false)
-        civButtonsTable.defaults().space(20f).fillX()
-        controlsColumn.defaults().space(20f).fillX()
-        controlsColumn.add(rankingTypeSelect).right().row()
-        controlsColumn.add(civButtonsScroll).fillY()
+        civButtonsTable.defaults().space(40f).fillX()
+
         defaults().fill().pad(20f)
-        add(controlsColumn)
+        add(scoreTypeScroll)
         add(chartHolder).growX().top().padLeft(0f)
-
-        rankingTypeSelect.onChange {
-            rankingType = RankingType.values()
-                .firstOrNull { it.name == rankingTypeSelect.selected.value }
-                ?: RankingType.Score
-            update()
-        }
+        add(civButtonsScroll)
     }
 
     private fun update() {
+        updateScoreTypes()
         updateControls()
         updateChart()
     }
 
+    private fun updateScoreTypes() {
+        scoreTypeTable.clear()
+        for (it in RankingType.values()) {
+            val button = IconTextButton(it.label)
+            button.touchable = Touchable.enabled
+            scoreTypeTable.add(button).row()
+            button.onClick {
+                rankingType = it
+                update()
+            }
+        }
+        scoreTypeTable.add().padBottom(20f).row()
+        scoreTypeTable.pack()
+        scoreTypeScroll.layout()
+    }
+
     private fun updateControls() {
         civButtonsTable.clear()
         val sortedCivs = gameInfo.civilizations.asSequence()
@@ -113,7 +124,6 @@
 
     override fun activated(index: Int, caption: String, pager: TabbedPager) {
         pager.setScrollDisabled(true)
-        getCell(controlsColumn).height(parent.height)
         getCell(chartHolder).height(parent.height)
         if (chartHolder.actor == null) update()
         civButtonsTable.invalidateHierarchy()

[yairm210]

Pics please

[SomeTroglodyte]

Relates loosely to #9138..
Also, without unshelving your patch, first question of UI change is - where, how much space is it gonna get and how should not-enough-space be handled? Hmmm, Scroll|Chart|Scroll side by side. Next - Gdx.ScrollPane - read the JavaDoc. It says it expects its size to be externally limited. Before, it was fillY within a holder that got its height set. Now - I see nowhere either ScrollPane height is limited. Lastly, be aware that I've made the screen "be" mostly a TabbedPager, and pager tabs are designed to be scrollable. That outer ScrollPane is blocked on tab activation (so the inner scroll is the only scroll listener), but it still exists and means the outer containers do not really limit your height or width here, gotta do it yourself.

It just might be enough to instead of getCell(chartHolder).height(parent.height) - the chart is hard-limited anyway - to limit both scrollpanes the same way. Width is another matter, could well be that on landscape the three side by side Widgets won't add up to more than a screen... On portrait even the current page needs test+optimization. Maybe even rearrange completely (in portrait) and put both selectors in expanders on top so the chart can have the entire width.. That would either mean the expander and chart are part of one layout meaning the expander pushes the chart down while open and one might want to reactivate the tab's scroller; or - have never tested such - make a floating expander with an empty placeholder underneath...

[github-actions]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 15 days.

[SeventhM]

Pics please

[github-actions]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 15 days.

[github-actions]

This issue was closed because it has been stalled for 5 days with no activity.