D20549: Improve the look of the Plymouth Boot Splash Screen KCM UI

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> kcm.cpp:88
> +        m_newStuffDialog->winId(); // so it creates the windowHandle();
>          connect(m_newStuffDialog.data(), &KNS3::DownloadDialog::accepted, this,  &KCMPlymouth::load);
>          connect(m_newStuffDialog.data(), &KNS3::DownloadDialog::finished, m_newStuffDialog.data(),  &KNS3::DownloadDialog::deleteLater);

Ideally here we would reload the model and then set the newly installed theme as current

> kcm.cpp:157
> +    const auto list = dir.entryInfoList();
> +    for (const QFileInfo &fileInfo : list) {
> +        const QString pluginName = fileInfo.fileName();

I think the model population must be split from the KCM `load()` so you can reload the model without marking it as non-dirty.

Right now when installing a new theme, the Apply button doesn't enable and no theme is selected. Ideally it would select the newly installed theme.

Also needs a `emit selectedPluginIndexChanged();` after reloading the model as a model reset will cause `ListView` to forget its `currentIndex`

> kcm.cpp:210
>      if (!rc) {
> -        KMessageBox::error(nullptr, i18n("Unable to authenticate/execute the action: %1, %2", job->error(), job->errorString()));
> +        emit showErrorMessage(i18n("Unable to authenticate/execute the action: %1 (%2)", job->error(), job->errorString()));
> +        emit load();

In error handling we should probably check for `error() == KAuth::ActionReply::UserCancelledError` to not show a pointless error message when user canceled

> kcm.cpp:211
> +        emit showErrorMessage(i18n("Unable to authenticate/execute the action: %1 (%2)", job->error(), job->errorString()));
> +        emit load();
>      }

This isn't a signal, also why do you need to reload when saving? It's not like any themes could magically appear here?

> kcm.h:72
> +
> +    void beginSave();
> +    void finishSave();

I think this should be a property `Q_PROPERTY(bool saving...)` or more generic `busy` like in the other KCMs and then proper bindings in the UI

> main.qml:68
>  
> -                            Rectangle {
> -                                anchors {
> -                                    left: parent.left
> -                                    right: parent.right
> -                                    bottom: parent.bottom
> -                                }
> -                                height: childrenRect.height
> -                                gradient: Gradient {
> -                                    GradientStop {
> -                                        position: 0.0
> -                                        color: "transparent"
> -                                    }
> -                                    GradientStop {
> -                                        position: 1.0
> -                                        color: Qt.rgba(0, 0, 0, 0.5)
> -                                    }
> -                                }
> -                                QtControls.Label {
> -                                    anchors {
> -                                        horizontalCenter: parent.horizontalCenter
> -                                    }
> -                                    color: "white"
> -                                    text: model.display
> -                                }
> -                            }
> -                        }
> -                        Rectangle {
> -                            opacity: grid.currentIndex == index ? 1.0 : 0
> -                            anchors.fill: parent
> -                            border.width: units.smallSpacing * 2
> -                            border.color: syspal.highlight
> -                            color: "transparent"
> -                            Behavior on opacity {
> -                                PropertyAnimation {
> -                                    duration: units.longDuration
> -                                    easing.type: Easing.OutQuad
> -                                }
> -                            }
> -                        }
> -                        MouseArea {
> -                            anchors.fill: parent
> -                            hoverEnabled: true
> -                            onClicked: {
> -                                grid.currentIndex = index
> -                                kcm.selectedPlugin = model.pluginName
> -                            }
> -                            Timer {
> -                                interval: 1000 // FIXME TODO: Use platform value for tooltip activation delay.
> +        Kirigami.InlineMessage {
> +            id: infoLabel

In all the other KCMs the `InlineMessage` is above the controls to not push them around as it comes and goes

> main.qml:74
>  
> -                                running: parent.containsMouse && !parent.pressedButtons
> +        QtControls.ProgressBar {
> +            id: progressBar

I don't think this "progress" bar adds much value

REPOSITORY
  R258 Plymouth KCM

BRANCH
  improve-the-look-of-the-plymouth-boot-splash-screen-kcm-ui (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D20549

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: abetts, ngraham, broulik, #vdg, plasma-devel, #plasma, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190417/9f71a3b3/attachment-0001.html>