diff --git a/.gitignore b/.gitignore --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ CMakeLists.txt.user* *.unc-backup* .cmake/ +.vscode/ diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt --- a/autotests/CMakeLists.txt +++ b/autotests/CMakeLists.txt @@ -31,5 +31,6 @@ tst_pagerouter.qml tst_routerwindow.qml pagepool/tst_pagepool.qml + pagepool/tst_layers.qml ) diff --git a/autotests/pagepool/tst_layers.qml b/autotests/pagepool/tst_layers.qml new file mode 100644 --- /dev/null +++ b/autotests/pagepool/tst_layers.qml @@ -0,0 +1,307 @@ +/* + * SPDX-FileCopyrightText: 2020 Mason McParlane + * + * SPDX-License-Identifier: LGPL-2.0-or-later + */ + +import QtQuick 2.7 +import QtQuick.Controls 2.7 +import QtQuick.Window 2.1 +import org.kde.kirigami 2.11 as Kirigami +import QtTest 1.0 + +TestCase { + id: testCase + width: 400 + height: 400 + name: "PagePoolWithLayers" + when: windowShown + + function initTestCase() { + mainWindow.show() + } + + function cleanupTestCase() { + mainWindow.close() + } + + Kirigami.ApplicationWindow { + id: mainWindow + width: 480 + height: 360 + } + + Kirigami.PagePool { + id: pool + } + + SignalSpy { + id: stackSpy + target: mainWindow.pageStack + signalName: "onCurrentItemChanged" + } + + SignalSpy { + id: layerSpy + target: mainWindow.pageStack.layers + signalName: "onCurrentItemChanged" + } + + + function init() { + pool.clear() + mainWindow.pageStack.layers.clear() + compare(mainWindow.pageStack.layers.depth, 1) + mainWindow.pageStack.clear() + + for (var spy of [stackSpy, layerSpy, checkSpy_A, checkSpy_B, checkSpy_C, checkSpy_D, checkSpy_E]) { + spy.clear() + } + } + + ActionGroup { + id: group + exclusive: false + + Kirigami.PagePoolAction { + id: stackPageA + objectName: "stackPageA" + pagePool: pool + pageStack: mainWindow.pageStack + page: "TestPage.qml?page=A" + initialProperties: { return {title: "A", objectName: "Page A" } } + } + + Kirigami.PagePoolAction { + id: stackPageB + objectName: "stackPageB" + pagePool: pool + pageStack: mainWindow.pageStack + page: "TestPage.qml?page=B" + initialProperties: { return {title: "B", objectName: "Page B" } } + } + + Kirigami.PagePoolAction { + id: layerPageC + objectName: "layerPageC" + pagePool: pool + pageStack: mainWindow.pageStack + useLayers: true + page: "TestPage.qml?page=C" + initialProperties: { return {title: "C", objectName: "Page C" } } + } + + Kirigami.PagePoolAction { + id: layerPageD + objectName: "layerPageD" + pagePool: pool + pageStack: mainWindow.pageStack + useLayers: true + page: "TestPage.qml?page=D" + initialProperties: { return {title: "D", objectName: "Page D" } } + } + + Kirigami.PagePoolAction { + id: stackPageE + objectName: "stackPageE" + pagePool: pool + pageStack: mainWindow.pageStack + page: "TestPage.qml?page=E" + initialProperties: { return {title: "E", objectName: "Page E" } } + } + } + + function tapBack () { + mouseClick(mainWindow, 10, 10) + } + + function test_pushLayerBackButtonPushAgain() { + var stack = mainWindow.pageStack + var layers = stack.layers + + function pushA() { + stackPageA.trigger() + compare(stack.currentItem, pool.lastLoadedItem) + } + + function pushC () { + layerPageC.trigger() + compare(layers.currentItem, pool.lastLoadedItem) + } + + function pushD () { + layerPageD.trigger() + compare(layers.currentItem, pool.lastLoadedItem) + } + + compare(stackSpy.count, 0) + pushA() + compare(stackSpy.count, 1) + compare(layerSpy.count, 0) + pushC() + compare(layerSpy.count, 1) + pushD() + compare(layerSpy.count, 2) + compare(stackSpy.count, 1) + tapBack() + compare(layerSpy.count, 3) + pushD() + compare(layerSpy.count, 4) + } + + SignalSpy { + id: checkSpy_A + target: stackPageA + signalName: "onCheckedChanged" + } + + SignalSpy { + id: checkSpy_B + target: stackPageB + signalName: "onCheckedChanged" + } + + SignalSpy { + id: checkSpy_C + target: layerPageC + signalName: "onCheckedChanged" + } + + SignalSpy { + id: checkSpy_D + target: layerPageD + signalName: "onCheckedChanged" + } + + SignalSpy { + id: checkSpy_E + target: stackPageE + signalName: "onCheckedChanged" + } + + function dump_layers(msg = "") { + for (var i = 0; i < mainWindow.pageStack.layers.depth; ++i) { + console.debug(`${msg} ${i}: ${mainWindow.pageStack.layers.get(i)}`) + } + } + + function test_checked() { + var stack = mainWindow.pageStack + var layers = stack.layers + + function testCheck(expected = {}) { + let defaults = { + a: false, b: false, c: false, d: false, e: false + } + let actual = Object.assign({}, defaults, expected) + let pages = {a: stackPageA, b: stackPageB, c: layerPageC, d: layerPageD, e: stackPageE} + + for (const prop in actual) { + compare(pages[prop].checked, actual[prop], + `${pages[prop]} should ${actual[prop] ? 'be checked' : 'not be checked'}`) + } + } + + testCheck() + + compare(stackSpy.count, 0) + compare(layerSpy.count, 0) + compare(checkSpy_A.count, 0) + compare(checkSpy_B.count, 0) + compare(checkSpy_C.count, 0) + compare(checkSpy_D.count, 0) + compare(checkSpy_E.count, 0) + + stackPageA.trigger() + compare(checkSpy_A.count, 1) + testCheck({a:true}) + compare(stack.currentItem, stackPageA.pageItem()) + + stackPageB.trigger() + compare(checkSpy_A.count, 2) + compare(checkSpy_B.count, 3) + testCheck({b:true}) + compare(stack.currentItem, stackPageB.pageItem()) + + layerPageC.trigger() + testCheck({b:true, c:true}) + compare(checkSpy_C.count, 1) + compare(stack.currentItem, stackPageB.pageItem()) + compare(layers.currentItem, layerPageC.pageItem()) + compare(layerPageC.layerContainsPage(), true) + + layerPageD.trigger() + compare(stack.currentItem, stackPageB.pageItem()) + compare(layers.currentItem, layerPageD.pageItem()) + testCheck({b:true, c:true, d:true}) + + stackPageE.basePage = stack.currentItem + stackPageE.trigger() + testCheck({b:true, e:true}) + compare(stack.currentItem, stackPageE.pageItem()) + verify(!(layers.currentItem instanceof Page), + `Current item ${layers.currentItem} is a page but all pages should be popped`) + + stackPageA.trigger() + testCheck({a:true}) + compare(stack.currentItem, stackPageA.pageItem()) + verify(!(layers.currentItem instanceof Page), + `Current item ${layers.currentItem} is a page but all pages should be popped`) + + compare(checkSpy_A.count, 5) + compare(checkSpy_B.count, 4) + compare(checkSpy_C.count, 2) + compare(checkSpy_D.count, 2) + compare(checkSpy_E.count, 2) + } + + function test_push_A_C_D_A_popsLayers() { + var stack = mainWindow.pageStack + var layers = stack.layers + + stackPageA.trigger() + compare(stack.currentItem, stackPageA.pageItem()) + + layerPageC.trigger() + compare(layers.currentItem, layerPageC.pageItem()) + + layerPageD.trigger() + compare(layers.currentItem, layerPageD.pageItem()) + + stackPageA.trigger() + compare(stack.currentItem, stackPageA.pageItem()) + verify(!(layers.currentItem instanceof Page), + `Current item ${layers.currentItem} is a page but all pages should be popped`) + } + + function test_push_A_C_D_back_back_C_back_C() { + var stack = mainWindow.pageStack + var layers = stack.layers + + stackPageA.trigger() + layerPageC.trigger() + layerPageD.trigger() + tapBack() + tapBack() + layerPageC.trigger() + tapBack() + layerPageC.trigger() + compare(layers.currentItem, layerPageC.pageItem()) + } + + function test_exclusive_group() { + var stack = mainWindow.pageStack + var layers = stack.layers + + group.exclusive = true + stackPageA.trigger() + compare(stackPageA.checked, true) + compare(layerPageC.checked, false) + layerPageC.trigger() + compare(stackPageA.checked, false) + compare(layerPageC.checked, true) + tapBack() + compare(stackPageA.checked, true) + compare(layerPageC.checked, false) + } +} diff --git a/src/controls/PagePoolAction.qml b/src/controls/PagePoolAction.qml --- a/src/controls/PagePoolAction.qml +++ b/src/controls/PagePoolAction.qml @@ -5,6 +5,7 @@ */ import QtQuick 2.7 +import QtQml 2.7 import QtQuick.Controls 2.5 as Controls import org.kde.kirigami 2.11 as Kirigami @@ -27,7 +28,7 @@ * pagePool: Kirigami.PagePool * The PagePool used by this PagePoolAction. * PagePool will make sure only one instance of the page identified by the page url will be created and reused. - *PagePool's lastLoaderUrl property will be used to control the mutual + * PagePool's lastLoaderUrl property will be used to control the mutual * exclusivity of the checked state of the PagePoolAction instances * sharing the same PagePool */ @@ -56,44 +57,96 @@ */ property var initialProperties - checked: pagePool && pagePool.resolvedUrl(page) == pagePool.lastLoadedUrl + /** useLayers: bool + * @since 5.70 + * @since org.kde.kirigami 2.12 + * When true the PagePoolAction will use the layers property of the pageStack. + * This is intended for use with PageRow layers to allow PagePoolActions to + * push context-specific pages onto the layers stack. + */ + property bool useLayers: false + + /** + * Retrieve the page item held in the PagePool or null if it has not been loaded yet. + */ + function pageItem() { + return pagePool.pageForUrl(page) + } + + /** + * Return true if the page has been loaded and placed on pageStack.layers + * and useLayers is true, otherwise returns null. + */ + function layerContainsPage() { + if (!useLayers || !pageStack.hasOwnProperty("layers")) return false + + var found = pageStack.layers.find((item, index) => { + return item === pagePool.pageForUrl(page) + }) + return found ? true: false + } + + /** + * Return true if the page has been loaded and placed on the pageStack, + * otherwise returns null. + */ + function stackContainsPage() { + if (useLayers) return false + return pageStack.columnView.containsItem(pagePool.pageForUrl(page)) + } + + checkable: true + onTriggered: { if (page.length == 0 || !pagePool || !pageStack) { return; } - if (initialProperties && typeof(initialProperties) !== "object") { - console.warn("initialProperties must be of type object"); - return; + // User intends to "go back" to this layer. + if (layerContainsPage() && pageItem() !== pageStack.layers.currentItem) { + pageStack.layers.replace(pageItem(), pageItem()) // force pop above + return } - if (pagePool.resolvedUrl(page) == pagePool.lastLoadedUrl) { + // User intends to "go back" to this page. + if (stackContainsPage()) { + if (pageStack.hasOwnProperty("layers")) { + pageStack.layers.clear() + } + } + + let pageStack_ = useLayers ? pageStack.layers : pageStack + + if (initialProperties && typeof(initialProperties) !== "object") { + console.warn("initialProperties must be of type object"); return; } - if (!pageStack.hasOwnProperty("pop") || typeof pageStack.pop !== "function" || !pageStack.hasOwnProperty("push") || typeof pageStack.push !== "function") { + if (!pageStack_.hasOwnProperty("pop") || typeof pageStack_.pop !== "function" || !pageStack_.hasOwnProperty("push") || typeof pageStack_.push !== "function") { return; } if (pagePool.isLocalUrl(page)) { if (basePage) { - pageStack.pop(basePage); - } else { - pageStack.clear(); + pageStack_.pop(basePage); + + } else if (!useLayers) { + pageStack_.clear(); } - pageStack.push(initialProperties ? + pageStack_.push(initialProperties ? pagePool.loadPageWithProperties(page, initialProperties) : pagePool.loadPage(page)); } else { var callback = function(item) { if (basePage) { - pageStack.pop(basePage); - } else { - pageStack.clear(); + pageStack_.pop(basePage); + + } else if (!useLayers) { + pageStack_.clear(); } - pageStack.push(item); + pageStack_.push(item); }; if (initialProperties) { @@ -104,4 +157,52 @@ } } } + + // Exposing this as a property is required as Action does not have a default property + property QtObject _private: QtObject { + id: _private + + function setChecked(checked) { + root.checked = checked + } + + function clearLayers() { + pageStack.layers.clear() + } + + property list connections: [ + Connections { + target: pageStack + + onCurrentItemChanged: { + if (root.useLayers) { + if (root.layerContainsPage()) { + _private.clearLayers() + } + if (root.checkable) + _private.setChecked(false); + + } else { + if (root.checkable) + _private.setChecked(root.stackContainsPage()); + } + } + }, + Connections { + enabled: pageStack.hasOwnProperty("layers") + target: pageStack.layers + + onCurrentItemChanged: { + if (root.useLayers && root.checkable) { + _private.setChecked(root.layerContainsPage()); + + } else { + if (pageStack.layers.depth == 1 && root.stackContainsPage()) { + _private.setChecked(true) + } + } + } + } + ] + } } diff --git a/src/controls/PageRow.qml b/src/controls/PageRow.qml --- a/src/controls/PageRow.qml +++ b/src/controls/PageRow.qml @@ -434,7 +434,7 @@ function clear () { //don't let it kill the main page row - var d = root.depth; + var d = layersStack.depth; for (var i = 1; i < d; ++i) { pop(); } diff --git a/src/pagepool.h b/src/pagepool.h --- a/src/pagepool.h +++ b/src/pagepool.h @@ -70,6 +70,11 @@ */ Q_INVOKABLE QUrl urlForPage(QQuickItem *item) const; + /** + * @returns The page associated with a given URL, nullptr if there is no correspondence + */ + Q_INVOKABLE QQuickItem *pageForUrl(const QUrl &url) const; + /** * @returns true if the is managed by the PagePool * @param the page can be either a QQuickItem or an url diff --git a/src/pagepool.cpp b/src/pagepool.cpp --- a/src/pagepool.cpp +++ b/src/pagepool.cpp @@ -38,19 +38,7 @@ } if (cache) { - for (auto *c : m_componentForUrl.values()) { - c->deleteLater(); - } - m_componentForUrl.clear(); - - for (auto *i : m_itemForUrl.values()) { - // items that had been deparented are safe to delete - if (!i->parentItem()) { - i->deleteLater(); - } - QQmlEngine::setObjectOwnership(i, QQmlEngine::JavaScriptOwnership); - } - m_itemForUrl.clear(); + clear(); } m_cachePages = cache; @@ -76,25 +64,23 @@ const QUrl actualUrl = resolvedUrl(url); - QQuickItem *foundItem = nullptr; - if (actualUrl == m_lastLoadedUrl && m_lastLoadedItem) { - foundItem = m_lastLoadedItem; - } else if (m_itemForUrl.contains(actualUrl)) { - foundItem = m_itemForUrl[actualUrl]; - } + auto found = m_itemForUrl.find(actualUrl); + if (found != m_itemForUrl.end()) { + m_lastLoadedUrl = found.key(); + m_lastLoadedItem = found.value(); - if (foundItem) { if (callback.isCallable()) { - QJSValueList args = {qmlEngine(this)->newQObject(foundItem)}; + QJSValueList args = {qmlEngine(this)->newQObject(found.value())}; callback.call(args); - m_lastLoadedUrl = actualUrl; emit lastLoadedUrlChanged(); + emit lastLoadedItemChanged(); // We could return the item, but for api coherence return null return nullptr; + } else { - m_lastLoadedUrl = actualUrl; emit lastLoadedUrlChanged(); - return foundItem; + emit lastLoadedItemChanged(); + return found.value(); } } @@ -232,6 +218,11 @@ return m_urlForItem.value(item); } +QQuickItem *PagePool::pageForUrl(const QUrl &url) const +{ + return m_itemForUrl.value(resolvedUrl(url.toString()), nullptr); +} + bool PagePool::contains(const QVariant &page) const { if (page.canConvert()) { @@ -282,9 +273,21 @@ void PagePool::clear() { - for (const auto& url : m_urlForItem) { - deletePage(url); + for (auto *c : m_componentForUrl.values()) { + c->deleteLater(); + } + m_componentForUrl.clear(); + + for (auto *i : m_itemForUrl.values()) { + // items that had been deparented are safe to delete + if (!i->parentItem()) { + i->deleteLater(); + } + QQmlEngine::setObjectOwnership(i, QQmlEngine::JavaScriptOwnership); } + m_itemForUrl.clear(); + + m_urlForItem.clear(); m_lastLoadedUrl = QUrl(); m_lastLoadedItem = nullptr;