simplify positioning code

Authored by mart on Jun 13 2017, 4:37 PM.



drop most of the positioning code, drop the animation of the
window position. position the popup in the signal handler of
visibleChanged as we are sure there the size is final and the
first expose event didn't arrive yet.
at that point we are sure the size is the final and correct one

the animation is supposed to be done by the morphingpopups effect

Test Plan

notification positions are always correct now, both on X11 and
Unfortunately on x11 the notification typ doesn't seem to pass,
so they are correctly animated only in wayland

Diff Detail

R120 Plasma Workspace
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.Jun 13 2017, 4:37 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJun 13 2017, 4:37 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart updated this revision to Diff 15463.Jun 15 2017, 2:41 PM

just rely on visibleChanged, abouttoshow dropped

mart edited the summary of this revision. (Show Details)Jun 15 2017, 2:46 PM

Mostly seems sensible, mine doesn't animate as-is anyway. Just one question.


I find it odd that we use the contentItem height not the window height which includes the frame margins.

Especially given that whole other patch is about making sure the window is resized to the contentItem.

mart added inline comments.Jun 15 2017, 2:48 PM

didn't change that, giving a try to it, may simplify things further

mart updated this revision to Diff 15464.Jun 15 2017, 2:52 PM

use dialog size instead of its mainitem's

davidedmundson accepted this revision.Jun 15 2017, 2:53 PM
This revision is now accepted and ready to land.Jun 15 2017, 2:53 PM
mart updated this revision to Diff 15568.Jun 19 2017, 9:57 AM
  • don't depend from visibility
mart updated this revision to Diff 15578.Jun 19 2017, 12:51 PM
  • make sure Dialog::setHeight is used

disable height binding when notifications are repopulated

davidedmundson requested changes to this revision.Jun 19 2017, 1:12 PM
davidedmundson added inline comments.

If in D6215 you've got a mainItem->setVisible() before the showEvent because that fixes what this is doing.

If that's the case, why do we still need these changes?


So instead of making this inhibitor and changing a bunch of code

just convert:



list = otherList

This revision now requires changes to proceed.Jun 19 2017, 1:12 PM
mart added inline comments.Jun 19 2017, 1:23 PM

not strictly need, but they should trigger less resizes anyways when the dialogs are shown i think, even if they happen before the window is actually visible


can't do that, as notificationItem.actions is a ListModel and notificationsProperties is a javascript array instead

apol added a subscriber: apol.Jun 21 2017, 10:11 AM

Can we maybe include a test? Then it would be much safer to do this kind of changes.

davidedmundson accepted this revision.Jun 21 2017, 11:15 AM
This revision is now accepted and ready to land.Jun 21 2017, 11:15 AM
This revision was automatically updated to reflect the committed changes.