D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham updated this revision to Diff 68440.
ngraham added a comment.


  Reduce label opacity

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24479?vs=68259=68440

BRANCH
  make-draggability-more-obvious (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24479

AFFECTED FILES
  kcm/package/contents/ui/Output.qml
  kcm/package/contents/ui/Screen.qml

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham added a comment.


  All right. :)

REPOSITORY
  R104 KScreen

BRANCH
  make-draggability-more-obvious (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Roman Gilg]

romangg added a comment.


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

REPOSITORY
  R104 KScreen

BRANCH
  make-draggability-more-obvious (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham marked an inline comment as done.
ngraham added a comment.


  Better?
  
  F7641631: Screenshot_20191021_081742.png 


REPOSITORY
  R104 KScreen

BRANCH
  make-draggability-more-obvious (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Roman Gilg]

romangg accepted this revision.
romangg added a comment.
This revision is now accepted and ready to land.


  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.

REPOSITORY
  R104 KScreen

BRANCH
  make-draggability-more-obvious (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham marked 3 inline comments as done.
ngraham added inline comments.

INLINE COMMENTS

> davidedmundson wrote in Output.qml:273
> This still needs fixing. Regardless of whether it happens to work now or not.

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.

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham updated this revision to Diff 68259.
ngraham marked an inline comment as done.
ngraham added a comment.


  Use anchors correctly so the new label elides if necessary

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24479?vs=67509=68259

BRANCH
  make-draggability-more-obvious (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24479

AFFECTED FILES
  kcm/package/contents/ui/Output.qml
  kcm/package/contents/ui/Screen.qml

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24479: [KCM] Make multi-screen draggability more obvious

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> davidedmundson wrote in Output.qml:273
> > Or the KCM re-creating the outputModel which would be weird.
> 
> It would, but:
> 
>   connect (m_config.get(), ::outputModelChanged,
>this, ::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.

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

> Screen.qml:56
> +top: parent.top
> +horizontalCenter: parent.horizontalCenter
> +margins: units.smallSpacing

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.

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham added a comment.


  Ping.

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham updated this revision to Diff 67509.
ngraham added a comment.


  Use Qt.SizeAllCursor

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24479?vs=67464=67509

BRANCH
  make-draggability-more-obvious (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24479

AFFECTED FILES
  kcm/package/contents/ui/Output.qml
  kcm/package/contents/ui/Screen.qml

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Björn Feber]

GB_2 added a comment.


  What about using a SizeAllCursor, like the screen edge button in the Plasma 
panel edit mode?

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: GB_2, davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D24479: [KCM] Make multi-screen draggability more obvious

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> broulik wrote in 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.

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

It would, but:

  connect (m_config.get(), ::outputModelChanged,
   this, ::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.

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: davidedmundson, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> ngraham wrote in Output.qml:273
> Nah it totally works! :)

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.

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham marked an inline comment as done.
ngraham added inline comments.

INLINE COMMENTS

> broulik wrote in 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

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

> broulik wrote in Output.qml:273
> That won't update when outputs are added/removed, an invokable cannot be 
> signalled

Nah it totally works! :)

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham updated this revision to Diff 67464.
ngraham added a comment.


  Translate

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24479?vs=67462=67464

BRANCH
  make-draggability-more-obvious (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24479

AFFECTED FILES
  kcm/package/contents/ui/Output.qml
  kcm/package/contents/ui/Screen.qml

To: ngraham, romangg, #plasma, #vdg
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> Output.qml:272
> +cursorShape: Qt.OpenHandCursor
> +acceptedButtons: Qt.NoButton // Otherwise it interferes with the 
> drag handler
> +visible: kcm.outputModel && kcm.outputModel.rowCount() > 1

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

> Output.qml:273
> +acceptedButtons: Qt.NoButton // Otherwise it interferes with the 
> drag handler
> +visible: kcm.outputModel && kcm.outputModel.rowCount() > 1
> +}

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

> Screen.qml:59
> +}
> +text: "Drag screens to re-arrange them"
> +visible: kcm.outputModel && kcm.outputModel.rowCount() > 1

`i18n`

REPOSITORY
  R104 KScreen

REVISION DETAIL
  https://phabricator.kde.org/D24479

To: ngraham, romangg, #plasma, #vdg
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D24479: [KCM] Make multi-screen draggability more obvious

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: romangg, Plasma, VDG.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION 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
  F7542106: grabby.png 

REPOSITORY
  R104 KScreen

BRANCH
  make-draggability-more-obvious (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24479

AFFECTED FILES
  kcm/package/contents/ui/Output.qml
  kcm/package/contents/ui/Screen.qml

To: ngraham, romangg, #plasma, #vdg
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart