systemdialog: Update to match plasma-workspace implementation, and simplify UI

SystemDialog was added in 2021 and did not receive any testing while the API was in flux when portals started using it.

Update the implementation to match plasma-workspace now that the API has settled.

Also simplify the UI, remove automatic scrollview and other loaders that make initialization very complicated.

Fixes https://invent.kde.org/plasma/plasma-mobile/-/issues/301

lookandfeel/contents/systemdialog/SystemDialog.qml

@@ -1,74 +1,35 @@
-/*
+// SPDX-FileCopyrightText: 2021-2024 Devin Lin <devin@kde.org>
- *  SPDX-FileCopyrightText: 2021 Devin Lin <espidev@gmail.com>
+// SPDX-License-Identifier: LGPL-2.0-or-later
- *
- *  SPDX-License-Identifier: LGPL-2.0-or-later
- */
 
-import QtQuick 2.15
+import QtQuick
-import QtQuick.Controls 2.15
+import QtQuick.Controls as QQC2
-import QtQuick.Templates as T
+import QtQuick.Layouts 
-import QtQuick.Layouts 1.15
+import QtQuick.Window
-import QtQuick.Window 2.15
 import Qt5Compat.GraphicalEffects
-import org.kde.kirigami 2.18 as Kirigami
+import QtQuick.Templates as T
-import "private" as Private
+import org.kde.kirigami as Kirigami
 
 Item {
     id: root
     
+    // -- public API: should match plasma-workspace implementation --
+
     default property Item mainItem
-
-    /**
-     * Title of the dialog.
-     */
     property string mainText: ""
-
-    /**
-     * Subtitle of the dialog.
-     */
     property string subtitle: ""
-
-    /**
-     * This property holds the default padding of the content.
-     */
-    property real padding: Kirigami.Units.smallSpacing
-
-    /**
-     * This property holds the left padding of the content. If not specified, it uses `padding`.
-     */
-    property real leftPadding: padding
-
-    /**
-     * This property holds the right padding of the content. If not specified, it uses `padding`.
-     */
-    property real rightPadding: padding
-
-    /**
-     * This property holds the top padding of the content. If not specified, it uses `padding`.
-     */
-    property real topPadding: padding
-
-    /**
-     * This property holds the bottom padding of the content. If not specified, it uses `padding`.
-     */
-    property real bottomPadding: padding
-    property alias standardButtons: footerButtonBox.standardButtons
-
-    readonly property int flags: Qt.FramelessWindowHint | Qt.Dialog
-    readonly property real dialogCornerRadius: Kirigami.Units.smallSpacing * 2
-    property list<T.Action> actions
     property string iconName
+    property list<T.Action> actions
+    readonly property alias dialogButtonBox: footerButtonBox
 
-    implicitWidth: loader.implicitWidth
+    required property Window window
-    implicitHeight: loader.implicitHeight
-
     readonly property real minimumHeight: implicitWidth
     readonly property real minimumWidth: implicitHeight
-
+    readonly property int flags: Qt.FramelessWindowHint | Qt.Dialog
-    required property Kirigami.AbstractApplicationWindow window
+    property var standardButtons // footerButtonBox standardButtons
+    readonly property int spacing: Kirigami.Units.gridUnit
     
     function present() {
-        window.showFullScreen()
+        window.showMaximized();
     }
 
     onWindowChanged: {
@@ -77,13 +38,9 @@ Item {
         }
     }
 
-    // load in async to speed up load times (especially on embedded devices)
+    Item {
-    Loader {
+        id: windowItem
-        id: loader
         anchors.centerIn: parent
-        asynchronous: true
-
-        sourceComponent: Item {
         // margins for shadow
         implicitWidth: Math.min(Screen.width, control.implicitWidth + 2 * Kirigami.Units.gridUnit)
         implicitHeight: Math.min(Screen.height, control.implicitHeight + 2 * Kirigami.Units.gridUnit)
@@ -101,75 +58,73 @@ Item {
         }
 
         // actual window
-            Control {
+        QQC2.Control {
             id: control
             anchors.fill: parent
             anchors.margins: glow.cornerRadius
-                topPadding: 0
+            topPadding: root.spacing
-                bottomPadding: 0
+            bottomPadding: root.spacing
-                rightPadding: 0
+            rightPadding: root.spacing
-                leftPadding: 0
+            leftPadding: root.spacing
+
+            implicitWidth: Kirigami.Units.gridUnit * 22
 
             background: Item {
                 Rectangle { // border
                     anchors.fill: parent
                     anchors.margins: -1
-                        radius: dialogCornerRadius + 1
+                    radius: Kirigami.Units.largeSpacing + 1
                     color: Qt.darker(Kirigami.Theme.backgroundColor, 1.5)
                 }
                 Rectangle { // background colour
                     anchors.fill: parent
-                        radius: dialogCornerRadius
+                    radius: Kirigami.Units.largeSpacing
                     color: Kirigami.Theme.backgroundColor
                 }
             }
 
-                contentItem: column
+            contentItem: ColumnLayout {
-            }
-        }
-    }
-
-    readonly property var contents: ColumnLayout {
                 id: column
                 spacing: 0
 
                 // header
-        Control {
-            id: headerControl
-
-            Layout.fillWidth: true
-            Layout.maximumWidth: root.window.maximumWidth
-
-            topPadding: 0
-            leftPadding: 0
-            rightPadding: 0
-            bottomPadding: 0
-
-            background: Item {}
-
-            contentItem: RowLayout {
                 Kirigami.Heading {
                     Layout.fillWidth: true
-                    Layout.topMargin: Kirigami.Units.largeSpacing
+                    Layout.maximumWidth: root.window.maximumWidth
-                    Layout.bottomMargin: Kirigami.Units.largeSpacing
+                    level: 3
-                    Layout.leftMargin: Kirigami.Units.largeSpacing
+                    font.weight: Font.Bold
-                    Layout.rightMargin: Kirigami.Units.largeSpacing
-                    Layout.alignment: Qt.AlignVCenter
-                    level: 2
                     text: root.mainText
                     wrapMode: Text.Wrap
                     elide: Text.ElideRight
                     horizontalAlignment: Text.AlignHCenter
                 }
+
+                QQC2.Label {
+                    Layout.topMargin: Kirigami.Units.largeSpacing
+                    Layout.bottomMargin: Kirigami.Units.largeSpacing
+                    Layout.maximumWidth: root.window.maximumWidth
+                    Layout.fillWidth: true
+                    text: root.subtitle
+                    visible: text.length > 0
+                    wrapMode: Text.Wrap
+                    horizontalAlignment: Text.AlignHCenter
                 }
+
+                Kirigami.Icon {
+                    Layout.topMargin: Kirigami.Units.largeSpacing
+                    Layout.alignment: Qt.AlignCenter
+                    source: root.iconName
+                    implicitWidth: Kirigami.Units.iconSizes.large
+                    implicitHeight: Kirigami.Units.iconSizes.large
                 }
 
                 // content
-        Control {
+                QQC2.Control {
                     id: content
 
                     Layout.fillHeight: true
                     Layout.fillWidth: true
+                    Layout.topMargin: Kirigami.Units.gridUnit
                     Layout.maximumWidth: root.window.maximumWidth
 
                     leftPadding: 0
@@ -177,150 +132,34 @@ Item {
                     topPadding: 0
                     bottomPadding: 0
 
-            background: Item {}
-            contentItem: ColumnLayout {
-                spacing: 0
-                clip: true
-
-                Label {
-                    id: subtitleLabel
-                    Layout.fillWidth: true
-                    Layout.topMargin: Kirigami.Units.largeSpacing
-                    Layout.bottomMargin: Kirigami.Units.largeSpacing
-                    Layout.leftMargin: Kirigami.Units.gridUnit * 3
-                    Layout.rightMargin: Kirigami.Units.gridUnit * 3
-                    visible: root.subtitle !== ""
-                    horizontalAlignment: Text.AlignHCenter
-                    text: root.subtitle
-                    wrapMode: Label.Wrap
-                }
-
-                // separator when scrolling
-                Kirigami.Separator {
-                    Layout.fillWidth: true
-                    opacity: root.mainItem && contentControl.flickableItem && contentControl.flickableItem.contentY !== 0 ? 1 : 0 // always maintain same height (as opposed to visible)
-                }
-
-                // mainItem is in scrollview, in case of overflow
-                Private.ScrollView {
-                    id: contentControl
-                    clip: true
-
-                    // we cannot have contentItem inside a sub control (allowing for content padding within the scroll area),
-                    // because if the contentItem is a Flickable (ex. ListView), the ScrollView needs it to be top level in order
-                    // to decorate it
                     contentItem: root.mainItem
-                    canFlickWithMouse: true
+                    background: Item {}
+                }
 
-                    // ensure window colour scheme, and background color
+                QQC2.DialogButtonBox {
-                    Kirigami.Theme.inherit: false
+                    id: footerButtonBox
-                    Kirigami.Theme.colorSet: Kirigami.Theme.Window
+                    // ensure we never have no buttons, we always must have the cancel button available
-
+                    standardButtons: (root.standardButtons === QQC2.DialogButtonBox.NoButton) ? QQC2.DialogButtonBox.Cancel : root.standardButtons
-                    // needs to explicitly be set for each side to work
-                    leftPadding: root.leftPadding; topPadding: root.topPadding
-                    rightPadding: root.rightPadding; bottomPadding: root.bottomPadding
-
-                    // height of everything else in the dialog other than the content
-                    property real otherHeights: headerControl.height + subtitleLabel.height + footerButtonBox.height + root.topPadding + root.bottomPadding;
-
-                    property real calculatedMaximumWidth: root.window.maximumWidth > root.absoluteMaximumWidth ? root.absoluteMaximumWidth : root.window.maximumWidth
-                    property real calculatedMaximumHeight: root.window.maximumHeight > root.absoluteMaximumHeight ? root.absoluteMaximumHeight : root.window.maximumHeight
-                    property real calculatedImplicitWidth: root.mainItem ? (root.mainItem.implicitWidth ? root.mainItem.implicitWidth : root.mainItem.width) + root.leftPadding + root.rightPadding : 0
-                    property real calculatedImplicitHeight: root.mainItem ? (root.mainItem.implicitHeight ? root.mainItem.implicitHeight : root.mainItem.height) + root.topPadding + root.bottomPadding : 0
-
-                    // don't enforce preferred width and height if not set
-                    Layout.preferredWidth: root.preferredWidth >= 0 ? root.preferredWidth : calculatedImplicitWidth + contentControl.rightSpacing
-                    Layout.preferredHeight: root.preferredHeight >= 0 ? root.preferredHeight - otherHeights : calculatedImplicitHeight + contentControl.bottomSpacing
                     
+                    Layout.topMargin: Kirigami.Units.largeSpacing
                     Layout.fillWidth: true
-                    Layout.fillHeight: true
+                    Layout.maximumWidth: root.window.maximumWidth
-                    Layout.maximumWidth: calculatedMaximumWidth
-                    Layout.maximumHeight: calculatedMaximumHeight >= otherHeights ? calculatedMaximumHeight - otherHeights : 0 // we enforce maximum height solely from the content
-
-                    // give an implied width and height to the contentItem so that features like word wrapping/eliding work
-                    // cannot placed directly in contentControl as a child, so we must use a property
-                    property var widthHint: Binding {
-                        target: root.mainItem
-                        property: "width"
-                        // we want to avoid horizontal scrolling, so we apply maximumWidth as a hint if necessary
-                        property real preferredWidthHint: contentControl.width - root.leftPadding - root.rightPadding - contentControl.rightSpacing
-                        property real maximumWidthHint: contentControl.calculatedMaximumWidth - root.leftPadding - root.rightPadding - contentControl.rightSpacing
-                        value: maximumWidthHint < preferredWidthHint ? maximumWidthHint : preferredWidthHint
-                    }
-                    property var heightHint: Binding {
-                        target: root.mainItem
-                        property: "height"
-                        // we are okay with overflow, if it exceeds maximumHeight we will allow scrolling
-                        value: contentControl.Layout.preferredHeight - root.topPadding - root.bottomPadding - contentControl.bottomSpacing
-                    }
-
-                    // give explicit warnings since the maximumHeight is ignored when negative, so developers aren't confused
-                    Component.onCompleted: {
-                        if (contentControl.Layout.maximumHeight < 0 || contentControl.Layout.maximumHeight === Infinity) {
-                            console.log("Dialog Warning: the calculated maximumHeight for the content is " + contentControl.Layout.maximumHeight + ", ignoring...");
-                        }
-                    }
-                }
-            }
-        }
-        Control {
-            Layout.fillWidth: true
-
                     leftPadding: 0
                     rightPadding: 0
                     topPadding: 0
                     bottomPadding: 0
-            contentItem: footerButtonBox
-        }
-    }
 
-    readonly property DialogButtonBox dialogButtonBox: DialogButtonBox {
-        //We want to report the same width on all so the button area is split equally
-        id: footerButtonBox
-        Layout.fillWidth: true
                     onAccepted: root.window.accept()
                     onRejected: root.window.reject()
-        implicitHeight: contentItem.implicitHeight
-        alignment: undefined
-
-        readonly property real sameWidth: 50
-        delegate: Private.MobileSystemDialogButton {
-            Layout.fillWidth: true
-            Layout.preferredWidth: footerButtonBox.sameWidth
-
-            readonly property point globalPos: root.window.visible ? mapToItem(footerButtonBox, Qt.point(x, y)) : Qt.point(0, 0)
-            verticalSeparator: globalPos.x > 0 && root.window.layout === Qt.Horizontal
-            horizontalSeparator: true
-            corners.bottomLeftRadius: verticalSeparator ? 0 : root.dialogCornerRadius
-            corners.bottomRightRadius: globalPos.x >= footerButtonBox.width ? root.dialogCornerRadius : 0
-        }
-
-        leftPadding: 0
-        rightPadding: 0
-        topPadding: 0
-        bottomPadding: 0
-
-        contentItem: GridLayout {
-            Layout.fillWidth: true
-
-            rowSpacing: 0
-            columnSpacing: 0
-            rows: root.window.layout === Qt.Vertical ? Number.MAX_VALUE : 1
-            columns: root.window.layout !== Qt.Vertical ? Number.MAX_VALUE : 1
 
                     Repeater {
                         model: root.actions
-                delegate: Private.MobileSystemDialogButton {
+                        delegate: QQC2.Button {
-                    Layout.fillWidth: true
-                    Layout.preferredWidth: footerButtonBox.sameWidth
-                    readonly property point globalPos: root.window.visible ? mapToItem(footerButtonBox, Qt.point(x, y)) : Qt.point(0, 0)
-                    verticalSeparator: globalPos.x > 0 && root.window.layout === Qt.Horizontal
-                    horizontalSeparator: true
-                    corners.bottomLeftRadius: model.index === root.actions.length - 1 ? root.dialogCornerRadius : 0
-                    corners.bottomRightRadius: model.index === root.actions.length - 1 && footerButtonBox.standardButtons === 0 ? root.dialogCornerRadius : 0
                             action: modelData
                         }
                     }
                 }
             }
+        }
+    }
 }

lookandfeel/contents/systemdialog/private/MobileSystemDialogButton.qml

@@ -1,83 +0,0 @@
-/*
- *  SPDX-FileCopyrightText: 2021 Devin Lin <espidev@gmail.com>
- *
- *  SPDX-License-Identifier: LGPL-2.0-or-later
- */
-
-import QtQuick 2.15
-import QtQuick.Controls 2.15
-import QtQuick.Layouts 1.15
-import QtQuick.Window 2.2
-import Qt5Compat.GraphicalEffects
-import org.kde.kirigami 2.18 as Kirigami
-
-AbstractButton {
-    id: root
-    
-    property alias corners: background.corners
-
-    property bool verticalSeparator: false
-    property bool horizontalSeparator: false
-
-    background: Kirigami.ShadowedRectangle {
-        id: background
-        Kirigami.Theme.colorSet: Kirigami.Theme.Button
-        Kirigami.Theme.inherit: false
-        color: {
-            if (root.down) {
-                let avg = (Kirigami.Theme.backgroundColor.r + Kirigami.Theme.backgroundColor.g + Kirigami.Theme.backgroundColor.b) / 3;
-                // sample down
-                avg = Math.max(0, (avg - 0.8) * 5);
-                return Qt.darker(Kirigami.Theme.backgroundColor, 1.1 + 0.4 * (1 - avg));
-            } else if (hoverHandler.hovered) {
-                return Qt.darker(Kirigami.Theme.backgroundColor, 1.05)
-            } else {
-                return Kirigami.Theme.backgroundColor
-            }
-        }
-
-        Kirigami.Separator {
-            anchors.left: parent.left
-            anchors.top: parent.top
-            anchors.bottom: parent.bottom
-            visible: root.verticalSeparator
-        }
-        Kirigami.Separator {
-            anchors.left: parent.left
-            anchors.right: parent.right
-            anchors.top: parent.top
-            visible: root.horizontalSeparator
-        }
-    }
-    
-    leftPadding: Kirigami.Units.largeSpacing
-    rightPadding: Kirigami.Units.largeSpacing
-    topPadding: Kirigami.Units.largeSpacing
-    bottomPadding: Kirigami.Units.largeSpacing
-    
-    contentItem: Item {
-        implicitHeight: row.height + Kirigami.Units.smallSpacing
-        implicitWidth: row.implicitWidth
-        RowLayout {
-            id: row
-            anchors.centerIn: parent
-            spacing: 0
-            
-            Kirigami.Icon {
-                Layout.preferredHeight: label.height
-                Layout.preferredWidth: height
-                Layout.rightMargin: Kirigami.Units.smallSpacing
-                visible: root.icon.name
-                source: root.icon.name
-            }
-            Label {
-                id: label
-                text: root.text
-            }
-        }
-        
-        HoverHandler {
-            id: hoverHandler
-        }
-    }
-}

lookandfeel/contents/systemdialog/private/ScrollView.qml

@@ -1,162 +0,0 @@
-/*
- *  SPDX-FileCopyrightText: 2016 Marco Martin <mart@kde.org>
- *
- *  SPDX-License-Identifier: LGPL-2.0-or-later
- */
-import QtQuick 2.7
-import QtQuick.Controls 2.0
-import QtQml 2.15
-import org.kde.kirigami 2.9 as Kirigami
-
-// taken from Kirigami
-MouseArea {
-    id: root
-    default property Item contentItem
-    property Flickable flickableItem
-    clip: true
-
-    property int horizontalScrollBarPolicy: Qt.ScrollBarAlwaysOff
-    property int verticalScrollBarPolicy: Qt.ScrollBarAsNeeded
-    property int topPadding: 0
-    property int leftPadding: 0
-    property int bottomPadding: 0
-    property int rightPadding: 0
-    property bool canFlickWithMouse: false
-
-    // Note: These are used because RefreshableScrollView overrides right and
-    // bottom padding properties.
-    property int rightSpacing: !Kirigami.Settings.hasTransientTouchInput && flickableItem && flickableItem.ScrollBar.vertical && flickableItem.ScrollBar.vertical.visible ? flickableItem.ScrollBar.vertical.width : 0
-    property int bottomSpacing: !Kirigami.Settings.hasTransientTouchInput && flickableItem && flickableItem.ScrollBar.horizontal && flickableItem.ScrollBar.horizontal.visible ? flickableItem.ScrollBar.horizontal.height : 0
-
-    Accessible.onScrollDownAction: flickableItem.Accessible.onScrollDownAction
-    Accessible.onScrollUpAction: flickableItem.Accessible.onScrollUpAction
-
-    Keys.onUpPressed: scroll(Kirigami.Units.gridUnit * 2)
-    Keys.onDownPressed: scroll(-Kirigami.Units.gridUnit * 2)
-
-    function scroll(y) {
-        // Don't scroll if the view isn't scrollable!
-        if (flickableItem.contentHeight < flickableItem.height + flickableItem.contentY) {
-            return;
-        }
-        const minYExtent = flickableItem.topMargin - flickableItem.originY;
-        const maxYExtent = flickableItem.height - (flickableItem.contentHeight + flickableItem.bottomMargin + flickableItem.originY);
-
-        flickableItem.contentY = Math.min(-maxYExtent, Math.max(-minYExtent, flickableItem.contentY - y));
-    }
-
-    focus: true
-
-    onPressed: mouse => { mouse.accepted = mouse.source !== Qt.MouseEventNotSynthesized; }
-    onVerticalScrollBarPolicyChanged: {
-        scrollBarCreationTimer.restart();
-    }
-    onHorizontalScrollBarPolicyChanged: {
-        scrollBarCreationTimer.restart();
-    }
-
-
-    onContentItemChanged: {
-        if (contentItem.hasOwnProperty("contentY")) {
-            flickableItem = contentItem;
-            if (typeof(flickableItem.keyNavigationEnabled) != "undefined") {
-                flickableItem.keyNavigationEnabled = true;
-                flickableItem.keyNavigationWraps = false;
-            }
-            contentItem.parent = flickableParent;
-        } else {
-            flickableItem = flickableComponent.createObject(flickableParent);
-            contentItem.parent = flickableItem.contentItem;
-        }
-
-        flickableItem.anchors.fill = flickableParent;
-
-        scrollBarCreationTimer.restart();
-    }
-
-    Binding {
-        when: !root.canFlickWithMouse
-        target: root.flickableItem
-        property: "interactive"
-        value: Kirigami.Settings.hasTransientTouchInput
-        restoreMode: Binding.RestoreBinding
-    }
-    Timer {
-        id: scrollBarCreationTimer
-        interval: 0
-        onTriggered: {
-            //create or destroy the vertical scrollbar
-            if ((!flickableItem.ScrollBar.vertical) &&
-                verticalScrollBarPolicy != Qt.ScrollBarAlwaysOff) {
-                flickableItem.ScrollBar.vertical = verticalScrollComponent.createObject(root);
-            } else if (flickableItem.ScrollBar.vertical &&
-                verticalScrollBarPolicy == Qt.ScrollBarAlwaysOff) {
-                flickableItem.ScrollBar.vertical.destroy();
-            }
-
-            //create or destroy the horizontal scrollbar
-            if ((!flickableItem.ScrollBar.horizontal) &&
-                horizontalScrollBarPolicy != Qt.ScrollBarAlwaysOff) {
-                flickableItem.ScrollBar.horizontal = horizontalScrollComponent.createObject(root);
-            } else if (flickableItem.ScrollBar.horizontal &&
-                horizontalScrollBarPolicy == Qt.ScrollBarAlwaysOff) {
-                flickableItem.ScrollBar.horizontal.destroy();
-            }
-        }
-    }
-    Kirigami.WheelHandler {
-        id: wheelHandler
-        target: root.flickableItem
-    }
-    Item {
-        id: flickableParent
-        clip: true
-        anchors {
-            fill: parent
-            leftMargin: root.leftPadding
-            topMargin: root.topPadding
-            rightMargin: root.rightPadding + root.rightSpacing
-            bottomMargin: root.bottomPadding + root.bottomSpacing
-        }
-    }
-    Component {
-        id: flickableComponent
-        Flickable {
-            anchors {
-                fill: parent
-            }
-            contentWidth: root.contentItem ? root.contentItem.width : 0
-            contentHeight: root.contentItem ? root.contentItem.height : 0
-        }
-    }
-    Component {
-        id: verticalScrollComponent
-        ScrollBar {
-            z: flickableParent.z + 1
-            visible: root.verticalScrollBarPolicy != Qt.ScrollBarAlwaysOff && root.contentItem.visible && size < 1
-            interactive: !Kirigami.Settings.hasTransientTouchInput
-
-            anchors {
-                right: parent.right
-                top: parent.top
-                bottom: parent.bottom
-                bottomMargin: root.bottomSpacing
-            }
-        }
-    }
-    Component {
-        id: horizontalScrollComponent
-        ScrollBar {
-            z: flickableParent.z + 1
-            visible: root.horizontalScrollBarPolicy != Qt.ScrollBarAlwaysOff && root.contentItem.visible && size < 1
-            interactive: !Kirigami.Settings.hasTransientTouchInput
-
-            anchors {
-                left: parent.left
-                right: parent.right
-                bottom: parent.bottom
-                rightMargin: root.rightSpacing
-            }
-        }
-    }
-}