Default action when we plug unknown monitor
Details
Diff Detail
- Repository
- R104 KScreen
- Branch
- customize_connect_unknown_monitor
- Lint
No Linters Available - Unit
No Unit Test Coverage
Unless I'm misreading things, I think there's one part that's going to appear very confusing.
I have my (small) laptop. Lets say I want any new (big) screen to appear on the left. So I save "Extend to left".
So I'd expect to see: monitor, laptop.
What will happen, is it will extend my biggest screen to the left, leaving me with:
laptop, monitor
rendering the option seemingly broken.
kded/generator.cpp | ||
---|---|---|
550 | Please don't use qforeach in new code as Qt might deprecate it. |
Once you're done with the code, please also add screenshot so usability can have a look at it. I'm not sure we should add UI for this, the screen setup is already very complex and hard to understand. Perhaps a set of clear use-cases would make this clearer?
No unit tests for the code (I know, hard to do) will add to the maintenance burden, which is already high.
Just seeing the code is definitely not enough for me to ACK this.
Ow, I might add that we wanted to address a similar thing by providing a popup when a display is connected that asks the user what do do now, and then an action can be chosen. Maybe your time is spent better working on that feature, as we already agreed that this would make sense.
(I was in vacation.)
Ok so I need to create an osd plasma for it.
I can trash this current patch.
@sebas: What do you want as widget design ?
Is it ok to add plasma dependancy in kscreen ?
I left a comment a month ago. It has not been addressed.
there's one part that's going to appear very confusing.
I have my (small) laptop. Lets say I want any new (big) screen to appear on the left. So I save "Extend to left".
So I'd expect to see: monitor, laptop.
What will happen, is it will extend my biggest screen to the left, leaving me with:
laptop, monitor
rendering the option seemingly broken.
It's artbitrary depending on what size monitor you happen to plug in.
I agree that this is broken, functionality-wise. But it was broken before too, right? So why was the old code looking up the biggest monitor and extending it, instead of using the primary one?
kded/generator.cpp | ||
---|---|---|
328 | this cleanup should happen in a separate patch |
It was semi broken. Behavior was arbitrary on size, but there's no ui pretending it's anything else.
I'd be happy with this patch if it extended from an in-built monitor (you can't use primary here as we're the ones setting that) instead of biggest.
@sebas suggested we approach it as an OSD that would appear on screen and allow the user to select an action they want to take. I think it's a good approach, even more flexible than an option in the KCM. And IIRC it was also what we planned long time ago when I was working on KScreen, but never got around to implement it :-).
Now the question is how should we approach it code-wise: should we write our own OSD in the KScreen KDED module, or should we extend the Plasmashell OSD code and expose it on the org.kde.osdService interface and only have KScreen interact with it via DBus? What would you guys prefer?
@dvratil Good question. I actually have a branch that adds an OSD to kscreen. It's in sebas/osd right now and I haven't merged it since it behaves weirdly on Wayland (placement is not reliable). That should be the start. The plasmashell OSD is doesn't allow placing it on a specific screen (or both) which is something we'll want for this, since we're messing with screens right during that stage, and it's also not interactive. So we'd have to add quite some cases to plasmashell that are really only useful for kscreen. I'd start with that OSD and add interactivity and the selector for layouts.
@mlaurent The idea is to popup a selector for the different layouts when a new screen is connected and then switch to the chosen layout. It will be remembered so on next connect, this layout will be picked automatically. The OSD should resemble the other Plasma OSDs, for a detailed design, please refer to @jensreuterberg or or other VDG members. There are already icons added to Breeze for that, and enums are in libkscreen as well (IIRC). This adds a runtime dependency on kscreen (not libkscreen), which is entirely fine. Hit me up on IRC or email for additional details.