Unity 8

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

  1. correctly_sized_menus_v2
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Rejected
Rejected by: Albert Astals Cid
Proposed branch: lp:~aacid/unity8/correctly_sized_menus_v2
Merge into: lp:unity8
Diff against target: 130 lines (+82/-5)
3 files modified
qml/ApplicationMenus/MenuPopup.qml (+8/-5)
tests/qmltests/ApplicationMenuDataLoader.qml (+59/-0)
tests/qmltests/ApplicationMenus/tst_MenuPopup.qml (+15/-0)
To merge this branch: bzr merge lp:~aacid/unity8/correctly_sized_menus_v2
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Approve
Unity Team Pending
Review via email: mp+314927@code.launchpad.net

Commit message

Make the MenuPopup size be the one of the biggest menu

Sadly introduces a binding loop, but we need it

Description of the change

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

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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)
Revision history for this message
Albert Astals Cid (aacid) wrote :

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

Make the MenuPopup size be the one of the biggest menu

Sadly introduces a binding loop, but we need it

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
154 objectName: "listView"154 objectName: "listView"
155 Layout.fillHeight: true155 Layout.fillHeight: true
156 Layout.fillWidth: true156 Layout.fillWidth: true
157 contentWidth: MathUtils.clamp(contentItem.childrenRect.width,157 contentWidth: contentItem.childrenRect.width
158 __minimumWidth,
159 __maximumWidth)
160
161 orientation: Qt.Vertical158 orientation: Qt.Vertical
162 interactive: contentHeight > height159 interactive: contentHeight > height
163 clip: interactive160 clip: interactive
@@ -222,7 +219,13 @@
222219
223 property int __ownIndex: index220 property int __ownIndex: index
224221
225 width: root.width222 width: {
223 if (listView.contentWidth > implicitWidth) {
224 return listView.contentWidth;
225 } else {
226 return MathUtils.clamp(implicitWidth, __minimumWidth, __maximumWidth)
227 }
228 }
226 enabled: model.isSeparator ? false : model.sensitive229 enabled: model.isSeparator ? false : model.sensitive
227230
228 sourceComponent: {231 sourceComponent: {
229232
=== 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 @@
72 "shortcut": "Alt+F"72 "shortcut": "Alt+F"
73 }73 }
74 }]74 }]
75
76 property var differentSizesMenu: [{
77 "rowData": { // 1.1
78 "label": "Short",
79 "sensitive": true,
80 "isSeparator": false,
81 "icon": "",
82 "type": "com.canonical.indicator.test",
83 "ext": {},
84 "action": "menu0",
85 "actionState": {},
86 "isCheck": false,
87 "isRadio": false,
88 "isToggled": false,
89 "shortcut": "Alt+F"
90 }}, {
91 "rowData": { // 1.2
92 "label": "This is Medium",
93 "sensitive": true,
94 "isSeparator": false,
95 "icon": "",
96 "type": "com.canonical.indicator.test",
97 "ext": {},
98 "action": "menu1",
99 "actionState": {},
100 "isCheck": false,
101 "isRadio": false,
102 "isToggled": false,
103 }}, {
104 "rowData": { // row 1.2
105 "label": "This is Medium v2",
106 "sensitive": true,
107 "isSeparator": false,
108 "icon": "",
109 "type": "com.canonical.indicator.test",
110 "ext": {},
111 "action": "menu2",
112 "actionState": {},
113 "isCheck": false,
114 "isRadio": false,
115 "isToggled": false,
116 "shortcut": "Alt+G"
117 }}, {
118 "rowData": { // row 1.2
119 "label": "This is really Looong",
120 "sensitive": true,
121 "isSeparator": false,
122 "icon": "",
123 "type": "com.canonical.indicator.test",
124 "ext": {},
125 "action": "menu2",
126 "actionState": {},
127 "isCheck": false,
128 "isRadio": false,
129 "isToggled": false,
130 "shortcut": "Ctrl+Shift+T"
131 }}
132 ];
133
75}134}
76135
=== 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 @@
90 // recurse into submenu90 // recurse into submenu
91 var submenu = rows[i]["submenu"];91 var submenu = rows[i]["submenu"];
92 if (submenu) {92 if (submenu) {
93 waitForRendering(menuItem);
93 mouseClick(menuItem, menuItem.width/2, menuItem.height/2);94 mouseClick(menuItem, menuItem.width/2, menuItem.height/2);
94 tryCompare(menuPriv, "currentItem", menuItem);95 tryCompare(menuPriv, "currentItem", menuItem);
9596
@@ -203,5 +204,19 @@
203 keyClick(Qt.Key_Left, Qt.NoModifier);204 keyClick(Qt.Key_Left, Qt.NoModifier);
204 tryCompareFunction(function() { return menuItem.popup !== null && menuItem.popup.visible }, false);205 tryCompareFunction(function() { return menuItem.popup !== null && menuItem.popup.visible }, false);
205 }206 }
207
208 function test_differentSizes() {
209 menuBackend.modelData = appMenuData.differentSizesMenu;
210
211 waitForRendering(page);
212 var longWidth = page.width;
213
214 var menuData = appMenuData.differentSizesMenu;
215 menuData.pop();
216 menuBackend.modelData = menuData;
217
218 waitForRendering(page);
219 verify(page.width < longWidth);
220 }
206 }221 }
207}222}

Subscribers

People subscribed via source and target branches