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

[Albert Astals Cid]

Make the MenuPopup size be the one of the biggest menu

Don't like this solution much since it involves saving the delegates of the list, but seems to work

See https://code.launchpad.net/~aacid/unity8/correctly_sized_menus_v2/+merge/314927 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/2917/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3806
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2203
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2203
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3834
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3677
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3677/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3677
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3677/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3677
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3677/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3677
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3677/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3677
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3677/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3677
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3677/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2917/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

Don't like this solution much since it involves saving the delegates of the list, but seems to work

=== modified file 'qml/ApplicationMenus/MenuPopup.qml'
--- qml/ApplicationMenus/MenuPopup.qml	2017-01-09 15:26:05 +0000
+++ qml/ApplicationMenus/MenuPopup.qml	2017-01-17 14:57:53 +0000
@@ -154,9 +154,42 @@
                 objectName: "listView"
                 Layout.fillHeight: true
                 Layout.fillWidth: true
-                contentWidth: MathUtils.clamp(contentItem.childrenRect.width,
-                                              __minimumWidth,
-                                              __maximumWidth)
+
+                property var items: []
+
+                // Batch updateContentWidthCalls
+                Timer {
+                    id: updateContentWidthTimer
+                    interval: 0
+                    onTriggered: listView.realUpdateContentWidth();
+                }
+
+                function newItem(item) {
+                    items.push(item);
+                    updateContentWidthTimer.start();
+                }
+
+                function removeItem(item) {
+                    var index = items.indexOf(item);
+                    if(index != -1) {
+                        items.splice(index, 1);
+                        updateContentWidthTimer.start();
+                    }
+                }
+
+                function updateContentWidth() {
+                    updateContentWidthTimer.start();
+                }
+
+                function realUpdateContentWidth() {
+                    var maxChildrenWidth = 0;
+
+                    for (var i in items) {
+                        maxChildrenWidth = Math.max(maxChildrenWidth, items[i].implicitWidth);
+                    }
+
+                    contentWidth = MathUtils.clamp(maxChildrenWidth, __minimumWidth, __maximumWidth)
+                }
 
                 orientation: Qt.Vertical
                 interactive: contentHeight > height
@@ -222,7 +255,7 @@
 
                     property int __ownIndex: index
 
-                    width: root.width
+                    width: listView.contentWidth
                     enabled: model.isSeparator ? false : model.sensitive
 
                     sourceComponent: {
@@ -289,6 +322,10 @@
                             objectName: loader.objectName + "-separator"
                         }
                     }
+
+                    Component.onCompleted: listView.newItem(loader);
+                    Component.onDestruction: listView.removeItem(loader);
+                    onImplicitWidthChanged: listView.updateContentWidth();
                 }
             } // ListView
 

=== modified file 'tests/qmltests/ApplicationMenuDataLoader.qml'
--- tests/qmltests/ApplicationMenuDataLoader.qml	2017-01-06 14:07:36 +0000
+++ tests/qmltests/ApplicationMenuDataLoader.qml	2017-01-17 14:57:53 +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 14:57:53 +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);
+        }
     }
 }