Unity 8

Merge lp:~aacid/unity8/makeSureMenuPositionOnScreen into lp:unity8

makeSureMenuPositionOnScreen
Merge into trunk

Proposed by Albert Astals Cid on 2017-01-24

Status:	Superseded
Proposed branch:	lp:~aacid/unity8/makeSureMenuPositionOnScreen
Merge into:	lp:unity8
Prerequisite:	lp:~nick-dedekind/unity8/menu.width.fix
Diff against target:	408 lines (+252/-24) 6 files modified qml/ApplicationMenus/ApplicationMenusLimits.qml (+24/-0) qml/ApplicationMenus/MenuBar.qml (+10/-3) qml/ApplicationMenus/MenuPopup.qml (+67/-21) qml/ApplicationMenus/qmldir (+1/-0) tests/qmltests/ApplicationMenus/tst_MenuBar.qml (+50/-0) tests/qmltests/Stage/tst_DesktopStage.qml (+100/-0)
To merge this branch:	bzr merge lp:~aacid/unity8/makeSureMenuPositionOnScreen
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Unity8 CI Bot	continuous-integration		Approve on 2017-01-26
Unity Team		2017-01-24	Pending
Review via email: mp+315470@code.launchpad.net

This proposal has been superseded by a proposal from 2017-01-26.

Commit message

Make the menus and submenus do not go outside the screen when popping out

Description of the change

* Are there any related MPs required for this MP to build/function as expected?
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

lp:~aacid/unity8/makeSureMenuPositionOnScreen updated on 2017-01-24

2773. By Albert Astals Cid on 2017-01-24: Now with better files!

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-01-24:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3011/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-01-24:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3013/rebuild

review: Needs Fixing (continuous-integration)

lp:~aacid/unity8/makeSureMenuPositionOnScreen updated on 2017-01-25

2774. By Albert Astals Cid on 2017-01-25

Do not use exact gu values for the compares

Since there's text involved the gu values are not the same depending your px per gu, so do it in a bit of more fuzzy way

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-01-25:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3014/rebuild

review: Approve (continuous-integration)

lp:~aacid/unity8/makeSureMenuPositionOnScreen updated on 2017-01-26

2775. By Albert Astals Cid on 2017-01-26: Merge
2776. By Albert Astals Cid on 2017-01-26: Merge clickOpenMenuClosesIt

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-01-26:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3035/rebuild

review: Approve (continuous-integration)

lp:~aacid/unity8/makeSureMenuPositionOnScreen updated on 2017-02-07

2777. By Albert Astals Cid on 2017-02-07: Merge

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Albert Astals Cid

Mohammad Rakibul Hasan

Unity Team

 === added file 'qml/ApplicationMenus/ApplicationMenusLimits.qml'
 --- qml/ApplicationMenus/ApplicationMenusLimits.qml	1970-01-01 00:00:00 +0000
 +++ qml/ApplicationMenus/ApplicationMenusLimits.qml	2017-01-26 09:29:42 +0000
