D19548: [Media frame] Modernize and improve settings window

[Filip Fila]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190307/9860bf77/attachment.html>