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

[Albert Astals Cid]

Make the MenuPopup size be the one of the biggest menu

Sadly introduces a binding loop, but we need it

See https://code.launchpad.net/~aacid/unity8/correctly_sized_menus/+merge/314920 for an alternative solution

* 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

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:2769
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2923/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3814
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2210
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2210
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3842
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3685
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3685/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3685
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3685/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3685
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3685/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3685
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3685/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3685
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3685/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3685
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3685/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Albert Astals Cid (aacid)]

Rejected in favour of nick's branch https://code.launchpad.net/~nick-dedekind/unity8/menu.width.fix/+merge/315021

Unmerged revisions

2769. By Albert Astals Cid on 2017-01-17

Make the MenuPopup size be the one of the biggest menu

Sadly introduces a binding loop, but we need it

=== modified file 'qml/ApplicationMenus/MenuPopup.qml'
--- qml/ApplicationMenus/MenuPopup.qml	2017-01-09 15:26:05 +0000
+++ qml/ApplicationMenus/MenuPopup.qml	2017-01-17 16:04:15 +0000
@@ -154,10 +154,7 @@
                 objectName: "listView"
                 Layout.fillHeight: true
                 Layout.fillWidth: true
-                contentWidth: MathUtils.clamp(contentItem.childrenRect.width,
-                                              __minimumWidth,
-                                              __maximumWidth)
-
+                contentWidth: contentItem.childrenRect.width
                 orientation: Qt.Vertical
                 interactive: contentHeight > height
                 clip: interactive
@@ -222,7 +219,13 @@
 
                     property int __ownIndex: index
 
-                    width: root.width
+                    width: {
+                        if (listView.contentWidth > implicitWidth) {
+                            return listView.contentWidth;
+                        } else {
+                            return MathUtils.clamp(implicitWidth, __minimumWidth, __maximumWidth)
+                        }
+                    }
                     enabled: model.isSeparator ? false : model.sensitive
 
                     sourceComponent: {

=== modified file 'tests/qmltests/ApplicationMenuDataLoader.qml'
--- tests/qmltests/ApplicationMenuDataLoader.qml	2017-01-06 14:07:36 +0000
+++ tests/qmltests/ApplicationMenuDataLoader.qml	2017-01-17 16:04:15 +0000
@@ -72,4 +72,63 @@
                 "shortcut": "Alt+F"
             }
         }]
+
+    property var differentSizesMenu: [{
+                "rowData": {                // 1.1
+                    "label": "Short",
+                    "sensitive": true,
+                    "isSeparator": false,
+                    "icon": "",
+                    "type": "com.canonical.indicator.test",
+                    "ext": {},
+                    "action": "menu0",
+                    "actionState": {},
+                    "isCheck": false,
+                    "isRadio": false,
+                    "isToggled": false,
+                    "shortcut": "Alt+F"
+                }}, {
+               "rowData": {                // 1.2
+                   "label": "This is Medium",
+                   "sensitive": true,
+                   "isSeparator": false,
+                   "icon": "",
+                   "type": "com.canonical.indicator.test",
+                   "ext": {},
+                   "action": "menu1",
+                   "actionState": {},
+                   "isCheck": false,
+                   "isRadio": false,
+                   "isToggled": false,
+               }}, {
+               "rowData": {                // row 1.2
+                   "label": "This is Medium v2",
+                   "sensitive": true,
+                   "isSeparator": false,
+                   "icon": "",
+                   "type": "com.canonical.indicator.test",
+                   "ext": {},
+                   "action": "menu2",
+                   "actionState": {},
+                   "isCheck": false,
+                   "isRadio": false,
+                   "isToggled": false,
+                    "shortcut": "Alt+G"
+               }}, {
+               "rowData": {                // row 1.2
+                   "label": "This is really Looong",
+                   "sensitive": true,
+                   "isSeparator": false,
+                   "icon": "",
+                   "type": "com.canonical.indicator.test",
+                   "ext": {},
+                   "action": "menu2",
+                   "actionState": {},
+                   "isCheck": false,
+                   "isRadio": false,
+                   "isToggled": false,
+                    "shortcut": "Ctrl+Shift+T"
+               }}
+            ];
+
 }

=== modified file 'tests/qmltests/ApplicationMenus/tst_MenuPopup.qml'
--- tests/qmltests/ApplicationMenus/tst_MenuPopup.qml	2017-01-09 15:26:05 +0000
+++ tests/qmltests/ApplicationMenus/tst_MenuPopup.qml	2017-01-17 16:04:15 +0000
@@ -90,6 +90,7 @@
                 // recurse into submenu
                 var submenu = rows[i]["submenu"];
                 if (submenu) {
+                    waitForRendering(menuItem);
                     mouseClick(menuItem, menuItem.width/2, menuItem.height/2);
                     tryCompare(menuPriv, "currentItem", menuItem);
 
@@ -203,5 +204,19 @@
             keyClick(Qt.Key_Left, Qt.NoModifier);
             tryCompareFunction(function() { return menuItem.popup !== null && menuItem.popup.visible }, false);
         }
+
+        function test_differentSizes() {
+            menuBackend.modelData = appMenuData.differentSizesMenu;
+
+            waitForRendering(page);
+            var longWidth = page.width;
+
+            var menuData = appMenuData.differentSizesMenu;
+            menuData.pop();
+            menuBackend.modelData = menuData;
+
+            waitForRendering(page);
+            verify(page.width < longWidth);
+        }
     }
 }