@@ -0,0 +1,24 @@
++/*
++ * Copyright (C) 2017 Canonical, Ltd.
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++pragma Singleton
++import QtQuick 2.4
++import QtQuick.Window 2.2
++
++QtObject {
++    property real screenWidth: Screen.width
++    property real screenHeight: Screen.height
++}
 === modified file 'qml/ApplicationMenus/MenuBar.qml'
 --- qml/ApplicationMenus/MenuBar.qml	2017-01-17 09:45:22 +0000
 +++ qml/ApplicationMenus/MenuBar.qml	2017-01-26 09:29:42 +0000
@@ -114,6 +114,7 @@
                      function show() {
                          if (!__popup) {
                              __popup = menuComponent.createObject(root, { objectName: visualItem.objectName + "-menu" });
++                            __popup.childActivated.connect(dismiss);
                              // force the current item to be the newly popped up menu
                          } else {
                              __popup.show();
@@ -148,8 +149,8 @@
                      Component {
                          id: menuComponent
                          MenuPopup {
--                            x: visualItem.x - units.gu(1)
--                            anchors.top: parent.bottom
++                            desiredX: visualItem.x - units.gu(1)
++                            desiredY: parent.height
                              unityMenuModel: root.unityMenuModel.submenu(visualItem.__ownIndex)
                              Component.onCompleted: reset();
@@ -212,7 +213,13 @@
                      updateCurrentItemFromPosition(Qt.point(mouse.x, mouse.y))
+                 }
+             }
--            onClicked: updateCurrentItemFromPosition(Qt.point(mouse.x, mouse.y))
++            onClicked: {
++                var prevItem = d.currentItem;
++                updateCurrentItemFromPosition(Qt.point(mouse.x, mouse.y))
++                if (prevItem && d.currentItem == prevItem) {
++                    prevItem.hide();
++                }
++            }
              function updateCurrentItemFromPosition(point) {
                  var pos = mapToItem(row, point.x, point.y);
 === modified file 'qml/ApplicationMenus/MenuPopup.qml'
 --- qml/ApplicationMenus/MenuPopup.qml	2017-01-18 12:08:05 +0000
 +++ qml/ApplicationMenus/MenuPopup.qml	2017-01-26 09:29:42 +0000
@@ -16,16 +16,50 @@
  import QtQuick 2.4
  import QtQuick.Layouts 1.1
--import QtQuick.Window 2.2
  import Ubuntu.Components 1.3
  import Ubuntu.Components.ListItems 1.3 as ListItems
  import "../Components"
++import "."
  UbuntuShape {
      id: root
      objectName: "menu"
      backgroundColor: theme.palette.normal.overlay
++    signal childActivated()
++
++    // true for submenus that need to show on the other side of their parent
++    // if they don't fit when growing right
++    property bool substractWidth: false
++
++    property real desiredX
++    x: {
++        var dummy = visible; // force recalc when shown/hidden
++        var parentTopLeft = parent.mapToItem(null, 0, 0);
++        var farX = ApplicationMenusLimits.screenWidth;
++        if (parentTopLeft.x + width + desiredX <= farX) {
++            return desiredX;
++        } else {
++            if (substractWidth) {
++                return -width;
++            } else {
++                return farX - parentTopLeft.x - width;
++            }
++        }
++    }
++
++    property real desiredY
++    y: {
++        var dummy = visible; // force recalc when shown/hidden
++        var parentTopLeft = parent.mapToItem(null, 0, 0);
++        var bottomY = ApplicationMenusLimits.screenHeight;
++        if (parentTopLeft.y + height + desiredY <= bottomY) {
++            return desiredY;
++        } else {
++            return bottomY - parentTopLeft.y - height;
++        }
++    }
++
      property alias unityMenuModel: repeater.model
      function show() {
@@ -64,9 +98,9 @@
          readonly property int currentIndex: currentItem ? currentItem.__ownIndex : -1
          property real __minimumWidth: units.gu(20)
--        property real __maximumWidth: Screen.width * 0.7
++        property real __maximumWidth: ApplicationMenusLimits.screenWidth * 0.7
          property real __minimumHeight: units.gu(2)
--        property real __maximumHeight: Screen.height * 0.7
++        property real __maximumHeight: ApplicationMenusLimits.screenHeight * 0.7
          signal dismissAll()
@@ -257,22 +291,29 @@
                                                  popup = submenuComponent.createObject(focusScope, {
                                                                                            objectName: loader.objectName + "-",
                                                                                            unityMenuModel: model,
--                                                                                          x: Qt.binding(function() { return root.width }),
--                                                                                          y: Qt.binding(function() {
++                                                                                          substractWidth: true,
++                                                                                          desiredX: Qt.binding(function() { return root.width }),
++                                                                                          desiredY: Qt.binding(function() {
                                                                                                var dummy = listView.contentY; // force a recalc on contentY change.
                                                                                                return mapToItem(container, 0, y).y;
                                                                                            })
                                                                                        });
++                                                popup.retreat.connect(function() {
++                                                    popup.destroy();
++                                                    popup = null;
++                                                    menuItem.forceActiveFocus();
++                                                });
++                                                popup.childActivated.connect(function() {
++                                                    popup.destroy();
++                                                    popup = null;
++                                                    root.childActivated();
++                                                });
                                              } else if (popup) {
                                                  popup.visible = true;
+                                             }
--                                            popup.retreat.connect(function() {
--                                                popup.destroy();
--                                                popup = null;
--                                                menuItem.forceActiveFocus();
--                                            })
                                          } else {
                                              root.unityMenuModel.activate(__ownIndex);
++                                            root.childActivated();
+                                         }
+                                     }
@@ -375,23 +416,28 @@
                  id: submenuLoader
                  source: "MenuPopup.qml"
++                property real desiredX
++                property real desiredY
++                property bool substractWidth
                  property var unityMenuModel: null
                  signal retreat()
--
--                Binding {
--                    target: item
--                    property: "unityMenuModel"
--                    value: submenuLoader.unityMenuModel
--                }
--
--                Binding {
--                    target: item
--                    property: "objectName"
--                    value: submenuLoader.objectName + "menu"
++                signal childActivated()
++
++                onLoaded: {
++                    item.unityMenuModel = Qt.binding(function() { return submenuLoader.unityMenuModel; });
++                    item.objectName = Qt.binding(function() { return submenuLoader.objectName + "menu"; });
++                    item.desiredX = Qt.binding(function() { return submenuLoader.desiredX; });
++                    item.desiredY = Qt.binding(function() { return submenuLoader.desiredY; });
++                    item.substractWidth = Qt.binding(function() { return submenuLoader.substractWidth; });
+                 }
                  Keys.onLeftPressed: retreat()
++                Connections {
++                    target: item
++                    onChildActivated: childActivated();
++                }
++
                  Component.onCompleted: item.select(0);
                  onVisibleChanged: if (visible) { item.select(0); }
+             }
 === added file 'qml/ApplicationMenus/qmldir'
 --- qml/ApplicationMenus/qmldir	1970-01-01 00:00:00 +0000
 +++ qml/ApplicationMenus/qmldir	2017-01-26 09:29:42 +0000
@@ -0,0 +1,1 @@
++singleton ApplicationMenusLimits 0.1 ApplicationMenusLimits.qml
 === modified file 'tests/qmltests/ApplicationMenus/tst_MenuBar.qml'
 --- tests/qmltests/ApplicationMenus/tst_MenuBar.qml	2017-01-24 07:41:35 +0000
 +++ tests/qmltests/ApplicationMenus/tst_MenuBar.qml	2017-01-26 09:29:42 +0000
@@ -180,6 +180,41 @@
              tryCompare(priv, "currentItem", menuItem);
+         }
++        function test_menuActivateClosesMenu() {
++            menuBackend.modelData = appMenuData.generateTestData(3,3,0,0,"menu");
++            var priv = findInvisibleChild(menuBar, "d");
++
++            var menuItem = findChild(menuBar, "menuBar-item0");
++            menuItem.show();
++            compare(priv.currentItem, menuItem, "CurrentItem should be set to item 0");
++            compare(priv.currentItem.popupVisible, true, "Popup should be visible");
++
++            var actionItem = findChild(menuBar, "menuBar-item0-menu-item0-actionItem");
++            mouseClick(actionItem);
++            compare(priv.currentItem, null, "CurrentItem should be null");
++        }
++
++        function test_subMenuActivateClosesMenu() {
++            menuBackend.modelData = appMenuData.generateTestData(3,4,1,0,"menu");
++            var priv = findInvisibleChild(menuBar, "d");
++
++            var menuItem = findChild(menuBar, "menuBar-item0");
++            menuItem.show();
++            compare(priv.currentItem, menuItem, "CurrentItem should be set to item 0");
++            compare(priv.currentItem.popupVisible, true, "Popup should be visible");
++
++            var actionItem = findChild(menuBar, "menuBar-item0-menu-item0-actionItem");
++            mouseClick(actionItem);
++
++            actionItem = findChild(menuBar, "menuBar-item0-menu-item0-menu-item0-actionItem");
++            mouseClick(actionItem);
++
++            actionItem = findChild(menuBar, "menuBar-item0-menu-item0-menu-item0-menu-item0-actionItem");
++            mouseClick(actionItem);
++
++            compare(priv.currentItem, null, "CurrentItem should be null");
++        }
++
          function test_openAppMenuShortcut() {
              var priv = findInvisibleChild(menuBar, "d");
@@ -192,5 +227,20 @@
              keyClick(Qt.Key_F10, Qt.AltModifier);
              compare(priv.currentItem, menuItem1, "First enabled item should be opened");
+         }
++
++        function test_clickOpenMenuClosesMenu() {
++            menuBackend.modelData = appMenuData.generateTestData(3,3,0,0,"menu");
++            var priv = findInvisibleChild(menuBar, "d");
++
++            var menuItem = findChild(menuBar, "menuBar-item0");
++            waitForRendering(menuItem);
++            mouseClick(menuItem);
++            compare(priv.currentItem, menuItem, "CurrentItem should be set to item 0");
++            compare(priv.currentItem.popupVisible, true, "Popup should be visible");
++
++            waitForRendering(menuItem);
++            mouseClick(menuItem);
++            compare(priv.currentItem, null, "CurrentItem should be null");
++        }
+     }
+ }
 === modified file 'tests/qmltests/Stage/tst_DesktopStage.qml'
 --- tests/qmltests/Stage/tst_DesktopStage.qml	2017-01-10 14:44:29 +0000
 +++ tests/qmltests/Stage/tst_DesktopStage.qml	2017-01-26 09:29:42 +0000
@@ -29,6 +29,7 @@
  import "../../../qml/Stage"
  import "../../../qml/Components"
  import "../../../qml/Components/PanelState"
++import "../../../qml/ApplicationMenus"
  Item {
      id: root
@@ -44,6 +45,8 @@
+     }
      Component.onCompleted: {
++        ApplicationMenusLimits.screenWidth = Qt.binding( function() { return stageLoader.width; } );
++        ApplicationMenusLimits.screenHeight = Qt.binding( function() { return stageLoader.height; } );
          QuickUtils.keyboardAttached = true;
          theme.name = "Ubuntu.Components.Themes.SuruDark";
          resetGeometry();
@@ -830,5 +833,102 @@
              mouseRelease(decoration);
              tryCompare(Mir, "cursorName", "");
+         }
++
++        function test_menuPositioning_data() {
++            return [
++                {tag: "good",
++                    windowPosition: Qt.point(units.gu(10),  units.gu(10))
++                },
++                {tag: "collides right",
++                    windowPosition: Qt.point(units.gu(100), units.gu(10)),
++                    minimumXDifference: units.gu(8)
++                },
++                {tag: "collides bottom",
++                    windowPosition: Qt.point(units.gu(10),  units.gu(80)),
++                    minimumYDifference: units.gu(7)
++                },
++            ]
++        }
++
++        function test_menuPositioning(data) {
++            var appDelegate = startApplication("dialer-app");
++            appDelegate.windowedX = data.windowPosition.x;
++            appDelegate.windowedY = data.windowPosition.y;
++
++            var menuItem = findChild(appDelegate, "menuBar-item3");
++            menuItem.show();
++
++            var menu = findChild(appDelegate, "menuBar-item3-menu");
++            tryCompare(menu, "visible", true);
++
++            var normalPositioningX = menuItem.x - units.gu(1);
++            var normalPositioningY = menuItem.height;
++
++            // We do this fuzzy checking because otherwise we would be duplicating the code
++            // that calculates the coordinates and any bug it may have, what we want is really
++            // to check that on collision with the border the menu is shifted substantially
++            if (data.minimumXDifference) {
++                verify(menu.x < normalPositioningX - data.minimumXDifference);
++            } else {
++                compare(menu.x, normalPositioningX);
++            }
++
++            if (data.minimumYDifference) {
++                verify(menu.y < normalPositioningY - data.minimumYDifference);
++            } else {
++                compare(menu.y, normalPositioningY);
++            }
++        }
++
++        function test_submenuPositioning_data() {
++            return [
++                {tag: "good",
++                    windowPosition: Qt.point(units.gu(10),  units.gu(10))
++                },
++                {tag: "collides right",
++                    windowPosition: Qt.point(units.gu(100), units.gu(10)),
++                    minimumXDifference: units.gu(35)
++                },
++                {tag: "collides bottom",
++                    windowPosition: Qt.point(units.gu(10),  units.gu(80)),
++                    minimumYDifference: units.gu(8)
++                },
++            ]
++        }
++
++        function test_submenuPositioning(data) {
++            var appDelegate = startApplication("dialer-app");
++            appDelegate.windowedX = data.windowPosition.x;
++            appDelegate.windowedY = data.windowPosition.y;
++
++            var menuItem = findChild(appDelegate, "menuBar-item3");
++            menuItem.show();
++
++            var menu = findChild(appDelegate, "menuBar-item3-menu");
++            menuItem = findChild(menu, "menuBar-item3-menu-item3-actionItem");
++            tryCompare(menuItem, "visible", true);
++            mouseMove(menuItem);
++            mouseClick(menuItem);
++
++            menu = findChild(appDelegate, "menuBar-item3-menu-item3-menu");
++
++            var normalPositioningX = menuItem.width;
++            var normalPositioningY = menuItem.parent.y;
++
++            // We do this fuzzy checking because otherwise we would be duplicating the code
++            // that calculates the coordinates and any bug it may have, what we want is really
++            // to check that on collision with the border the menu is shifted substantially
++            if (data.minimumXDifference) {
++                verify(menu.x < normalPositioningX - data.minimumXDifference);
++            } else {
++                compare(menu.x, normalPositioningX);
++            }
++
++            if (data.minimumYDifference) {
++                verify(menu.y < normalPositioningY - data.minimumYDifference);
++            } else {
++                compare(menu.y, normalPositioningY);
++            }
++        }
+     }
+ }