D24224: Start of the accessibility KCM

Kevin Ottens noreply at phabricator.kde.org
Mon Nov 18 07:50:10 GMT 2019


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


  A few changes still needed.
  
  Also, in all the QML code we need to handle immutability of the settings (unfortunately that part can't be easily made magic like for the widgets). This would require for each control to have something like:
  `enabled: kcm.fooSettings.isImmutable("mySetting")`

INLINE COMMENTS

> kcmaccess.cpp:260
>  
> -void KAccessConfig::changeFlashScreenColor()
> -{
> -    ui.invertScreen->setChecked(false);
> -    ui.flashScreen->setChecked(true);
> -    configChanged();
> -}
> -
> -void KAccessConfig::selectSound()
> -{
> -    const QStringList list = QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, QStringLiteral("sound/"));
> -    QString start;
> -    if (!list.isEmpty())
> -        start = list[0];
> -    const QString fname = QFileDialog::getOpenFileName(this, QString(), start);
> -    if (!fname.isEmpty())
> -        ui.soundEdit->setText(fname);
> -}
> -
> -
> -void KAccessConfig::configChanged()
> -{
> -    emit changed(true);
> -}
> -
> -
>  void KAccessConfig::load()
>  {

I think this override can completely go away.

> kcmaccess.cpp:271
>  {
> -    KConfigGroup cg(KSharedConfig::openConfig(QStringLiteral("kaccessrc")), "Bell");
> -
> -    cg.writeEntry("SystemBell", ui.systemBell->isChecked());
> -    cg.writeEntry("ArtsBell", ui.customBell->isChecked());
> -    cg.writePathEntry("ArtsBellFile", ui.soundEdit->text());
> -
> -    cg.writeEntry("VisibleBell", ui.visibleBell->isChecked());
> -    cg.writeEntry("VisibleBellInvert", ui.invertScreen->isChecked());
> -    cg.writeEntry("VisibleBellColor", ui.colorButton->color());
> -
> -    cg.writeEntry("VisibleBellPause", ui.duration->value());
> -    cg.sync();
> -
> -    KConfigGroup keyboardGroup(KSharedConfig::openConfig(QStringLiteral("kaccessrc")), "Keyboard");
> -
> -    keyboardGroup.writeEntry("StickyKeys", ui.stickyKeys->isChecked());
> -    keyboardGroup.writeEntry("StickyKeysLatch", ui.stickyKeysLock->isChecked());
> -    keyboardGroup.writeEntry("StickyKeysAutoOff", ui.stickyKeysAutoOff->isChecked());
> -    keyboardGroup.writeEntry("StickyKeysBeep", ui.stickyKeysBeep->isChecked());
> -    keyboardGroup.writeEntry("ToggleKeysBeep", ui.toggleKeysBeep->isChecked());
> -    keyboardGroup.writeEntry("kNotifyModifiers", ui.kNotifyModifiers->isChecked());
> -
> -    keyboardGroup.writeEntry("SlowKeys", ui.slowKeys->isChecked());
> -    keyboardGroup.writeEntry("SlowKeysDelay", ui.slowKeysDelay->value());
> -    keyboardGroup.writeEntry("SlowKeysPressBeep", ui.slowKeysPressBeep->isChecked());
> -    keyboardGroup.writeEntry("SlowKeysAcceptBeep", ui.slowKeysAcceptBeep->isChecked());
> -    keyboardGroup.writeEntry("SlowKeysRejectBeep", ui.slowKeysRejectBeep->isChecked());
> -
> -
> -    keyboardGroup.writeEntry("BounceKeys", ui.bounceKeys->isChecked());
> -    keyboardGroup.writeEntry("BounceKeysDelay", ui.bounceKeysDelay->value());
> -    keyboardGroup.writeEntry("BounceKeysRejectBeep", ui.bounceKeysRejectBeep->isChecked());
> -
> -    keyboardGroup.writeEntry("Gestures", ui.gestures->isChecked());
> -    keyboardGroup.writeEntry("AccessXTimeout", ui.timeout->isChecked());
> -    keyboardGroup.writeEntry("AccessXTimeoutDelay", ui.timeoutDelay->value());
> -
> -    keyboardGroup.writeEntry("AccessXBeep", ui.accessxBeep->isChecked());
> -    keyboardGroup.writeEntry("GestureConfirmation", ui.gestureConfirmation->isChecked());
> -    keyboardGroup.writeEntry("kNotifyAccess", ui.kNotifyAccess->isChecked());
> -
> +    m_mouseSettings->save();
> +    m_keyboardSettings->save();

I don't think those calls to save are still needed. For sure you need to call the super class save() implementation though.

> bport wrote in kcmaccess.cpp:171
> new MouseSettings(this)
> 
> in order to avoid memory leak (same for other settings)

What @bport said, pass the parent to the ctor (will require adjusting the kcfgc files to get the corresponding ctors generated though).
Also it's generally better to do this kind of member initialization in the ctor member initializers list.

> kcmaccessibilitymouse.kcfg:34
> +    </entry>
> +    <entry name="MKCurve" type="Int">
> +      <label>Mouse Key Curve</label>

Wouldn't an Enum be more suited here?

> CMakeLists.txt:26
>      KF5::WindowSystem
> +    Qt5::X11Extras
>  )

Why is there still changes in colors?

REPOSITORY
  R119 Plasma Desktop

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

To: tcanabrava, ngraham, bport, ervin
Cc: broulik, bport, ervin, mart, ngraham, whiting, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20191118/db9bc180/attachment-0001.html>


More information about the Plasma-devel mailing list