broulik added a comment.
Minor nitpicks but otherwise looking good
INLINE COMMENTS
> ConfigOverlay.qml:360
> enabled: currentApplet
> - width: handleRow.childrenRect.width + (2 * handleRow.spacing)
> - height: Math.max(configureButton.height, label.contentHeight,
> closeButton.height)
> + width: Math.max(handleButtons.width, label.width)
> + height: handleButtons.height
The `label` is *inside* the `ColumnLayout` so just doing `handleButtons.width`
should suffice.
> ConfigOverlay.qml:368
> + id: handleButtons
> anchors.horizontalCenter: parent.horizontalCenter
> spacing: units.smallSpacing
Could be removed now that the dialog has the exact size of the column,
previously it added `2 * spacing`
> ConfigOverlay.qml:372
> + PlasmaExtras.Heading {
> + id: label
> + level: 3
Can be removed once you addressed the `width` above
> ConfigOverlay.qml:375
> + Layout.fillWidth: true
> + Layout.leftMargin: units.smallSpacing
> + Layout.rightMargin: units.smallSpacing
This doesn't seem to do anything? (You might want to use `leftPadding` and the
like but I recall that messing up the width calculation of the label/column)
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D14145
To: ngraham, #plasma
Cc: gregormi, abetts, broulik, plasma-devel, ragreen, Pitel, ZrenBot,
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
