D6216: simplify positioning code

2017-06-21 Thread Marco Martin
This revision was automatically updated to reflect the committed changes. Closed by commit R120:415b6d81a696: simplify positioning code (authored by mart). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6216?vs=15578&id=15683 REVISION DETAIL https:

D6216: simplify positioning code

2017-06-21 Thread David Edmundson
davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R120 Plasma Workspace BRANCH arcpatch-D6216 REVISION DETAIL https://phabricator.kde.org/D6216 To: mart, #plasma, davidedmundson Cc: apol, davidedmundson, plasma-devel, ZrenBot, spstarr, prog

D6216: simplify positioning code

2017-06-21 Thread Aleix Pol Gonzalez
apol added a comment. Can we maybe include a test? Then it would be much safer to do this kind of changes. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D6216 To: mart, #plasma, davidedmundson Cc: apol, davidedmundson, plasma-devel, ZrenBot, spstarr, progw

D6216: simplify positioning code

2017-06-21 Thread Marco Martin
mart added a comment. ping? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D6216 To: mart, #plasma, davidedmundson Cc: davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas

D6216: simplify positioning code

2017-06-19 Thread Marco Martin
mart added inline comments. INLINE COMMENTS > davidedmundson wrote in NotificationItem.qml:33 > If in https://phabricator.kde.org/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?

D6216: simplify positioning code

2017-06-19 Thread David Edmundson
davidedmundson requested changes to this revision. davidedmundson added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > NotificationItem.qml:33 > width: parent.width > -implicitHeight: Math.max(appIconItem.visible || imageItem.visible ? > units.iconSize

D6216: simplify positioning code

2017-06-19 Thread Marco Martin
mart updated this revision to Diff 15578. mart added a comment. - make sure Dialog::setHeight is used disable height binding when notifications are repopulated REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6216?vs=15568&id=15578 BRANCH arc

D6216: simplify positioning code

2017-06-19 Thread Marco Martin
mart updated this revision to Diff 15568. mart added a comment. - don't depend from visibility REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6216?vs=15464&id=15568 BRANCH arcpatch-D6216 REVISION DETAIL https://phabricator.kde.org/D6216 AFFE

D6216: simplify positioning code

2017-06-15 Thread David Edmundson
davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R120 Plasma Workspace BRANCH master REVISION DETAIL https://phabricator.kde.org/D6216 To: mart, #plasma, davidedmundson Cc: davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliez

D6216: simplify positioning code

2017-06-15 Thread Marco Martin
mart updated this revision to Diff 15464. mart added a comment. use dialog size instead of its mainitem's REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6216?vs=15463&id=15464 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6216 A

D6216: simplify positioning code

2017-06-15 Thread Marco Martin
mart added inline comments. INLINE COMMENTS > davidedmundson wrote in notificationshelper.cpp:294 > 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 > resize

D6216: simplify positioning code

2017-06-15 Thread David Edmundson
davidedmundson added a comment. Mostly seems sensible, mine doesn't animate as-is anyway. Just one question. INLINE COMMENTS > notificationshelper.cpp:294 > } else { > -int posY = m_plasmoidScreen.bottom() - cumulativeHeight - > m_popupsOnScreen[i]->contentItem()->height

D6216: simplify positioning code

2017-06-15 Thread Marco Martin
mart edited the summary of this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D6216 To: mart, #plasma Cc: plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas

D6216: simplify positioning code

2017-06-15 Thread Marco Martin
mart updated this revision to Diff 15463. mart added a comment. just rely on visibleChanged, abouttoshow dropped REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6216?vs=15430&id=15463 BRANCH master REVISION DETAIL https://phabricator.kde.org/D

D6216: simplify positioning code

2017-06-13 Thread Marco Martin
mart created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY drop most of the positioning code, drop the animation of the window position. use the newly introduced aboutToShow() signal and position the p