filipf added a comment.
Tested it, works well, looks good. What I really like about this are the
extended time options and better wording of the options.
I added some minor comments in the code, mostly just related to my
understanding of the code, not any issues.
And while we're at this, could we also think about a different icon for the
General category? We use different icons for "General" all across different
configuration settings so it might not be bad to agree on one that we will use
consistently. I guess I have a problem with this specific one because I
associate it with "No preview available" :P
INLINE COMMENTS
> ConfigGeneral.qml:60
> + Layout.fillWidth: true
> + spacing: Kirigami.Units.largeSpacing / 2
> +
Do we need this? It ends up looking the same for me without it.
> ConfigGeneral.qml:72
> + id: hoursInterval
> + Layout.minimumWidth: textMetrics.width + Kirigami.Units.gridUnit
> + width: Kirigami.Units.gridUnit * 3
And do we need to set this? I removed `Layout.minimumWidth` and width for all
the `spinboxes`, everything looks the same and the numbers fit in nicely.
> ConfigGeneral.qml:73
> + Layout.minimumWidth: textMetrics.width + Kirigami.Units.gridUnit
> + width: Kirigami.Units.gridUnit * 3
> + value: root.hoursIntervalValue
Ditto.
> ConfigGeneral.qml:121
>
> - Label {
> - id: fillLabel
> - Layout.row: 1
> - Layout.column: 0
> - Layout.alignment: Qt.AlignRight
> - text: i18nc("@label:listbox", "Fill mode:")
> + ComboBox {
> + id: comboBox
QQC2 Combobox seems to be behaving well here. What I did in my task manager
port was add (or keep?) `Layout.fillWidth; true` to all of the comboboxes. But
because the first RowLayout is quite wide it would look odd here so perhaps
it's better to leave it as it is, right?
REPOSITORY
R114 Plasma Addons
REVISION DETAIL
https://phabricator.kde.org/D19548
To: ngraham, #vdg, #plasma
Cc: filipf, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
