Skip to content

feat: add dynamic position updates for notification bubbles #1500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
qxp930712 wants to merge 1 commit into
base: master
Choose a base branch
from
+104 −4
Open

feat: add dynamic position updates for notification bubbles #1500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions frame/layershell/dlayershellwindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,12 @@
return new DLayerShellWindow(window);
}

void DLayerShellWindow::requestPositionUpdate(int width, int height)
{
Q_EMIT positionUpdateRequested(width, height);
}

DLayerShellWindow* DLayerShellWindow::qmlAttachedProperties(QObject *object)

Check warning on line 239 in frame/layershell/dlayershellwindow.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

The function 'qmlAttachedProperties' is never used.
{
auto window = qobject_cast<QWindow*>(object);
if (window)
Expand Down
9 changes: 9 additions & 0 deletions frame/layershell/dlayershellwindow.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,14 @@
void setCloseOnDismissed(bool close);
bool closeOnDismissed() const;

/**
* Request position update with specified width and height
* This can be called from QML to trigger a position recalculation
* @param width The window width to use for position calculation
* @param height The window height to use for position calculation
*/
Q_INVOKABLE void requestPositionUpdate(int width, int height);

/**
* Gets the LayerShell Window for a given Qt Window
* Ownership is not transferred
Expand All @@ -145,9 +153,10 @@
void keyboardInteractivityChanged();
void layerChanged();
void scopeChanged();
void positionUpdateRequested(int width, int height);

Check warning on line 156 in frame/layershell/dlayershellwindow.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Local variable 'positionUpdateRequested' shadows outer function

private:
DLayerShellWindow(QWindow* window);

Check warning on line 159 in frame/layershell/dlayershellwindow.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Class 'DLayerShellWindow' has a constructor with 1 argument that is not explicit. Such, so called "Converting constructors", should in general be explicit for type safety reasons as that prevents unintended implicit conversions.
QScopedPointer<DLayerShellWindowPrivate> d;
};

Expand Down
43 changes: 43 additions & 0 deletions frame/layershell/x11dlayershellemulation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ LayerShellEmulation::LayerShellEmulation(QWindow* window, QObject *parent)
onPositionChanged();
connect(m_dlayerShellWindow, &DLayerShellWindow::anchorsChanged, this, &LayerShellEmulation::onPositionChanged);
connect(m_dlayerShellWindow, &DLayerShellWindow::marginsChanged, this, &LayerShellEmulation::onPositionChanged);
connect(m_dlayerShellWindow, &DLayerShellWindow::positionUpdateRequested, this, &LayerShellEmulation::onPositionUpdateRequested);

onExclusionZoneChanged();
m_exclusionZoneChangedTimer.setSingleShot(true);
Expand Down Expand Up @@ -120,6 +121,9 @@ void LayerShellEmulation::onPositionChanged()
auto screenRect = screen->geometry();
auto x = screenRect.left() + (screenRect.width() - m_window->width()) / 2;
auto y = screenRect.top() + (screenRect.height() - m_window->height()) / 2;

qWarning() << "caimengci position x=" << x << "y=" << y << "window width=" << m_window->width() << "window height=" << m_window->height();
Copy link
Member

@BLumia BLumia Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个日志还有用吗？甚至是warning


if (anchors & DLayerShellWindow::AnchorRight) {
Comment on lines 122 to 127
Copy link

@sourcery-ai sourcery-ai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (performance): Avoid hard-coded debug logging left in production code

This qWarning() appears to be temporary debug logging and will emit on every position change. Please remove it, or switch to a qDebug() with a neutral message if you still need this information.

Suggested change
auto x = screenRect.left() + (screenRect.width() - m_window->width()) / 2;
auto y = screenRect.top() + (screenRect.height() - m_window->height()) / 2;
qWarning() << "caimengci position x=" << x << "y=" << y << "window width=" << m_window->width() << "window height=" << m_window->height();
if (anchors & DLayerShellWindow::AnchorRight) {
auto x = screenRect.left() + (screenRect.width() - m_window->width()) / 2;
auto y = screenRect.top() + (screenRect.height() - m_window->height()) / 2;
if (anchors & DLayerShellWindow::AnchorRight) {

// https://doc.qt.io/qt-6/qrect.html#right
x = (screen->geometry().right() + 1 - m_window->width() - m_dlayerShellWindow->rightMargin());
Expand Down Expand Up @@ -153,6 +157,45 @@ void LayerShellEmulation::onPositionChanged()
m_window->setGeometry(rect);
Copy link

@sourcery-ai sourcery-ai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Factor out shared positioning logic to avoid duplication

onPositionUpdateRequested mostly duplicates onPositionChanged, differing only in where the size comes from. Consider extracting the anchor/geometry calculation into a helper (e.g. computeGeometry(QSize size)) and calling it from both, to keep the logic in sync as anchor handling evolves.

Suggested implementation:

QRect LayerShellEmulation::computeGeometry(const QSize &size)
{
    auto anchors = m_dlayerShellWindow->anchors();
    auto screen = m_window->screen();
    auto screenRect = screen->geometry();
    int x = screenRect.left() + (screenRect.width() - size.width()) / 2;
    int y = screenRect.top() + (screenRect.height() - size.height()) / 2;

    if (anchors & DLayerShellWindow::AnchorRight) {
        // https://doc.qt.io/qt-6/qrect.html#right
        x = (screen->geometry().right() + 1 - size.width() - m_dlayerShellWindow->rightMargin());
    }

    return QRect(x, y, size.width(), size.height());
}

void LayerShellEmulation::onPositionUpdateRequested(int width, int height)

{
    const QSize size(width, height);
    const QRect rect = computeGeometry(size);

    // Keep behavior consistent with other positioning code: move/resize the window.
    m_window->setGeometry(rect);

about similarly updatingonPositionChanged` once the full function body is available.

Here are the changes:

<file_operations>
<file_operation operation="edit" file_path="frame/layershell/x11dlayershellemulation.cpp">
<<<<<<< SEARCH
void LayerShellEmulation::onPositionUpdateRequested(int width, int height)

QRect LayerShellEmulation::computeGeometry(const QSize &size)
{
auto anchors = m_dlayerShellWindow->anchors();
auto screen = m_window->screen();
auto screenRect = screen->geometry();
int x = screenRect.left() + (screenRect.width() - size.width()) / 2;
int y = screenRect.top() + (screenRect.height() - size.height()) / 2;

if (anchors & DLayerShellWindow::AnchorRight) {
    // https://doc.qt.io/qt-6/qrect.html#right
    x = (screen->geometry().right() + 1 - size.width() - m_dlayerShellWindow->rightMargin());
}

return QRect(x, y, size.width(), size.height());

}

void LayerShellEmulation::onPositionUpdateRequested(int width, int height)

REPLACE

<<<<<<< SEARCH
{
auto anchors = m_dlayerShellWindow->anchors();
auto screen = m_window->screen();
auto screenRect = screen->geometry();
auto x = screenRect.left() + (screenRect.width() - width) / 2;
auto y = screenRect.top() + (screenRect.height() - height) / 2;

if (anchors & DLayerShellWindow::AnchorRight) {
    x = (screen->geometry().right() + 1 - width - m_dlayerShellWindow->rightMargin());
}

=======
{
const QSize size(width, height);
const QRect rect = computeGeometry(size);

// Keep behavior consistent with other positioning code: move/resize the window.
m_window->setGeometry(rect);

REPLACE
</file_operation>
</file_operations>

<additional_changes>
To fully realize the refactoring you suggested, the following should also be done once you can see the complete function:

  1. In LayerShellEmulation::onPositionChanged(...) (or whichever method contains the snippet starting with:
    auto y = screenRect.top() + (screenRect.height() - m_window->height()) / 2;
    and using anchors & DLayerShellWindow::AnchorRight), replace the inlined anchor/geometry logic with:

    • Constructing a QSize size(m_window->width(), m_window->height());
    • Calling const QRect rect = computeGeometry(size);
    • Using rect for logging (e.g. rect.x(), rect.y()) and for m_window->setGeometry(rect);

  2. If onPositionChanged also handles other anchors (top, bottom, left, etc.), consider moving that logic into computeGeometry as well, mirroring the pattern used for AnchorRight, so that all anchor handling is centralized.

}

void LayerShellEmulation::onPositionUpdateRequested(int width, int height)
{
auto anchors = m_dlayerShellWindow->anchors();
auto screen = m_window->screen();
auto screenRect = screen->geometry();
auto x = screenRect.left() + (screenRect.width() - width) / 2;
auto y = screenRect.top() + (screenRect.height() - height) / 2;

if (anchors & DLayerShellWindow::AnchorRight) {
x = (screen->geometry().right() + 1 - width - m_dlayerShellWindow->rightMargin());
}

if (anchors & DLayerShellWindow::AnchorBottom) {
y = (screen->geometry().bottom() + 1 - height - m_dlayerShellWindow->bottomMargin());
}
if (anchors & DLayerShellWindow::AnchorLeft) {
x = (screen->geometry().left() + m_dlayerShellWindow->leftMargin());
}
if (anchors & DLayerShellWindow::AnchorTop) {
y = (screen->geometry().top() + m_dlayerShellWindow->topMargin());
}

QRect rect(x, y, width, height);

const bool horizontallyConstrained = anchors.testFlags({DLayerShellWindow::AnchorLeft, DLayerShellWindow::AnchorRight});
const bool verticallyConstrained = anchors.testFlags({DLayerShellWindow::AnchorTop, DLayerShellWindow::AnchorBottom});

if (horizontallyConstrained) {
rect.setX(screen->geometry().left() + m_dlayerShellWindow->leftMargin());
rect.setWidth(screen->geometry().width() - m_dlayerShellWindow->leftMargin() - m_dlayerShellWindow->rightMargin());
}
if (verticallyConstrained) {
rect.setY(screen->geometry().top() + m_dlayerShellWindow->topMargin());
rect.setHeight(screen->geometry().height() - m_dlayerShellWindow->topMargin() - m_dlayerShellWindow->bottomMargin());
}

m_window->setGeometry(rect);
}

/**
* https://specifications.freedesktop.org/wm-spec/wm-spec-1.4.html#idm45649101327728
*/
Expand Down
1 change: 1 addition & 0 deletions frame/layershell/x11dlayershellemulation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ private slots:
void onLayerChanged();
// margins or anchor changed
void onPositionChanged();
void onPositionUpdateRequested(int width, int height);
void onExclusionZoneChanged();
void onScopeChanged();
// void onKeyboardInteractivityChanged();
Expand Down
11 changes: 10 additions & 1 deletion panels/notification/bubble/bubblepanel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
#include "bubblepanel.h"
#include "bubbleitem.h"
#include "bubblemodel.h"
#include "dataaccessorproxy.h"

Check warning on line 8 in panels/notification/bubble/bubblepanel.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: "dataaccessorproxy.h" not found.
#include "pluginfactory.h"

Check warning on line 9 in panels/notification/bubble/bubblepanel.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: "pluginfactory.h" not found.
#include <qtimer.h>

Check warning on line 10 in panels/notification/bubble/bubblepanel.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <qtimer.h> not found. Please note: Cppcheck does not need standard library headers to get proper results.
Copy link
Member

@BLumia BLumia Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No description provided.


#include <QTimer>

Check warning on line 12 in panels/notification/bubble/bubblepanel.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QTimer> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QLoggingCategory>

Check warning on line 13 in panels/notification/bubble/bubblepanel.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QLoggingCategory> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QQueue>

#include <appletbridge.h>
Expand Down Expand Up @@ -54,6 +55,7 @@
connect(m_bubbles, &BubbleModel::rowsInserted, this, &BubblePanel::onBubbleCountChanged);
connect(m_bubbles, &BubbleModel::rowsRemoved, this, &BubblePanel::onBubbleCountChanged);

setVisible(true);
return true;
}

