Settings: Add 'Opportunity Loads' page #2782
Conversation
DanielMcInnes
force-pushed
the
2556-settings-add-a-sortable-priority-list
branch
from
401b395 to
366d4a3
Compare
|
|
||
| Item { | ||
| id: root | ||
|
|
There was a problem hiding this comment.
Could you add a new JSON file for an opportunityloads service to data/mock/conf/services/ and add the file to data/mock/conf/maximal.json? Then we can easily choose whether to load the service for different configurations.
| pageSource: "/pages/settings/PageControllableLoads.qml" | ||
|
|
||
| VeQuickItem { | ||
| id: _mode |
There was a problem hiding this comment.
Use a more specific id like opportunityLoadsMode in this context?
| //% "Limiting the maximum charging power can improve simultaneity with other controllable devices." | ||
| text: qsTrId("pagecontrollableloads_limiting_the_maximum") | ||
| font.pixelSize: Theme.font_size_tiny | ||
| width: 528 |
There was a problem hiding this comment.
Maybe can be a caption within the ListQuantityField, instead of creating a separate SettingsListHeader?
| font.pixelSize: Theme.font_size_body2 | ||
| wrapMode: Text.Wrap | ||
| text: serviceType === "battery" | ||
| ? "Battery" |
There was a problem hiding this comment.
"Battery" needs translation. Also, it looks like this expression can be simplified to:
text: serviceType === "battery" ? CommonWords.battery : longName || device?.name || uniqueIdentifier || ""
|
|
||
| FilteredDeviceModel { | ||
| id: acLoadDevices | ||
| objectName: "PageControllableLoads.acLoadDevices" |
There was a problem hiding this comment.
Are the objectNames from leftover debug?
| component Arrow: ListItemButton { | ||
| icon.source: "qrc:/images/icon_arrow.svg" | ||
| flat: false | ||
| height: parent.height - 2 * Theme.geometry_opportunityLoad_margin |
There was a problem hiding this comment.
The button in the design is like this:
but the one here is much wider:
| //% "Arrange the controllable devices according to their priority; the control algorithm will control them based on the currently available PV excess." | ||
| text: qsTrId("pagecontrollableloads_arrange") | ||
| font.pixelSize: Theme.font_size_tiny | ||
| width: 528 |
There was a problem hiding this comment.
Hardcoded width? Also should the font size is too small, maybe font_size_caption instead?
| for (var i = 0; i < opportunityLoadsModel.count; ++i) { | ||
| newValue.push(opportunityLoadsModel.get(i)) | ||
| } | ||
|
|
There was a problem hiding this comment.
Unnecessary newline; also, use let instead of var.
| SettingsListHeader { | ||
| //% "This helps the BatteryLife algorithm recharge the battery to 100%." | ||
| text: qsTrId("pagecontrollableloads_battery_this_supports_the_batterylife_algorithm") | ||
| font.pixelSize: Theme.font_size_tiny |
There was a problem hiding this comment.
Maybe can be a caption within the ListSwitch, instead of creating a separate SettingsListHeader?
| for (var i = 0; jsv && i < jsv.length; ++i) { | ||
| const newValue = jsv[i] | ||
| if ((opportunityLoadsModel.count < (i + 1)) || (newValue !== opportunityLoadsModel.get(i))) | ||
| opportunityLoadsModel.set(i, jsv[i]) |
There was a problem hiding this comment.
Style: indent and add braces around this block
Fixes #2556