D26041: Move applet configuration to KCM
Nathaniel Graham
noreply at phabricator.kde.org
Mon Dec 16 22:28:37 GMT 2019
ngraham added a comment.
Lovely. Works great, and much better UI. I have just a few UI and code suggestions:
INLINE COMMENTS
> ConfigurationDialog.qml:23
> +import QtQuick.Dialogs 1.2
> +import QtQuick.Controls 2.5 as QtControls
> +import org.kde.kirigami 2.5 as Kirigami
`as QQC2`
> ConfigurationDialog.qml:38
> + implicitHeight: 200
> + implicitWidth: 300
> +
A bit wider would be better so the window's title (which is quite long) doesn't get elided
> ConfigurationDialog.qml:48
> + Kirigami.FormLayout {
> + anchors.left: parent.left
> + anchors.right: parent.right
a top padding of one gridUnit might make this look nicer
> ConfigurationDialog.qml:55
> + onClicked: okButton.enabled = true
> + Component.onCompleted: checked = configuration.unlockModemOnDetection
> + }
this shouldn't need to be in `Component.onCompleted:`; just set `checked` directly as a binding: `checked: configuration.unlockModemOnDetection`
> ConfigurationDialog.qml:62
> + onClicked: okButton.enabled = true
> + Component.onCompleted: checked = configuration.manageVirtualConnections
> + }
ditto
> ConfigurationDialog.qml:71
> + right: parent.right
> + margins: Math.round(units.gridUnit / 2)
> + }
use `units.smallSpacing` instead
> ConfigurationDialog.qml:73
> + }
> + spacing: Math.round(units.gridUnit / 2)
> +
ditto
> main.qml:196
> + left: parent.left
> + margins: Math.round(units.gridUnit / 3)
> + }
`units.smallSpacing`
> main.qml:198
> + }
> + spacing: Math.round(units.gridUnit / 2)
> +
ditto
> main.qml:205
> +
> + QQC2.ToolTip {
> + text: i18n("Configuration")
Use the attached ToolTip property instead of creating a whole object; it's lighter that way
REPOSITORY
R116 Plasma Network Management Applet
REVISION DETAIL
https://phabricator.kde.org/D26041
To: jgrulich, ngraham, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20191216/59194d58/attachment-0001.html>
More information about the Plasma-devel
mailing list