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
