One of LiMux client's requirements is for display configuration to be easily accessible by mouse.
The OSD cannot be accessed by mouse, so this applet offers commonly used screen layouts in an easily accessible place.
To keep the screen on during a presentation (when the application does not do that or the user is actually demonstrating something on the machine itself) one needs to uncheck the non-userfriendly labeled "Enable powermanagement" check box in the battery monitor. Since this is also affects "screen setup", it is placed in this plasmoid as well.
The widget can be placed as an always-visible plasmoid in the panel or in System Tray where it would only show if presentation mode is enabled or more then one screen is connected.
Details
Updated layout:
Diff Detail
- Repository
- R104 KScreen
- Lint
Lint Skipped - Unit
Unit Tests Skipped
- Use "application enforce presentation mode" to communicate better that the checkbox might be overruled
On the first string I would change the wording:
"Configurations become enabled when another screen is connected"
The second string could read:
"It will prevent the screen from turning off" (automatically = optional)
This looks generally nice, I like it. Since we're short on people maintaining this stuff, I'd like more of the code to be shared though.
kded/daemon.h | ||
---|---|---|
66 | Could this be unified in some nice way with the above function (applyOsdAction) which does the same based on the enum...? Maybe just use the enum, or is there a reason for using a string here but not in other places? | |
plasmoid/kscreenapplet.cpp | ||
51 | What is the ownership of the GetConfigOperation? Is it deleted? (I don't know this code very well) | |
62 | This seems completely unused? Remove it maybe. | |
plasmoid/kscreenapplet.h | ||
43 | This enum is a duplicate of OsdAction::Action, and it's off by one. I don't think that's a good idea. Let's share the enum somehow (and move to enum class for new enums). | |
plasmoid/package/contents/ui/main.qml | ||
51 | This is completely duplicated from the OSD, is there no way to share the data instead? I know it doesn't have the "do nothing" action, but I'd rather see improvements in the OSD at the same time (which maybe could get rid of do nothing in favor of just disappearing when the user clicks somewhere else...). | |
plasmoid/package/metadata.desktop | ||
3 | We usually refer to the "monitors" as screen or display, this adds a third term ;) So I'd prefer to have either screen or display here. |
kded/daemon.h | ||
---|---|---|
66 | This thing is called by DBus, so I can only send a string over (or a random int, if you prefer that). I use QMetaEnum to map the string to an enum value to avoid having a duplicated list of strings. I don't think this can be optimized further. | |
plasmoid/kscreenapplet.cpp | ||
51 | GetConfigOperation self-deleteLaters when done |
Instead of:
Presentation mode This will prevent your screen and computer from turning off automatically [] Enable Presentation Mode
I would prefer to get rid of some redundancy and instead do this:
[] Presentation Mode This will prevent your screen and computer from turning off automatically
Alternatively, we could re-add the word "Enable" if we make it not a big bold header. In that case, we might also want to-rethink the big bold header for the screen layout buttons too.
plasmoid/package/contents/ui/main.qml | ||
---|---|---|
45 ↗ | (On Diff #39763) | Per https://hig.kde.org/style/writing/labels.html#using-ellipses-in-labels, if you want this to have ellipses, it has to start with an action verb (e.g. "Configure blabla..."). If not, it shouldn't have ellipses, because it's not an action, it's for navigation. |
The presentation mode doesn't really relate to the layouts that's why I put the headings in there. Just placing a checkbox there will make it drown underneath the layout buttons
Then please make a final layout image that I can change my work to, preferably not being *completely* different from what I have now … also, the headings are smaller than e.g. System Tray's "Status and Notifications" heading
@broulik, i'm pretty sure that it has a review that enable accessing OSD by mouse, can you investigate and ship it. I'm not against this but accessing OSD by mouse is pretty good feature after all.
@broulik Could we clarify the copyright?
plasmoid/kscreenapplet.h | ||
---|---|---|
4–5 ↗ | (On Diff #39763) | Hi Kai. Imo this copyright is fuzzy to me: Do you have the copyright here explicitly or is "the LiMuX project" also copyright holder? |