From bce6b56c8d26df707da3c9f925dd3530b41d6255 Mon Sep 17 00:00:00 2001 From: James Turner Date: Tue, 9 Jun 2020 16:34:23 +0100 Subject: [PATCH] Remove use of QQ.Window / PopupTracker Overlay menus work better than native popups for most use-cases, and simplify integration (no window focus changes). Switch the remaining menus to always use the overlay system As part of this, fix how Overlay positions are adjusted, to avoid the ugly zero-interval timer. --- src/GUI/CMakeLists.txt | 2 - src/GUI/LauncherController.cxx | 2 - src/GUI/PopupWindowTracker.cxx | 67 ------------------ src/GUI/PopupWindowTracker.hxx | 38 ----------- src/GUI/qml/AircraftVariantChoice.qml | 98 +++++---------------------- src/GUI/qml/HistoryPopup.qml | 68 +++---------------- src/GUI/qml/NumericalEdit.qml | 84 +++-------------------- src/GUI/qml/Overlay.qml | 44 +++++------- src/GUI/qml/OverlayMenu.qml | 6 +- src/GUI/qml/VariantMenu.qml | 96 ++++++++++++++++++++++++++ src/GUI/resources.qrc | 1 + 11 files changed, 153 insertions(+), 353 deletions(-) delete mode 100644 src/GUI/PopupWindowTracker.cxx delete mode 100644 src/GUI/PopupWindowTracker.hxx create mode 100644 src/GUI/qml/VariantMenu.qml diff --git a/src/GUI/CMakeLists.txt b/src/GUI/CMakeLists.txt index b9cfb7031..91c1982c1 100644 --- a/src/GUI/CMakeLists.txt +++ b/src/GUI/CMakeLists.txt @@ -161,8 +161,6 @@ if (HAVE_QT) PreviewImageItem.hxx ThumbnailImageItem.cxx ThumbnailImageItem.hxx - PopupWindowTracker.cxx - PopupWindowTracker.hxx QmlPropertyModel.hxx QmlPropertyModel.cxx QmlPositioned.hxx diff --git a/src/GUI/LauncherController.cxx b/src/GUI/LauncherController.cxx index 953aa2856..d58841fb5 100644 --- a/src/GUI/LauncherController.cxx +++ b/src/GUI/LauncherController.cxx @@ -44,7 +44,6 @@ #include "NavaidSearchModel.hxx" #include "PathUrlHelper.hxx" #include "PixmapImageItem.hxx" -#include "PopupWindowTracker.hxx" #include "PreviewImageItem.hxx" #include "QmlAircraftInfo.hxx" #include "QmlPositioned.hxx" @@ -157,7 +156,6 @@ void LauncherController::initQML() qmlRegisterType("FlightGear", 1, 0, "FileDialog"); qmlRegisterType("FlightGear.Launcher", 1, 0, "AircraftInfo"); - qmlRegisterType("FlightGear.Launcher", 1, 0, "PopupWindowTracker"); qmlRegisterUncreatableType("FlightGear.Launcher", 1, 0, "LocalAircraftCache", "Aircraft cache"); qmlRegisterUncreatableType("FlightGear.Launcher", 1, 0, "AircraftModel", "Built-in model"); diff --git a/src/GUI/PopupWindowTracker.cxx b/src/GUI/PopupWindowTracker.cxx deleted file mode 100644 index 540db9b2d..000000000 --- a/src/GUI/PopupWindowTracker.cxx +++ /dev/null @@ -1,67 +0,0 @@ -#include "PopupWindowTracker.hxx" - -#include -#include -#include -#include - -PopupWindowTracker::PopupWindowTracker(QObject *parent) : QObject(parent) -{ - connect(qGuiApp, &QGuiApplication::applicationStateChanged, - this, &PopupWindowTracker::onApplicationStateChanged); -} - -PopupWindowTracker::~PopupWindowTracker() -{ - if (m_window) { - qApp->removeEventFilter(this); - } -} - - -void PopupWindowTracker::setWindow(QWindow *window) -{ - if (m_window == window) - return; - - if (m_window) { - qApp->removeEventFilter(this); - } - - m_window = window; - - if (m_window) { - qApp->installEventFilter(this); - } - - emit windowChanged(m_window); -} - -void PopupWindowTracker::onApplicationStateChanged(Qt::ApplicationState as) -{ - if (m_window && (as != Qt::ApplicationActive)) { - m_window->close(); - setWindow(nullptr); - } -} - -bool PopupWindowTracker::eventFilter(QObject *watched, QEvent *event) -{ - if (!m_window) - return false; - - if (event->type() == QEvent::MouseButtonPress) { - QMouseEvent* me = static_cast(event); - QRect globalGeometry(m_window->mapToGlobal(QPoint(0,0)), m_window->size()); - - if (globalGeometry.contains(me->globalPos())) { - // click inside the window, process as normal fall through - } else { - m_window->close(); - setWindow(nullptr); - // still fall through - } - } - - return false; -} diff --git a/src/GUI/PopupWindowTracker.hxx b/src/GUI/PopupWindowTracker.hxx deleted file mode 100644 index 2eeb72860..000000000 --- a/src/GUI/PopupWindowTracker.hxx +++ /dev/null @@ -1,38 +0,0 @@ -#ifndef POPUPWINDOWTRACKER_HXX -#define POPUPWINDOWTRACKER_HXX - -#include - -class QWindow; - -class PopupWindowTracker : public QObject -{ - Q_OBJECT - - Q_PROPERTY(QWindow* window READ window WRITE setWindow NOTIFY windowChanged) - QWindow* m_window = nullptr; - -public: - explicit PopupWindowTracker(QObject *parent = nullptr); - - ~PopupWindowTracker(); - - QWindow* window() const - { - return m_window; - } - -signals: - - void windowChanged(QWindow* window); - -public slots: - void setWindow(QWindow* window); - - void onApplicationStateChanged(Qt::ApplicationState as); - -protected: - bool eventFilter(QObject *watched, QEvent *event) override; -}; - -#endif // POPUPWINDOWTRACKER_HXX diff --git a/src/GUI/qml/AircraftVariantChoice.qml b/src/GUI/qml/AircraftVariantChoice.qml index 43d564d7f..8dcfdbae2 100644 --- a/src/GUI/qml/AircraftVariantChoice.qml +++ b/src/GUI/qml/AircraftVariantChoice.qml @@ -1,7 +1,6 @@ import QtQuick 2.4 -import QtQuick.Window 2.0 import "." // -> forces the qmldir to be loaded - +import FlightGear 1.0 import FlightGear.Launcher 1.0 @@ -22,6 +21,9 @@ Rectangle { property alias aircraft: aircraftInfo.uri property alias currentIndex: aircraftInfo.variant property alias fontPixelSize: title.font.pixelSize + + // allow users to control the font sizing + // (aircraft variants can have excessively long names) property int popupFontPixelSize: 0 // expose this to avoid a second AircraftInfo in the compact delegate @@ -30,17 +32,6 @@ Rectangle { signal selected(var index) - - - -// Image { -// id: upDownIcon -// source: "qrc:///up-down-arrow" -// x: root.centerX + Math.min(title.implicitWidth * 0.5, title.width * 0.5) -// anchors.verticalCenter: parent.verticalCenter -// visible: __enabled -// } - Row { anchors.centerIn: parent height: title.implicitHeight @@ -85,16 +76,9 @@ Rectangle { anchors.fill: parent hoverEnabled: true onClicked: { - var screenPos = _launcher.mapToGlobal(title, Qt.point(0, -title.height * currentIndex)) - if (screenPos.y < 0) { - // if the popup would appear off the screen, use the first item - // position instead - screenPos = _launcher.mapToGlobal(title, Qt.point(0, 0)) - } - - var pop = popup.createObject(root, {x:screenPos.x, y:screenPos.y }) - tracker.window = pop; - pop.show(); + var hCenter = root.width / 2; + OverlayShared.globalOverlay.showOverlayAtItemOffset(popup, root, + Qt.point(hCenter, root.height)) } } @@ -103,67 +87,17 @@ Rectangle { id: aircraftInfo } - PopupWindowTracker { - id: tracker - } - Component { id: popup - - Window { - id: popupFrame - - width: root.width - flags: Qt.Popup - height: choicesColumn.childrenRect.height - color: "white" - - Rectangle { - border.width: 1 - border.color: Style.minorFrameColor - anchors.fill: parent + VariantMenu { + id: menu + aircraftUri: aircraftInfo.uri + popupFontPixelSize: root.popupFontPixelSize + + onSelect: { + aircraftInfo.variant = index; + root.selected(index) } - - Column { - id: choicesColumn - - Repeater { - // would prefer the model to be conditional on visiblity, - // but this trips up the Window sizing on Linux (Ubuntu at - // least) and we get a mis-aligned origin - model: aircraftInfo.variantNames - - delegate: Item { - width: popupFrame.width - height: choiceText.implicitHeight - - Text { - id: choiceText - text: modelData - - // allow override the size in case the title size is enormous - font.pixelSize: (root.popupFontPixelSize > 0) ? root.popupFontPixelSize : title.font.pixelSize - - color: choiceArea.containsMouse ? Style.themeColor : Style.baseTextColor - anchors { - left: parent.left - right: parent.right - margins: 4 - } - } - - MouseArea { - id: choiceArea - hoverEnabled: true - anchors.fill: parent - onClicked: { - popupFrame.close() - root.selected(model.index) - } - } - } // of delegate Item - } // of Repeater - } - } // of popup frame + } } // of popup component } diff --git a/src/GUI/qml/HistoryPopup.qml b/src/GUI/qml/HistoryPopup.qml index 83aa58a50..35d1e2490 100644 --- a/src/GUI/qml/HistoryPopup.qml +++ b/src/GUI/qml/HistoryPopup.qml @@ -1,6 +1,6 @@ import QtQuick 2.4 -import QtQuick.Window 2.0 import FlightGear.Launcher 1.0 +import FlightGear 1.0 import "." @@ -37,68 +37,18 @@ Item { hoverEnabled: root.enabled enabled: root.enabled onClicked: { - var pop = popup.createObject(root) - var screenPos = _launcher.mapToGlobal(button, Qt.point(-pop.width, 0)) - pop.y = screenPos.y; - pop.x = screenPos.x; - tracker.window = pop; - pop.show(); + OverlayShared.globalOverlay.showOverlayAtItemOffset(menu, root, + Qt.point(root.width, root.height)) } } } - PopupWindowTracker { - id: tracker - } - Component { - id: popup - - Window { - id: popupFrame - - flags: Qt.Popup - height: choicesColumn.childrenRect.height + Style.margin * 2 - width: choicesColumn.childrenRect.width + Style.margin * 2 - color: "white" - - Rectangle { - border.width: 1 - border.color: Style.minorFrameColor - anchors.fill: parent - } - - // text repeater - Column { - id: choicesColumn - spacing: Style.margin - x: Style.margin - y: Style.margin - - Repeater { - id: choicesRepeater - model: root.model - delegate: - StyledText { - id: choiceText - - // Taken from TableViewItemDelegateLoader.qml to follow QML role conventions - text: model && model.hasOwnProperty(displayRole) ? model[displayRole] // Qml ListModel and QAbstractItemModel - : modelData && modelData.hasOwnProperty(displayRole) ? modelData[displayRole] // QObjectList / QObject - : modelData != undefined ? modelData : "" // Models without role - height: implicitHeight + Style.margin - - MouseArea { - width: popupFrame.width // full width of the popup - height: parent.height - onClicked: { - root.selected(model.index); - popupFrame.close() - } - } - } // of Text delegate - } // text repeater - } // text column - } // of popup Window + id: menu + OverlayMenu { + model: root.model + onSelect: root.selected(index) + alignRightEdge: true + } } } diff --git a/src/GUI/qml/NumericalEdit.qml b/src/GUI/qml/NumericalEdit.qml index 8cb3d934f..77fc1e476 100644 --- a/src/GUI/qml/NumericalEdit.qml +++ b/src/GUI/qml/NumericalEdit.qml @@ -1,6 +1,5 @@ import QtQuick 2.4 import FlightGear 1.0 -import QtQuick.Window 2.0 import FlightGear.Launcher 1.0 import "." @@ -89,10 +88,7 @@ FocusScope { units.selectedIndex = unitIndex; var newQuantity = q.convertToUnit(units.selectedUnit) - console.info("Changing text to:" + newQuantity.value.toFixed(units.numDecimals)); edit.text = newQuantity.value.toFixed(units.numDecimals) - - console.info("Change units commit:" + newQuantity.value) commit(newQuantity); } else { // not focused, easy @@ -103,11 +99,8 @@ FocusScope { function showUnitsMenu() { - - var screenPos = _launcher.mapToGlobal(editFrame, Qt.point(0, editFrame.height)) - var pop = popup.createObject(root, {x:screenPos.x, y:screenPos.y }) - tracker.window = pop; - pop.show(); + OverlayShared.globalOverlay.showOverlayAtItemOffset(menu, root, + Qt.point(editFrame.x, editFrame.height)) } Component.onCompleted: { @@ -329,73 +322,14 @@ FocusScope { } } // of frame rectangle - PopupWindowTracker { - id: tracker - } - Component { - id: popup - Window { - id: unitSelectionPopup - flags: Qt.Popup - color: "white" - height: choicesColumn.childrenRect.height + Style.margin * 2 - width: choicesColumn.width + Style.margin * 2 + id: menu - Rectangle { - border.width: 1 - border.color: Style.minorFrameColor - anchors.fill: parent - } - - // choice layout column - Column { - id: choicesColumn - spacing: Style.margin - x: Style.margin - y: Style.margin - width: menuWidth - - - function calculateMenuWidth() - { - var minWidth = 0; - for (var i = 0; i < choicesRepeater.count; i++) { - minWidth = Math.max(minWidth, choicesRepeater.itemAt(i).implicitWidth); - } - return minWidth; - } - - readonly property int menuWidth: calculateMenuWidth() - - // main item repeater - Repeater { - id: choicesRepeater - model: units - delegate: - Text { - id: choiceText - readonly property bool selected: units.selectedIndex === model.index - - text: model.longName - height: implicitHeight + Style.margin - font.pixelSize: Style.baseFontPixelSize - color: choiceArea.containsMouse ? Style.themeColor : Style.baseTextColor - - MouseArea { - id: choiceArea - width: unitSelectionPopup.width // full width of the popup - height: parent.height - hoverEnabled: true - - onClicked: { - root.changeToUnits(model.index); - unitSelectionPopup.close(); - } - } - } // of Text delegate - } // text repeater - } // text column + OverlayMenu { + model: units + displayRole: "longName" + currentIndex: units.selectedIndex + onSelect: root.changeToUnits(index); } - } // of popup component + } } diff --git a/src/GUI/qml/Overlay.qml b/src/GUI/qml/Overlay.qml index 611559406..ad57b92bf 100644 --- a/src/GUI/qml/Overlay.qml +++ b/src/GUI/qml/Overlay.qml @@ -6,19 +6,25 @@ Item { readonly property bool dismissOnClickOutside: activeOverlayLoader.item && activeOverlayLoader.item.hasOwnProperty("dismissOnClickOutside") ? activeOverlayLoader.item.dismissOnClickOutside : false + property var __showPos: Qt.point(0,0) + function showOverlay(comp) { + __showPos = Qt.point(0,0) activeOverlayLoader.sourceComponent = comp; activeOverlayLoader.visible = true; + activeOverlayLoader.x = __showPos.x; + activeOverlayLoader.y = __showPos.y; } function showOverlayAtItemOffset(comp, item, offset) { - var pt = mapFromItem(item, offset.x, offset.y) + __showPos = mapFromItem(item, offset.x, offset.y) + activeOverlayLoader.sourceComponent = comp; activeOverlayLoader.visible = true; - activeOverlayLoader.x = pt.x; - activeOverlayLoader.y = pt.y; + activeOverlayLoader.x = __showPos.x; + activeOverlayLoader.y = __showPos.y; } function dismissOverlay() @@ -27,6 +33,14 @@ Item { activeOverlayLoader.visible = false; } + function verticalOverflowFor(itemHeight) + { + // assumes we fill the whole window, so anything + // extending past our bottom is overflowing + var ov = (itemHeight + activeOverlayLoader.y) - root.height; + return Math.max(ov, 0); + } + MouseArea { anchors.fill: parent hoverEnabled: true @@ -38,29 +52,5 @@ Item { { id: activeOverlayLoader // no size, size to the component - - onStatusChanged: { - if (status == Loader.Ready) { - if (item.hasOwnProperty("adjustYPosition")) { - // setting position here doesn't work, so we use a 0-interval - // timer to do it shortly afterwards - adjustPosition.start(); - } - } - } - - function adjustY() - { - var overflowHeight = (y + item.height) - root.height; - if (overflowHeight > 0) { - activeOverlayLoader.y = Math.max(0, y - overflowHeight) - } - } - - Timer { - id: adjustPosition - interval: 0 - onTriggered: activeOverlayLoader.adjustY(); - } } } diff --git a/src/GUI/qml/OverlayMenu.qml b/src/GUI/qml/OverlayMenu.qml index fbe293631..9b08e938f 100644 --- a/src/GUI/qml/OverlayMenu.qml +++ b/src/GUI/qml/OverlayMenu.qml @@ -9,6 +9,7 @@ Rectangle { property int currentIndex: 0 property string headerText: "" property int maximumPermittedHeight: 450 + property string displayRole: "display" color: "white" border.width: 1 @@ -19,7 +20,7 @@ Rectangle { // Overlay control properties property bool dismissOnClickOutside: true - property bool adjustYPosition: true + property bool alignRightEdge: false signal select(var index); @@ -27,6 +28,9 @@ Rectangle { computeMenuWidth(); } + y: -OverlayShared.globalOverlay.verticalOverflowFor(height) + x: alignRightEdge ? -width : 0 + function close() { OverlayShared.globalOverlay.dismissOverlay() diff --git a/src/GUI/qml/VariantMenu.qml b/src/GUI/qml/VariantMenu.qml new file mode 100644 index 000000000..3dd691ec9 --- /dev/null +++ b/src/GUI/qml/VariantMenu.qml @@ -0,0 +1,96 @@ +import QtQuick 2.4 +import FlightGear 1.0 +import FlightGear.Launcher 1.0 + +import "." + +Rectangle { + id: root + + property alias currentIndex: aircraft.variant + property alias aircraftUri: aircraft.uri + + AircraftInfo { + id: aircraft + } + + color: "white" + border.width: 1 + border.color: Style.minorFrameColor + width: itemsColumn.width + height: itemsColumn.height + clip: true + + property int popupFontPixelSize: 0 + + // Overlay control properties + property bool dismissOnClickOutside: true + + signal select(var index); + + y: -OverlayShared.globalOverlay.verticalOverflowFor(height) + x: -width / 2 // align horizontal center + + Component.onCompleted: { + computeMenuWidth(); + } + + function close() + { + OverlayShared.globalOverlay.dismissOverlay() + } + + function doSelect(index) + { + select(index); + close(); + } + + function computeMenuWidth() + { + var minWidth = 0; + for (var i = 0; i < choicesRepeater.count; i++) { + minWidth = Math.max(minWidth, choicesRepeater.itemAt(i).implicitWidth); + } + + itemsColumn.width = minWidth + } + + Column { + id: itemsColumn + spacing: Style.margin + + Repeater { + id: choicesRepeater + model: aircraft.variantNames + delegate: Item { + width: itemsColumn.width + height: choiceText.implicitHeight + implicitWidth: choiceText.implicitWidth + (Style.margin * 2) + + Text { + id: choiceText + text: modelData + x: Style.margin + width: parent.width - (Style.margin * 2) + + // allow override the size in case the title size is enormous + font.pixelSize: (root.popupFontPixelSize > 0) ? root.popupFontPixelSize + : Style.baseFontPixelSize * 2 + + color: choiceArea.containsMouse ? Style.themeColor : Style.baseTextColor + } + + MouseArea { + id: choiceArea + hoverEnabled: true + anchors.fill: parent + onClicked: { + root.doSelect(model.index) + } + } + } // of delegate Item + } // menu item repeater + } // of menu contents column + +} diff --git a/src/GUI/resources.qrc b/src/GUI/resources.qrc index eed10409e..00ec2557c 100644 --- a/src/GUI/resources.qrc +++ b/src/GUI/resources.qrc @@ -132,6 +132,7 @@ qml/EnableDisableButton.qml qml/IconButton.qml qml/FavouriteToggleButton.qml + qml/VariantMenu.qml assets/icons8-christmas-star-filled.png assets/icons8-christmas-star-outline.png