Move ListItem bottom padding into GradientListView and Column spacing

Remove the built-in bottom padding from ListItem. This was previously required as a way to add spacing between items, instead of setting explicit 'spacing' on GradientListView or Column views, due to the fact that this would produce additional unwanted spacing for non-visible items within the view. Now that visible items are no longer rendered by the view if VisibleItemModel is used, this built-in padding can be removed, and the spacing can be inserted directly into ListView and Column views instead. Fixes #1827
victronenergy · Feb 17, 2025 · f294665 · f294665
1 parent 2eaaeaa
commit f294665
Show file tree

Hide file tree

Showing 48 changed files with 109 additions and 68 deletions.
diff --git a/components/FlatListItemSeparator.qml b/components/FlatListItemSeparator.qml
@@ -7,10 +7,8 @@ import QtQuick
 import Victron.VenusOS
 
 Item {
-	// ListItem has built-in spacing below each item (Theme.geometry_gradientList_spacing), so add
-	// this spacing below each separator to balance out the space between successful flat ListItems.
 	implicitWidth: parent ? parent.width : 0
-	implicitHeight: bar.height + Theme.geometry_gradientList_spacing
+	implicitHeight: bar.height
 
 	SeparatorBar {
 		id: bar

diff --git a/components/GradientListView.qml b/components/GradientListView.qml
@@ -16,9 +16,7 @@ ListView {
 	leftMargin: Theme.geometry_page_content_horizontalMargin
 	rightMargin: Theme.geometry_page_content_horizontalMargin
 	boundsBehavior: Flickable.StopAtBounds
-
-	// Note: do not set spacing here, as it creates extra spacing if an item has visible=false.
-	// Instead, the spacing is added visually within ListItem's ListItemBackground.
+	spacing: Theme.geometry_gradientList_spacing
 
 	ViewGradient {
 		anchors.bottom: root.bottom

diff --git a/components/InverterAcOutSettings.qml b/components/InverterAcOutSettings.qml
@@ -13,6 +13,7 @@ Column {
 	readonly property bool isInverterCharger: isInverterChargerItem.value === 1
 
 	width: parent ? parent.width : 0
+	spacing: Theme.geometry_gradientList_spacing
 
 	VeQuickItem {
 		id: isInverterChargerItem

diff --git a/components/PageGensetModel.qml b/components/PageGensetModel.qml
@@ -151,6 +151,7 @@ VisibleItemModel {
 
 	Column {
 		width: parent ? parent.width : 0
+		spacing: Theme.geometry_gradientList_spacing
 
 		Repeater {
 			id: phaseRepeater

diff --git a/components/RsSystemAcIODisplay.qml b/components/RsSystemAcIODisplay.qml
@@ -28,6 +28,8 @@ Loader {
 					: acOutL2.isValid ? "L2"
 					: "L1"  // i.e. if _phase.value === 0 || !_phase.isValid
 
+			spacing: Theme.geometry_gradientList_spacing
+
 			VeQuickItem { id: acOutL1; uid: root.serviceUid + "/Ac/Out/L1/P" }
 			VeQuickItem { id: acOutL2; uid: root.serviceUid + "/Ac/Out/L2/P" }
 			VeQuickItem { id: acOutL3; uid: root.serviceUid + "/Ac/Out/L3/P" }

diff --git a/components/TemperatureRelaySettings.qml b/components/TemperatureRelaySettings.qml
@@ -24,6 +24,7 @@ Column {
 	}
 
 	width: parent ? parent.width : 0
+	spacing: Theme.geometry_gradientList_spacing
 
 	VeQuickItem {
 		id: relay1Item

diff --git a/components/ThreePhaseIOTable.qml b/components/ThreePhaseIOTable.qml
@@ -17,7 +17,6 @@ Row {
 	property int voltPrecision: Units.defaultUnitPrecision(VenusOS.Units_Volt_AC)
 
 	spacing: Theme.geometry_vebusDeviceListPage_quantityTable_row_spacing
-	bottomPadding: Theme.geometry_gradientList_spacing
 
 	ThreePhaseQuantityTable {
 		id: inputTable

diff --git a/components/VeBusAcIODisplay.qml b/components/VeBusAcIODisplay.qml
@@ -30,6 +30,8 @@ Loader {
 		id: singlePhaseAcInOut
 
 		Column {
+			spacing: Theme.geometry_gradientList_spacing
+
 			PVCFListQuantityGroup {
 				text: CommonWords.ac_in
 				data: AcPhase { serviceUid: root.serviceUid + "/Ac/ActiveIn/L1" }

diff --git a/components/listitems/core/ListItem.qml b/components/listitems/core/ListItem.qml
@@ -86,14 +86,14 @@ Item {
 	signal clicked()
 
 	visible: effectiveVisible
-	implicitHeight: effectiveVisible ? (contentLayout.height + Theme.geometry_gradientList_spacing) : 0
+	implicitHeight: effectiveVisible ? contentLayout.height : 0
 	implicitWidth: parent ? parent.width : 0
 
 	ListItemBackground {
 		id: backgroundRect
 
 		z: -2
-		height: root.height - Theme.geometry_gradientList_spacing
+		height: root.height
 		color: Theme.color_listItem_background
 		visible: !root.flat
 		// TODO how to indicate read-only setting?

diff --git a/components/listitems/core/ListQuantityGroupNavigation.qml b/components/listitems/core/ListQuantityGroupNavigation.qml
@@ -21,6 +21,6 @@ ListNavigation {
 			right: parent.right
 			rightMargin: Theme.geometry_listItem_content_horizontalMargin + Theme.geometry_icon_size_medium
 		}
-		height: parent.height - Theme.geometry_gradientList_spacing
+		height: parent.height
 	}
 }
diff --git a/components/listitems/core/ListRadioButtonGroup.qml b/components/listitems/core/ListRadioButtonGroup.qml
@@ -95,6 +95,10 @@ ListNavigation {
 						popTimer.restartIfNeeded()
 					}
 
+					// TODO this is a hack to cancel out the GradientListView spacing, to avoid
+					// showing extra spacing between items if an item is not visible. See #1907.
+					height: effectiveVisible ? implicitHeight : -Theme.geometry_gradientList_spacing
+
 					text: Array.isArray(root.optionModel)
 						  ? modelData.display || ""
 						  : model.display || ""
@@ -137,6 +141,7 @@ ListNavigation {
 							}
 
 							width: parent.width
+							spacing: Theme.geometry_gradientList_spacing
 
 							PrimaryListLabel {
 								topPadding: 0

diff --git a/components/listitems/core/SettingsListHeader.qml b/components/listitems/core/SettingsListHeader.qml
@@ -12,7 +12,6 @@ Label {
 		leftMargin: Theme.geometry_listItem_content_horizontalMargin
 	}
 	topPadding: Theme.geometry_settingsListHeader_verticalPadding
-	bottomPadding: Theme.geometry_settingsListHeader_verticalPadding
 	width: Math.max(implicitWidth, 1)
 	font.pixelSize: Theme.font_size_body1
 	wrapMode: Text.Wrap

diff --git a/pages/evcs/EvChargerPage.qml b/pages/evcs/EvChargerPage.qml
@@ -69,10 +69,7 @@ Page {
 				QuantityTable {
 					id: phaseTable
 
-					anchors {
-						top: chargerSummary.bottom
-						topMargin: Theme.geometry_gradientList_spacing
-					}
+					anchors.top: chargerSummary.bottom
 					visible: root.evCharger.phases.count > 1
 					metrics.equalWidthColumns: true
 					headerVisible: false
@@ -98,11 +95,6 @@ Page {
 				}
 			}
 
-			Item {
-				width: 1
-				height: Theme.geometry_gradientList_spacing
-			}
-
 			ListRadioButtonGroup {
 				id: chargeMode
 				//% "Charge mode"

diff --git a/pages/invertercharger/OverviewInverterChargerPage.qml b/pages/invertercharger/OverviewInverterChargerPage.qml
@@ -72,6 +72,7 @@ Page {
 
 			Column {
 				width: parent ? parent.width : 0
+				spacing: Theme.geometry_gradientList_spacing
 
 				Repeater {
 					model: AcInputSettingsModel {

diff --git a/pages/settings/DvccCommonSettings.qml b/pages/settings/DvccCommonSettings.qml
@@ -10,6 +10,8 @@ Column {
 	property alias dvccActive: dvccSwitch.checked
 	readonly property alias userHasWriteAccess: dvccSwitch.userHasWriteAccess
 
+	spacing: Theme.geometry_gradientList_spacing
+
 	ListSwitchForced {
 		id: dvccSwitch
 

diff --git a/pages/settings/PageChargeCurrentLimits.qml b/pages/settings/PageChargeCurrentLimits.qml
@@ -33,6 +33,7 @@ Page {
 
 	GradientListView {
 		header: DvccCommonSettings {
+			bottomPadding: Theme.geometry_gradientList_spacing
 			width: parent.width
 		}
 

diff --git a/pages/settings/PageSettingsBatteries.qml b/pages/settings/PageSettingsBatteries.qml
@@ -53,8 +53,8 @@ Page {
 
 			Column {
 				spacing: Theme.geometry_gradientList_spacing
-				bottomPadding: Theme.geometry_gradientList_spacing
 				width: parent ? parent.width : 0
+
 				Repeater {
 					model: batteryModel
 					width: parent.width

diff --git a/pages/settings/PageSettingsBleSensors.qml b/pages/settings/PageSettingsBleSensors.qml
@@ -84,6 +84,7 @@ Page {
 
 			Column {
 				width: parent ? parent.width : 0
+				spacing: Theme.geometry_gradientList_spacing
 
 				Repeater {
 					model: VeQItemSortTableModel {

diff --git a/pages/settings/PageSettingsConnectivity.qml b/pages/settings/PageSettingsConnectivity.qml
@@ -69,6 +69,7 @@ Page {
 
 			Column {
 				width: parent ? parent.width : 0
+				spacing: Theme.geometry_gradientList_spacing
 
 				Repeater {
 					model: canInterfaces.value || []

diff --git a/pages/settings/PageSettingsDisplayBrief.qml b/pages/settings/PageSettingsDisplayBrief.qml
@@ -47,19 +47,24 @@ Page {
 			}
 		}
 
-		footer: ListRadioButtonGroup {
-			//: Show percentage values in Brief view
-			//% "Brief view unit"
-			text: qsTrId("settings_briefview_unit")
-			optionModel: [
-				//% "No labels"
-				{ display: qsTrId("settings_briefview_unit_none"), value: VenusOS.BriefView_Unit_None },
-				//% "Show tank volumes"
-				{ display: qsTrId("settings_briefview_unit_absolute"), value: VenusOS.BriefView_Unit_Absolute },
-				//% "Show percentages"
-				{ display: qsTrId("settings_briefview_unit_percentages"), value: VenusOS.BriefView_Unit_Percentage },
-			]
-			dataItem.uid: Global.systemSettings.serviceUid + "/Settings/Gui/BriefView/Unit"
+		footer: Column {
+			width: parent.width
+			topPadding: Theme.geometry_gradientList_spacing
+
+			ListRadioButtonGroup {
+				//: Show percentage values in Brief view
+				//% "Brief view unit"
+				text: qsTrId("settings_briefview_unit")
+				optionModel: [
+					//% "No labels"
+					{ display: qsTrId("settings_briefview_unit_none"), value: VenusOS.BriefView_Unit_None },
+					//% "Show tank volumes"
+					{ display: qsTrId("settings_briefview_unit_absolute"), value: VenusOS.BriefView_Unit_Absolute },
+					//% "Show percentages"
+					{ display: qsTrId("settings_briefview_unit_percentages"), value: VenusOS.BriefView_Unit_Percentage },
+				]
+				dataItem.uid: Global.systemSettings.serviceUid + "/Settings/Gui/BriefView/Unit"
+			}
 		}
 	}
 }
diff --git a/pages/settings/PageSettingsDisplayMinMax.qml b/pages/settings/PageSettingsDisplayMinMax.qml
@@ -53,6 +53,7 @@ Page {
 
 			Column {
 				width: parent ? parent.width : 0
+				spacing: Theme.geometry_gradientList_spacing
 
 				Repeater {
 					id: acInputsRepeater
@@ -66,8 +67,9 @@ Page {
 						}
 
 						width: parent ? parent.width : 0
+						spacing: Theme.geometry_gradientList_spacing
 
-						SectionHeader {
+						SettingsListHeader {
 							text: {
 								const inputInfo = Global.acInputs["input" + (index + 1) + "Info"]
 								if (inputInfo.source === VenusOS.AcInputs_InputSource_NotAvailable) {
@@ -99,7 +101,7 @@ Page {
 				}
 			}
 
-			SectionHeader {
+			SettingsListHeader {
 				//% "DC input"
 				text: qsTrId("settings_minmax_dc_input")
 			}
@@ -111,7 +113,7 @@ Page {
 				unit: VenusOS.Units_Watt
 			}
 
-			SectionHeader {
+			SettingsListHeader {
 				//% "AC output"
 				text: qsTrId("settings_minmax_acout_max_power")
 			}
@@ -140,7 +142,7 @@ Page {
 				unit: VenusOS.Units_Amp
 			}
 
-			SectionHeader {
+			SettingsListHeader {
 				//% "DC output"
 				text: qsTrId("settings_minmax_dc_out")
 			}
@@ -152,7 +154,7 @@ Page {
 				unit: VenusOS.Units_Watt
 			}
 
-			SectionHeader {
+			SettingsListHeader {
 				//% "Solar"
 				text: qsTrId("settings_minmax_solar")
 			}

diff --git a/pages/settings/PageSettingsDisplayStartPage.qml b/pages/settings/PageSettingsDisplayStartPage.qml
@@ -105,6 +105,7 @@ Page {
 
 					Column {
 						width: parent ? parent.width : 0
+						spacing: Theme.geometry_gradientList_spacing
 
 						Repeater {
 							model: Global.systemSettings.startPageConfiguration.options

diff --git a/pages/settings/PageSettingsModbusDevices.qml b/pages/settings/PageSettingsModbusDevices.qml
@@ -42,6 +42,7 @@ Page {
 		model: VisibleItemModel {
 			Column {
 				width: parent ? parent.width : 0
+				spacing: Theme.geometry_gradientList_spacing
 
 				Repeater {
 					model: _devices.value ? _devices.value.split(',') : []

diff --git a/pages/settings/PageSettingsRvcDeviceConfiguration.qml b/pages/settings/PageSettingsRvcDeviceConfiguration.qml
@@ -71,6 +71,7 @@ Page {
 					model: 3
 					delegate: Column {
 						width: parent ? parent.width : 0
+						spacing: Theme.geometry_gradientList_spacing
 
 						ListSpinBox {
 							text: root._hasMultipleDcSources

diff --git a/pages/settings/PageSettingsWifi.qml b/pages/settings/PageSettingsWifi.qml
@@ -24,6 +24,8 @@ Page {
 
 		header: Column {
 			width: parent.width
+			spacing: Theme.geometry_gradientList_spacing
+
 			ListSwitch {
 				//% "Create access point"
 				text: qsTrId("settings_wifi_create_ap")
@@ -59,7 +61,7 @@ Page {
 				}
 			}
 
-			SectionHeader {
+			SettingsListHeader {
 				//% "Wi-Fi networks"
 				text: qsTrId("settings_wifi_networks")
 				preferredVisible: scanItem.isValid && accessPoint.isValid

diff --git a/pages/settings/PageTzInfo.qml b/pages/settings/PageTzInfo.qml
@@ -154,24 +154,30 @@ Page {
 						GradientListView {
 							id: tzListView
 
-							header: ListSwitch {
-								text: "UTC"
-								writeAccessLevel: VenusOS.User_AccessType_User
-								checked: tzData.city === text
-								onClicked: {
-									if (!checked) {
-										tzData.saveTimeZone("", text)
-										popTimer.start()
+							header: Column {
+								width: parent.width
+								bottomPadding: Theme.geometry_gradientList_spacing
+
+								ListSwitch {
+									text: "UTC"
+									writeAccessLevel: VenusOS.User_AccessType_User
+									checked: tzData.city === text
+									onClicked: {
+										if (!checked) {
+											tzData.saveTimeZone("", text)
+											popTimer.start()
+										}
 									}
-								}
 
+								}
 								Timer {
 									id: popTimer
 
 									interval: Theme.animation_settings_radioButtonPage_autoClose_duration
 									onTriggered: if (!!Global.pageManager) Global.pageManager.popPage(root)
 								}
 							}
+
 							model: root._timeZoneModels
 
 							delegate: ListRadioButtonGroup {

diff --git a/pages/settings/devicelist/DeviceListPage.qml b/pages/settings/devicelist/DeviceListPage.qml
@@ -10,6 +10,8 @@ Page {
 	id: root
 
 	GradientListView {
+		id: deviceListView
+
 		model: Global.allDevicesModel
 
 		delegate: Loader {
@@ -56,6 +58,9 @@ Page {
 
 		footer: Column {
 			width: parent.width
+			topPadding: Theme.geometry_gradientList_spacing
+			spacing: Theme.geometry_gradientList_spacing
+
 			ListButton {
 				//% "Remove disconnected devices"
 				text: qsTrId("devicelist_remove_disconnected_devices")