Skip to content

fix: Dragging applications in application folder cannot create a new page #558

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

Merged
BLumia merged 1 commit into from
May 13, 2025
Merged

fix: Dragging applications in application folder cannot create a new page #558

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
25 changes: 22 additions & 3 deletions qml/FolderGridViewPopup.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,14 @@ Popup {
readonly property real paddingColumns: 0.3
readonly property int horizontalPadding: contentRoot.width * paddingColumns
anchors.fill: parent

property bool createdEmptyPage: false

function checkDragMove() {
if (drag.x < horizontalPadding) {
pageIntent = -1
} else if (drag.x > (width - horizontalPadding)) {
let isLastPage = folderPagesView.currentIndex === folderPagesView.count - 1
if (isLastPage) {
if (isLastPage && folderPageDropArea.createdEmptyPage) {
return
}
pageIntent = 1
Expand All @@ -181,9 +181,11 @@ Popup {
let dragId = drop.getDataAsString("text/x-dde-launcher-dnd-desktopId")
dropOnPage(dragId, "internal/folders/" + folderLoader.currentFolderId, folderPagesView.currentIndex)
pageIntent = 0
createdEmptyPage = false
}
onExited: {
pageIntent = 0
Comment thread
wjyrich marked this conversation as resolved.
createdEmptyPage = false
}
onPageIntentChanged: {
if (pageIntent !== 0) {
Expand All @@ -198,7 +200,16 @@ Popup {
interval: 1000
onTriggered: {
if (parent.pageIntent > 0) {
incrementPageIndex(folderPagesView)
let isLastPage = (folderPagesView.currentIndex === folderPagesView.count - 1)
if (isLastPage && !folderPageDropArea.createdEmptyPage) {
let newPageIndex = ItemArrangementProxyModel.creatEmptyPage(folderLoader.currentFolderId)
folderPageDropArea.createdEmptyPage = true
folderPagesView.setCurrentIndex(newPageIndex)
parent.pageIntent = 0
return
}else{
incrementPageIndex(folderPagesView)
}
} else if (parent.pageIntent < 0) {
decrementPageIndex(folderPagesView)
}
Expand Down Expand Up @@ -240,6 +251,14 @@ Popup {

currentIndex: folderPageIndicator.currentIndex

Connections {
target: ItemArrangementProxyModel
function onFolderPageCountChanged(folderId) {
if (folderId === folderLoader.currentFolderId) {
gridViews.model = ItemArrangementProxyModel.pageCount(folderId)
}
}
}
Repeater {
id: gridViews
model: ItemArrangementProxyModel.pageCount(folderLoader.currentFolderId) // FIXME: should be a property?
Expand Down
23 changes: 20 additions & 3 deletions src/models/itemarrangementproxymodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,22 @@ void ItemArrangementProxyModel::commitDndOperation(const QString &dragId, const
}

// return new empty page index
int ItemArrangementProxyModel::creatEmptyPage() const
int ItemArrangementProxyModel::creatEmptyPage(int folderId) const
{
m_topLevel->appendEmptyPage();
return m_topLevel->pageCount() - 1;
if(folderId == 0){
m_topLevel->appendEmptyPage();
return m_topLevel->pageCount() - 1;
}
QString fullId("internal/folders/" + QString::number(folderId));
Q_ASSERT(m_folders.contains(fullId));
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot May 13, 2025

Choose a reason for hiding this comment

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

suggestion: Using Q_ASSERT here may abort in debug builds

Replace the assert with a runtime check—since you already warn and return on missing folders—to avoid debug-only aborts and keep behavior consistent.

Suggested change
Q_ASSERT(m_folders.contains(fullId));
if (!m_folders.contains(fullId)) {
qWarning() << "Folder not found, returning 0. fullId is" << fullId;
return 0;
}


if (auto itemPage = m_folders.value(fullId); itemPage) {
itemPage->appendEmptyPage();
return itemPage->pageCount() - 1;
} else {
qWarning() << "itemPage create empty page false, return 0. fullId is" << fullId;
return 0;
}
}

void ItemArrangementProxyModel::removeEmptyPage() const
Expand Down Expand Up @@ -408,6 +420,11 @@ ItemsPage *ItemArrangementProxyModel::createFolder(const QString &id)
folder->setData(fullId, AppItem::DesktopIdRole);
m_folderModel.appendRow(folder);

connect(page, &ItemsPage::pageCountChanged, this, [this, fullId]() {
int folderId = QStringView{fullId}.mid(17).toInt();
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot May 13, 2025

Choose a reason for hiding this comment

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

suggestion: Hard-coded index for parsing folderId is brittle

Store the numeric ID separately or derive it with split/regex rather than relying on a hard-coded offset.

emit folderPageCountChanged(folderId);
});

return page;
}

Expand Down
3 changes: 2 additions & 1 deletion src/models/itemarrangementproxymodel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class ItemArrangementProxyModel : public QConcatenateTablesProxyModel
Q_INVOKABLE void updateFolderName(int folderId, const QString & name);
Q_INVOKABLE void bringToFront(const QString & id);
Q_INVOKABLE void commitDndOperation(const QString & dragId, const QString & dropId, const DndOperation op, int pageHint = -1);
Q_INVOKABLE int creatEmptyPage() const;
Q_INVOKABLE int creatEmptyPage(int folderId = 0) const;
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot May 13, 2025

Choose a reason for hiding this comment

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

suggestion: Method name typo and const correctness

Rename creatEmptyPage to createEmptyPage for clarity, and drop the const qualifier since this method mutates internal state.

Q_INVOKABLE void removeEmptyPage() const;

ItemsPage *itemsPage() { return m_topLevel; }
Expand All @@ -70,6 +70,7 @@ class ItemArrangementProxyModel : public QConcatenateTablesProxyModel

signals:
void topLevelPageCountChanged();
void folderPageCountChanged(int folderId);

private:
explicit ItemArrangementProxyModel(QObject *parent = nullptr);
Expand Down