[KCM] Make multi-screen draggability more obvious
ClosedPublic

Authored by ngraham on Oct 7 2019, 9:17 PM.

Details

Summary

Add a label and use a grabby hand cursor when hovering over a screen when there are
more than one.

BUG: 412303
FIXED-IN: 5.18.0

Test Plan

Diff Detail

Repository
R104 KScreen
Branch
make-draggability-more-obvious (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17421
Build 17439: arc lint + arc unit
ngraham created this revision.Oct 7 2019, 9:17 PM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 7 2019, 9:17 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Oct 7 2019, 9:17 PM
broulik added a subscriber: broulik.Oct 7 2019, 9:21 PM
broulik added inline comments.
kcm/package/contents/ui/Output.qml
272

Hm, can you still make the hand "close" when you press the button, which is what is typically done for dragging things

273

That won't update when outputs are added/removed, an invokable cannot be signalled

kcm/package/contents/ui/Screen.qml
59

i18n

ngraham marked an inline comment as done.Oct 7 2019, 9:24 PM
ngraham added inline comments.
kcm/package/contents/ui/Output.qml
272

I can do that if I accept the left button, but doing so interferes with the drag handler. I couldn;t figure out how to make these two play nicely together; assistance would be appreciated

273

Nah it totally works! :)

ngraham updated this revision to Diff 67464.Oct 7 2019, 9:24 PM

Translate

ngraham marked 2 inline comments as done.Oct 7 2019, 9:24 PM
broulik added inline comments.Oct 8 2019, 7:21 AM
kcm/package/contents/ui/Output.qml
273

This is against the laws of Physics.
I suspect it just erroring out (possibly rowCount requiring a QModelIndex argument) and just always staying visible. Or the KCM re-creating the outputModel which would be weird.

davidedmundson added inline comments.
kcm/package/contents/ui/Output.qml
273

Or the KCM re-creating the outputModel which would be weird.

It would, but:

connect (m_config.get(), &ConfigHandler::outputModelChanged,
         this, &KCMKScreen::outputModelChanged);

which gets emitted in

ConfigHandler::setConfig(KScreen::ConfigPtr config)

A new screen would be a new config.

I think output model is the same object, but it would retrigger this evaluation.

GB_2 added a subscriber: GB_2.EditedOct 8 2019, 2:26 PM

What about using a SizeAllCursor, like the screen edge button in the Plasma panel edit mode? The hand is usually used for scrolling.

ngraham updated this revision to Diff 67509.Oct 8 2019, 2:47 PM

Use Qt.SizeAllCursor

kcm/package/contents/ui/Output.qml
273

This still needs fixing. Regardless of whether it happens to work now or not.

kcm/package/contents/ui/Screen.qml
56

It's better to set the left + right anchors and then setting the text alignment to be horizontally centered.

That way we will wrap or elide if the translated string is too long rather than overflow.

ngraham updated this revision to Diff 68259.Oct 18 2019, 5:43 PM
ngraham marked an inline comment as done.

Use anchors correctly so the new label elides if necessary

ngraham marked 3 inline comments as done.Oct 18 2019, 5:47 PM
ngraham added inline comments.
kcm/package/contents/ui/Output.qml
273

Per discussion, we can do it this way for now since other pieces of code that are already committed do the same thing, and then we'll fix it for all of them in a follow-up patch.

romangg accepted this revision.Oct 21 2019, 2:10 PM

I'm not 100% happy with the font of the message in the overview. Maybe decrease opacity? But it's also not a pressing issue.

This revision is now accepted and ready to land.Oct 21 2019, 2:10 PM
ngraham marked an inline comment as done.Oct 21 2019, 2:18 PM

Better?

Not sure. Do what you feel is best and then commit. ;)

All right. :)

ngraham updated this revision to Diff 68440.Oct 21 2019, 2:22 PM

Reduce label opacity

This revision was automatically updated to reflect the committed changes.