D14855: Add applet with screen layouts and presentation mode

[Frederik Gladhorn]

gladhorn added a comment.


  This looks generally nice, I like it. Since we're short on people maintaining this stuff, I'd like more of the code to be shared though.

INLINE COMMENTS

> daemon.h:66
> +        // DBus
> +        void applyLayoutPreset(const QString &presetName);
>      Q_SIGNALS:

Could this be unified in some nice way with the above function (applyOsdAction) which does the same based on the enum...? Maybe just use the enum, or is there a reason for using a string here but not in other places?

> kscreenapplet.cpp:51
> +
> +    connect(new KScreen::GetConfigOperation(KScreen::GetConfigOperation::NoEDID), &KScreen::ConfigOperation::finished,
> +            this, [this](KScreen::ConfigOperation *op) {

What is the ownership of the GetConfigOperation? Is it deleted? (I don't know this code very well)

> kscreenapplet.cpp:62
> +
> +void KScreenApplet::configChanged()
> +{

This seems completely unused? Remove it maybe.

> kscreenapplet.h:43
> +
> +    enum Action {
> +        SwitchToExternal,

This enum is a duplicate of OsdAction::Action, and it's off by one. I don't think that's a good idea. Let's share the enum somehow (and move to enum class for new enums).

> main.qml:51
> +
> +    readonly property var screenLayouts: [
> +        {

This is completely duplicated from the OSD, is there no way to share the data instead? I know it doesn't have the "do nothing" action, but I'd rather see improvements in the OSD at the same time (which maybe could get rid of do nothing in favor of just disappearing when the user clicks somewhere else...).

> metadata.desktop:3
> +Name=Display Configuration
> +Comment=Quickly switch between monitor layouts and presentation mode
> +Icon=preferences-desktop-display

We usually refer to the "monitors" as screen or display, this adds a third term ;) So I'd prefer to have either screen or display here.

REPOSITORY
  R104 KScreen

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

To: broulik, #plasma, #vdg, fischbach, harmathy
Cc: gladhorn, abetts, plasma-devel, 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/20180817/f5e7e707/attachment.html>