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