| bshah | |
| romangg |
| Plasma |
lockAutoRotate(true) would enable the automatic rotation and
lockAutoRotate(false) would disable auto rotation which is the opposite
of what the function name would imply, so rename it to setAutoRotate.
We also need a getter for the value to display the status in the UI.
This getter checks if all applicable outputs have auto rotate enabled
and returns that value.
tested on Plasma Mobile
| No Linters Available |
| No Unit Test Coverage |
| Buildable 25153 | |
| Build 25171: arc lint + arc unit |
I'm not sure about the renaming. the lock... implies that the rotation stays the way it is at the moment (what it does). But I'm fine either way.
These are normally two separate commits, one refactor and one feature addition. But since it's only a small change and Phabricator sucks let's not bitch too much about it.
@bshah: ok with the change?
| kded/config.cpp | ||
|---|---|---|
| 104 ↗ | (On Diff #79930) | Should we check if all or only one panel display are set to auto rotate. Granted I'm not aware of a device with multiple panel displays which runs KScreen but maybe you had a reason for using && here? Just want to hear a rational if you had one. If you didn't and just picked && by random it's also fine. |
| kded/config.cpp | ||
|---|---|---|
| 99 ↗ | (On Diff #79930) | I wasn't aware that C++ had such fancy methods :) I'm using equivalent functions a lot in Kotlin but they are honestly much nicer to use in Kotlin than in C++. In this case instead of the "continue" keyword I would have to return true then I guess. |
| 104 ↗ | (On Diff #79930) | I thought it makes more sense to say "auto rotate is enabled when all panels have auto rotate enabled" rather than "auto rotate is enabled when at least one panel has auto rotate enabled" which is why I've chosen to do it this way. |
LGTM, Please wait for @romangg to approve.
I am fine with behavior change of the,
| kded/config.cpp | ||
|---|---|---|
| 99 | extract m_data->outputs() into a variable. | |
| kded/config.cpp | ||
|---|---|---|
| 99 ↗ | (On Diff #79930) | const auto |