[Differential] [Requested Changes To] D1771: [Workspace Options] Add option to disable OSD

[graesslin (Martin Gräßlin)]

graesslin requested changes to this revision.
graesslin added a reviewer: graesslin.
graesslin added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> mainpage.ui:60
> +     <property name="text">
> +      <string>Visual feedback for keyboard shortcuts:</string>
> +     </property>

The OSD will also be triggered for things like "disable touch pad when external mouse gets connected". So that wording seems wrong to me.

> workspaceoptions.cpp:55
>      connect(m_ui->showToolTips, SIGNAL(toggled(bool)), this, SLOT(changed()));
> +    connect(m_ui->showOsd, SIGNAL(toggled(bool)), this, SLOT(changed()));
>  }

new connect syntax? (yes I am aware that changed is both a slot and a signal here and needs to be casted)

> workspaceoptions.cpp:66
>  {
> +    KConfig config(QStringLiteral("plasmarc"));
> +

KSharedConfig::openConfig

> workspaceoptions.cpp:75
> +        KConfigGroup cg(&config, QStringLiteral("OSD"));
> +        cg.writeEntry("Enabled", m_ui->showOsd->isChecked());
> +    }

for the readEntry you use QStringLiteral

> workspaceoptions.cpp:90
> +    {
> +        KConfigGroup cg(&config, QStringLiteral("OSD"));
> +        m_ui->showOsd->setChecked(cg.readEntry(QStringLiteral("Enabled"), true));

QStringLiteral("OSD") is used twice in this file. Make it const static

> workspaceoptions.cpp:90
> +    {
> +        KConfigGroup cg(&config, QStringLiteral("OSD"));
> +        m_ui->showOsd->setChecked(cg.readEntry(QStringLiteral("Enabled"), true));

as you only read from the cg you can make it const

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: broulik, #plasma:_design, #plasma, graesslin
Cc: graesslin, plasma-devel, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160606/050e1971/attachment-0001.html>