Members of the KDE Community are recommended to subscribe to the kde-community mailing list at https://mail.kde.org/mailman/listinfo/kde-community to allow them to participate in important discussions and receive other important announcements

Commit 415b6d81 authored by Marco Martin's avatar Marco Martin

simplify positioning code

Summary:
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
instead.

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

Reviewers: #plasma, davidedmundson

Reviewed By: #plasma, davidedmundson

Subscribers: apol, davidedmundson, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D6216
parent 295af397
......@@ -30,7 +30,7 @@ import org.kde.kquickcontrolsaddons 2.0
MouseArea {
id: notificationItem
width: parent.width
implicitHeight: Math.max(appIconItem.visible || imageItem.visible ? units.iconSizes.large : 0, mainLayout.height)
implicitHeight: Math.max(appIconItem.valid || imageItem.nativeWidth > 0 ? units.iconSizes.large : 0, mainLayout.height)
// We need to clip here because we support displaying images through <img/>
// and if we don't clip, they will be painted over the borders of the dialog/item
......@@ -153,7 +153,7 @@ MouseArea {
left: parent.left
}
visible: !imageItem.visible && valid
visible: imageItem.nativeWidth == 0 && valid
animated: false
}
......@@ -170,7 +170,7 @@ MouseArea {
anchors {
top: parent.top
left: appIconItem.visible || imageItem.visible ? appIconItem.right : parent.left
left: appIconItem.valid || imageItem.nativeWidth > 0 ? appIconItem.right : parent.left
right: parent.right
leftMargin: units.smallSpacing
}
......@@ -244,7 +244,7 @@ MouseArea {
// If there is a big notification followed by a small one, the height
// of the popup does not always shrink back, so this forces it to
// height=0 when those are invisible. -1 means "default to implicitHeight"
Layout.maximumHeight: bodyText.visible || actionsColumn.visible ? -1 : 0
Layout.maximumHeight: bodyText.length > 0 || notificationItem.actions.count > 0 ? -1 : 0
PlasmaExtras.ScrollArea {
id: bodyTextScrollArea
......
......@@ -53,11 +53,14 @@ PlasmaCore.Dialog {
function populatePopup(notification) {
notificationProperties = notification
notificationTimer.interval = notification.expireTimeout
notificationTimer.restart()
notificationTimer.restart();
//temporarly disable height binding, avoids an useless window resize when removing the old actions
heightBinding.when = false;
// notification.actions is a JS array, but we can easily append that to our model
notificationItem.actions.clear()
notificationItem.actions.append(notificationProperties.actions)
//enable height binding again, finally do the resize
heightBinding.when = true;
}
function clearPopup() {
......@@ -65,13 +68,6 @@ PlasmaCore.Dialog {
notificationItem.actions.clear()
}
Behavior on y {
NumberAnimation {
duration: units.longDuration
easing.type: Easing.OutQuad
}
}
mainItem: NotificationItem {
id: notificationItem
hoverEnabled: true
......@@ -79,7 +75,12 @@ PlasmaCore.Dialog {
LayoutMirroring.enabled: Qt.application.layoutDirection === Qt.RightToLeft
LayoutMirroring.childrenInherit: true
height: implicitHeight
//the binding needs to be disabled when re-populating actions, to minimize resizes
Binding on height {
id: heightBinding
value: notificationItem.implicitHeight
when: true
}
Timer {
id: notificationTimer
......
......@@ -76,23 +76,10 @@ void NotificationsHelper::addNotificationPopup(QObject *win)
this, SLOT(onPopupClosed()));
connect(popup, &QWindow::heightChanged, this, &NotificationsHelper::repositionPopups, Qt::UniqueConnection);
connect(popup, &QWindow::visibleChanged, this, &NotificationsHelper::onPopupShown, Qt::UniqueConnection);
popup->setProperty("initialPositionSet", false);
}
void NotificationsHelper::onPopupShown()
{
QWindow *popup = qobject_cast<QWindow*>(sender());
if (!popup || !popup->isVisible()) {
return;
}
// Make sure Dialog lays everything out and gets proper geometry
QMetaObject::invokeMethod(popup, "updateVisibility", Qt::DirectConnection, Q_ARG(bool, true));
// Now we can position the popups properly as the geometry is now known
repositionPopups();
//We are sure that after visibleChanged the size is final
//and the first expose event didn't arrive yet
connect(popup, &QQuickWindow::visibleChanged, this, &NotificationsHelper::repositionPopups);
}
void NotificationsHelper::processQueues()
......@@ -151,7 +138,8 @@ void NotificationsHelper::processShow()
QMetaObject::invokeMethod(popup, "populatePopup", Qt::DirectConnection, Q_ARG(QVariant, notificationData));
Q_EMIT popupShown(popup);
QTimer::singleShot(300, popup, &QWindow::show);
//use setproperty so the Dialog reimplementation will be used
popup->setProperty("visible", true);
if (!m_dispatchTimer->isActive()) {
m_dispatchTimer->start();
......@@ -177,10 +165,6 @@ void NotificationsHelper::processHide()
popup->hide();
// Make sure the popup gets placed correctly
// next time it's put on screen
popup->setProperty("initialPositionSet", false);
QMetaObject::invokeMethod(popup, "clearPopup", Qt::DirectConnection);
}
......@@ -298,30 +282,17 @@ void NotificationsHelper::repositionPopups()
m_mutex->lockForWrite();
QPoint pos;
for (int i = 0; i < m_popupsOnScreen.size(); ++i) {
if (m_popupLocation == NotificationsHelper::TopLeft
|| m_popupLocation == NotificationsHelper::TopCenter
|| m_popupLocation == NotificationsHelper::TopRight) {
int posY = m_plasmoidScreen.top() + cumulativeHeight;
pos.setY(m_plasmoidScreen.top() + cumulativeHeight);
if (m_popupsOnScreen[i]->isVisible() && m_popupsOnScreen[i]->property("initialPositionSet").toBool() == true && m_popupsOnScreen[i]->y() != 0) {
//if it's visible, go through setProperty which animates it
m_popupsOnScreen[i]->setProperty("y", posY);
} else {
// ...otherwise just set it directly
m_popupsOnScreen[i]->setY(posY);
m_popupsOnScreen[i]->setProperty("initialPositionSet", true);
}
} else {
int posY = m_plasmoidScreen.bottom() - cumulativeHeight - m_popupsOnScreen[i]->contentItem()->height();
if (m_popupsOnScreen[i]->isVisible() && m_popupsOnScreen[i]->property("initialPositionSet").toBool() == true && m_popupsOnScreen[i]->y() != 0) {
m_popupsOnScreen[i]->setProperty("y", posY);
} else {
m_popupsOnScreen[i]->setY(posY);
m_popupsOnScreen[i]->setProperty("initialPositionSet", true);
}
pos.setY(m_plasmoidScreen.bottom() - cumulativeHeight - m_popupsOnScreen[i]->height());
}
switch (m_popupLocation) {
......@@ -332,15 +303,15 @@ void NotificationsHelper::repositionPopups()
//fall through to top right
case TopRight:
case BottomRight:
m_popupsOnScreen[i]->setX(m_plasmoidScreen.right() - m_popupsOnScreen[i]->contentItem()->width() - m_offset);
pos.setX(m_plasmoidScreen.right() - m_popupsOnScreen[i]->width() - m_offset);
break;
case TopCenter:
case BottomCenter:
m_popupsOnScreen[i]->setX(m_plasmoidScreen.x() + (m_plasmoidScreen.width() / 2) - (m_popupsOnScreen[i]->contentItem()->width() / 2));
pos.setX(m_plasmoidScreen.x() + (m_plasmoidScreen.width() / 2) - (m_popupsOnScreen[i]->width() / 2));
break;
case TopLeft:
case BottomLeft:
m_popupsOnScreen[i]->setX(m_plasmoidScreen.left() + m_offset);
pos.setX(m_plasmoidScreen.left() + m_offset);
break;
case Left:
case Center:
......@@ -348,8 +319,8 @@ void NotificationsHelper::repositionPopups()
// Fall-through to make the compiler happy
break;
}
cumulativeHeight += (m_popupsOnScreen[i]->contentItem()->height() + m_offset);
m_popupsOnScreen[i]->setPosition(pos);
cumulativeHeight += (m_popupsOnScreen[i]->height() + m_offset);
}
m_mutex->unlock();
......
......@@ -71,7 +71,6 @@ Q_SIGNALS:
// void plasmoidScreenChanged();
private Q_SLOTS:
void onPopupShown();
void onPopupClosed();
void processQueues();
void processShow();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment