By default the mouse wheel action of switchdesktop always
wraps around/rolls over when switching virtual desktops.
Changed to read the corresponding kwinrc setting and behave
accordingly.
Also added the option to invert the direction of the mouse
wheel when switching.
Details
Details
- Reviewers
hein broulik davidedmundson - Group Reviewers
Plasma
Diff Detail
Diff Detail
- Repository
- R120 Plasma Workspace
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 3540 Build 3558: arc lint + arc unit
| containmentactions/switchdesktop/desktop.cpp | ||
|---|---|---|
| 36 | The rollover option is from another kde component (kwin), is there a way to get notified whenever this config changes? | |
Comment Actions
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.
| containmentactions/switchdesktop/desktop.cpp | ||
|---|---|---|
| 36 | I have this coming: | |
| 124 | Unforunately, that's not how our i18n works. a script greps for i18n("some text here") to generate the english translations. | |
| containmentactions/switchdesktop/desktop.h | ||
| 56 | 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. | |
Comment Actions
disabled checkbox for rollover
That was partially meant to overcome the shortcoming that changing the kwin value does not change this value until some settings dialog is opened, but with your patch this is no longer required.
Plus I wanted to avoid two options for the same thing and add some kind of hint where this behavior could be changed. Should I still mention it as plain text (though translating that might be complicated) or just leave users to figure it out themselves? Again, "go to first config gui of this key" would be nice :)
| containmentactions/switchdesktop/desktop.cpp | ||
|---|---|---|
| 36 | Great, will keep an eye on that. | |
| containmentactions/switchdesktop/desktop.h | ||
| 56 | Or, even simpler: I'll just drop the duplication between the in-class bool (i.e. the the cached value), and the m_options config bool and always look it up in m_options, should be fast enough. Btw, would a std::shared_ptr have been Ok for once? That does not have the bug prone non-explicit bool problem and would work as expected. | |
| containmentactions/switchdesktop/desktop.cpp | ||
|---|---|---|
| 156 | if you turn this into a method you don't need the rollover/invert args | |
| containmentactions/switchdesktop/desktop.h | ||
| 56 |
TBH, I still would have considered it overkill. It's only two checkboxes, it doesn't need any custom design patterns. If you did want to reduce the few lines of duplication, KConfigXT is the framework to look at for doing this sort of thing. | |
| containmentactions/switchdesktop/desktop.cpp | ||
|---|---|---|
| 47 | The m_kwinrcRollOverDesktops variable does not work at runtime because two instances of SwitchDesktop are created: One when plasma-workspace/KDE is started, a second when the settings window is opened. The actual desktop switching is done by the former, but subsequent changes to the kwinrc/RollOverDesktops setting are only seen and set by the latter. So the desktop rollover setting remains what it was when KDE was started. Is there a way to sync these without writing another config key? Or wait for "Add mechanism to notify other clients of config changes over DBus" / https://phabricator.kde.org/D13034 ? | |