D14436: SwitchDesktop mousewheel options with config dialog added

[David Edmundson]

davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  Showing a disabled checkbox for rollover doesn't make much sense.
  IMHO we should either have it as a separate option from kwin and have a checkbox, or not have a checkbox at all.

INLINE COMMENTS

> totto wrote in desktop.cpp:36
> The rollover option is from another kde component (kwin), is there a way to get notified whenever this config changes?

I have this coming: 
https://phabricator.kde.org/D13034

> desktop.cpp:122
> +        QCheckBox *item = new QCheckBox(widget);
> +        item->setText(i18nc(e.cfgKey.toStdString().c_str(),
> +                            e.name.toStdString().c_str()));

Unforunately, that's not how our i18n works.

a script greps for i18n("some text here") to generate the english translations.

> desktop.h:54
>  
> +        // QSharedPtr has non-explicit operator bool which is bugprone, work around that
> +        struct explicitBool { bool val; };

or just use a regular bool and copy the value in configurationAccepted

Long term, having simpler code tends to be much better than clever code.

REPOSITORY
  R120 Plasma Workspace

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

To: totto, hein, broulik, #plasma, davidedmundson
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180729/51382546/attachment.html>