Expand Down Expand Up @@ -111,7 +113,14 @@
void BubblePanel::onBubbleCountChanged()
{
bool isEmpty = m_bubbles->items().isEmpty();
setVisible(!isEmpty && enabled());

if (isEmpty) {
QTimer::singleShot(400, this, [this]() {
Comment on lines +117 to +118
Copy link

@sourcery-ai sourcery-ai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Pending hide timer can fire even after bubbles are added

Using QTimer::singleShot introduces a race: if the list becomes empty (scheduling the hide) and a bubble is added within 400 ms, the lambda will still call setVisible(false), hiding a non-empty panel. Use a member QTimer you can stop when items are added again, or at least re-check that the model is still empty inside the lambda before hiding.

setVisible(false);
});
} else {
setVisible(!isEmpty && enabled());
}
}

void BubblePanel::addBubble(qint64 id)
Expand Down
39 changes: 36 additions & 3 deletions panels/notification/bubble/package/main.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ Window {

visible: Applet.visible
width: 390
height: Math.max(10, bubbleView.height + bubbleView.anchors.topMargin + bubbleView.anchors.bottomMargin)
DLayerShellWindow.layer: DLayerShellWindow.LayerOverlay
height: 0
Copy link

@sourcery-ai sourcery-ai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (bug_risk): Clarify relationship between QML height and geometry set from C++

height is now hard-coded to 0 while the actual geometry is driven from C++ via requestPositionUpdate. This may confuse QML code or break layouts that read Window.height. Either bind height to the same value used in requestPositionUpdate, or confirm and document that no QML logic depends on an accurate height.

DLayerShellWindow.layer: DLayerShellWindow.LayerTop
DLayerShellWindow.anchors: DLayerShellWindow.AnchorBottom | DLayerShellWindow.AnchorRight
DLayerShellWindow.topMargin: windowMargin(0)
DLayerShellWindow.rightMargin: windowMargin(1)
Expand All @@ -89,6 +89,11 @@ Window {
root.screen = Qt.binding(function () { return Qt.application.screens[0]})
}

// Function to trigger position update with custom width and height
function updatePosition(width, height) {
DLayerShellWindow.requestPositionUpdate(width, height)
}

ListView {
id: bubbleView
width: 360
Expand All @@ -98,13 +103,18 @@ Window {
bottom: parent.bottom
bottomMargin: 10
rightMargin: 10
margins: 30
topMargin: 10
}

spacing: 10
model: Applet.bubbles
interactive: false
verticalLayoutDirection: ListView.BottomToTop

// Monitor height changes and update position
onHeightChanged: {
updatePosition(390, bubbleView.height + bubbleView.anchors.topMargin + bubbleView.anchors.bottomMargin)
}
add: Transition {
id: addTrans
// Before starting the new animation, forcibly complete the previous notification bubble's animation
Expand All @@ -129,8 +139,31 @@ Window {
}
}
delegate: Bubble {
id: delegateItem
width: 360
bubble: model

ListView.onRemove: SequentialAnimation {
PropertyAction {
target: delegateItem
property: "ListView.delayRemove"
value: true
}
ParallelAnimation {
NumberAnimation {
target: delegateItem
property: "x"
to: 360
duration: 400
easing.type: Easing.InExpo
}
}
PropertyAction {
target: delegateItem
property: "ListView.delayRemove"
value: false
}
}
}
}
}
Loading