Unity 8

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

  1. create_less_components
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Michael Zanetti
Approved revision: 2838
Merged at revision: 2862
Proposed branch: lp:~aacid/unity8/create_less_components
Merge into: lp:unity8
Diff against target: 272 lines (+95/-91)
4 files modified
qml/ApplicationMenus/MenuBar.qml (+13/-12)
qml/ApplicationMenus/MenuItem.qml (+6/-6)
qml/ApplicationMenus/MenuPopup.qml (+75/-73)
tests/qmltests/ApplicationMenus/tst_MenuBar.qml (+1/-0)
To merge this branch: bzr merge lp:~aacid/unity8/create_less_components
Reviewer Review Type Date Requested Status
Michael Zanetti (community) Approve
Unity8 CI Bot continuous-integration Approve
Review via email: mp+317953@code.launchpad.net

Commit message

Move menus Component {} out of the repeater

They are not very expensive to create but since it is easy to not create that many, better move them out of the repeater

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

To post a comment you must log in.
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Tested this, works still the same as before. Code changes look good.

waiting for CI for top approval

review: Approve
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:2835
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3173/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4164
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2449
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2449
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4192
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4029
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4029/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4029
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4029/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4029
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4029/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4029
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4029/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4029
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4029/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4029
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4029/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

looks like it fails some tests

review: Needs Fixing
lp:~aacid/unity8/create_less_components updated
2836. By Albert Astals Cid

New code is faster and test needs to actually wait for the menubar to be in the initial state

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:2836
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3175/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4168
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2453
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2453
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4196
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4033
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4033/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4033
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4033/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4033
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4033/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4033
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4033/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4033
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4033/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4033
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4033/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/create_less_components updated
2837. By Albert Astals Cid

less warnings about menuData being null

2838. By Albert Astals Cid

Make these bindings as they were before

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:2838
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3191/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4188
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2470
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2470
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4216
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4052
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4052/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4052
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4052/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4052
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4052/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4052
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4052/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4052
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4052/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4052
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4052/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

CI is happy now. Latest code changes look good and are still working fine.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'qml/ApplicationMenus/MenuBar.qml'
--- qml/ApplicationMenus/MenuBar.qml 2017-02-16 13:43:18 +0000
+++ qml/ApplicationMenus/MenuBar.qml 2017-02-23 13:20:34 +0000
@@ -96,6 +96,11 @@
96 onModelReset: d.firstInvisibleIndex = undefined96 onModelReset: d.firstInvisibleIndex = undefined
97 }97 }
9898
99 Component {
100 id: menuComponent
101 MenuPopup { }
102 }
103
99 Repeater {104 Repeater {
100 id: rowRepeater105 id: rowRepeater
101106
@@ -119,7 +124,14 @@
119124
120 function show() {125 function show() {
121 if (!__popup) {126 if (!__popup) {
122 __popup = menuComponent.createObject(root, { objectName: visualItem.objectName + "-menu" });127 __popup = menuComponent.createObject(root,
128 {
129 objectName: visualItem.objectName + "-menu",
130 desiredX: Qt.binding(function() { return visualItem.x - units.gu(1); }),
131 desiredY: Qt.binding(function() { return root.height; }),
132 unityMenuModel: Qt.binding(function() { return root.unityMenuModel.submenu(visualItem.__ownIndex); })
133 });
134 __popup.reset();
123 __popup.childActivated.connect(dismiss);135 __popup.childActivated.connect(dismiss);
124 // force the current item to be the newly popped up menu136 // force the current item to be the newly popped up menu
125 } else {137 } else {
@@ -162,17 +174,6 @@
162 onDismissAll: visualItem.dismiss()174 onDismissAll: visualItem.dismiss()
163 }175 }
164176
165 Component {
166 id: menuComponent
167 MenuPopup {
168 desiredX: visualItem.x - units.gu(1)
169 desiredY: parent.height
170 unityMenuModel: root.unityMenuModel.submenu(visualItem.__ownIndex)
171
172 Component.onCompleted: reset();
173 }
174 }
175
176 RowLayout {177 RowLayout {
177 id: column178 id: column
178 spacing: units.gu(1)179 spacing: units.gu(1)
179180
=== modified file 'qml/ApplicationMenus/MenuItem.qml'
--- qml/ApplicationMenus/MenuItem.qml 2017-02-07 14:10:41 +0000
+++ qml/ApplicationMenus/MenuItem.qml 2017-02-23 13:20:34 +0000
@@ -34,7 +34,7 @@
34 val += units.gu(1) + title.contentWidth;34 val += units.gu(1) + title.contentWidth;
35 if (hasSubmenu) {35 if (hasSubmenu) {
36 val += units.gu(1) + chevronIcon.width;36 val += units.gu(1) + chevronIcon.width;
37 } else if (shortcut != undefined) {37 } else if (menuData && menuData.shortcut != undefined) {
38 val += units.gu(3) + shortcutLabel.contentWidth;38 val += units.gu(3) + shortcutLabel.contentWidth;
39 }39 }
40 return val + units.gu(1);40 return val + units.gu(1);
@@ -49,9 +49,9 @@
49 enabled: root.enabled49 enabled: root.enabled
5050
51 // FIXME - SDK Action:text modifies menu text with html underline for mnemonic51 // FIXME - SDK Action:text modifies menu text with html underline for mnemonic
52 text: menuData.label.replace("_", "&").replace("<u>", "&").replace("</u>", "")52 text: menuData ? menuData.label.replace("_", "&").replace("<u>", "&").replace("</u>", "") : ""
53 checkable: menuData.isCheck || menuData.isRadio53 checkable: menuData && (menuData.isCheck || menuData.isRadio)
54 checked: menuData.isToggled54 checked: menuData && menuData.isToggled
55 }55 }
5656
57 width: {57 width: {
@@ -127,8 +127,8 @@
127 color: enabled ? theme.palette.normal.overlaySecondaryText :127 color: enabled ? theme.palette.normal.overlaySecondaryText :
128 theme.palette.disabled.overlaySecondaryText128 theme.palette.disabled.overlaySecondaryText
129129
130 visible: menuData.shortcut != undefined && !root.hasSubmenu && QuickUtils.keyboardAttached130 visible: menuData && menuData.shortcut != undefined && !root.hasSubmenu && QuickUtils.keyboardAttached
131 text: menuData.shortcut ? menuData.shortcut : ""131 text: menuData && menuData.shortcut ? menuData.shortcut : ""
132 }132 }
133 }133 }
134134
135135
=== modified file 'qml/ApplicationMenus/MenuPopup.qml'
--- qml/ApplicationMenus/MenuPopup.qml 2017-01-31 15:24:06 +0000
+++ qml/ApplicationMenus/MenuPopup.qml 2017-02-23 13:20:34 +0000
@@ -276,6 +276,79 @@
276 }276 }
277 }277 }
278278
279 Component {
280 id: separatorComponent
281 ListItems.ThinDivider {
282 // Parent will be loader
283 objectName: parent.objectName + "-separator"
284 implicitHeight: units.dp(2)
285 }
286 }
287
288 Component {
289 id: menuItemComponent
290 MenuItem {
291 // Parent will be loader
292 id: menuItem
293 menuData: parent.__menuData
294 objectName: parent.objectName + "-actionItem"
295
296 width: MathUtils.clamp(implicitWidth, d.__minimumWidth, d.__maximumWidth)
297
298 action.onTriggered: {
299 submenuHoverTimer.stop();
300
301 d.currentItem = parent;
302
303 if (hasSubmenu) {
304 if (!popup) {
305 var model = root.unityMenuModel.submenu(__ownIndex);
306 popup = submenuComponent.createObject(focusScope, {
307 objectName: parent.objectName + "-",
308 unityMenuModel: model,
309 substractWidth: true,
310 desiredX: Qt.binding(function() { return root.width }),
311 desiredY: Qt.binding(function() {
312 var dummy = listView.contentY; // force a recalc on contentY change.
313 return mapToItem(container, 0, y).y;
314 })
315 });
316 popup.retreat.connect(function() {
317 popup.destroy();
318 popup = null;
319 menuItem.forceActiveFocus();
320 });
321 popup.childActivated.connect(function() {
322 popup.destroy();
323 popup = null;
324 root.childActivated();
325 });
326 } else if (popup) {
327 popup.visible = true;
328 }
329 } else {
330 root.unityMenuModel.activate(__ownIndex);
331 root.childActivated();
332 }
333 }
334
335 Connections {
336 target: d
337 onCurrentIndexChanged: {
338 if (popup && d.currentIndex != __ownIndex) {
339 popup.visible = false;
340 }
341 }
342 onDismissAll: {
343 if (popup) {
344 popup.destroy();
345 popup = null;
346 }
347 }
348 }
349 }
350 }
351
279 ColumnLayout {352 ColumnLayout {
280 id: menuColumn353 id: menuColumn
281 spacing: 0354 spacing: 0
@@ -289,6 +362,8 @@
289 id: loader362 id: loader
290 objectName: root.objectName + "-item" + __ownIndex363 objectName: root.objectName + "-item" + __ownIndex
291364
365 property Item popup: null
366 property var __menuData: model
292 property int __ownIndex: index367 property int __ownIndex: index
293 property bool __isSeparator: model.isSeparator368 property bool __isSeparator: model.isSeparator
294369
@@ -301,80 +376,7 @@
301 return menuItemComponent;376 return menuItemComponent;
302 }377 }
303378
304 property Item popup: null
305
306 Layout.fillWidth: true379 Layout.fillWidth: true
307
308 Component {
309 id: menuItemComponent
310 MenuItem {
311 id: menuItem
312 menuData: model
313 objectName: loader.objectName + "-actionItem"
314
315 width: MathUtils.clamp(implicitWidth, d.__minimumWidth, d.__maximumWidth)
316
317 action.onTriggered: {
318 submenuHoverTimer.stop();
319
320 d.currentItem = loader;
321
322 if (hasSubmenu) {
323 if (!popup) {
324 var model = root.unityMenuModel.submenu(__ownIndex);
325 popup = submenuComponent.createObject(focusScope, {
326 objectName: loader.objectName + "-",
327 unityMenuModel: model,
328 substractWidth: true,
329 desiredX: Qt.binding(function() { return root.width }),
330 desiredY: Qt.binding(function() {
331 var dummy = listView.contentY; // force a recalc on contentY change.
332 return mapToItem(container, 0, y).y;
333 })
334 });
335 popup.retreat.connect(function() {
336 popup.destroy();
337 popup = null;
338 menuItem.forceActiveFocus();
339 });
340 popup.childActivated.connect(function() {
341 popup.destroy();
342 popup = null;
343 root.childActivated();
344 });
345 } else if (popup) {
346 popup.visible = true;
347 }
348 } else {
349 root.unityMenuModel.activate(__ownIndex);
350 root.childActivated();
351 }
352 }
353
354 Connections {
355 target: d
356 onCurrentIndexChanged: {
357 if (popup && d.currentIndex != __ownIndex) {
358 popup.visible = false;
359 }
360 }
361 onDismissAll: {
362 if (popup) {
363 popup.destroy();
364 popup = null;
365 }
366 }
367 }
368 }
369 }
370
371 Component {
372 id: separatorComponent
373 ListItems.ThinDivider {
374 objectName: loader.objectName + "-separator"
375 implicitHeight: units.dp(2)
376 }
377 }
378 }380 }
379381
380 }382 }
381383
=== modified file 'tests/qmltests/ApplicationMenus/tst_MenuBar.qml'
--- tests/qmltests/ApplicationMenus/tst_MenuBar.qml 2017-02-07 14:10:41 +0000
+++ tests/qmltests/ApplicationMenus/tst_MenuBar.qml 2017-02-23 13:20:34 +0000
@@ -85,6 +85,7 @@
85 menuBar.dismiss();85 menuBar.dismiss();
86 menuBackend.modelData = appMenuData.generateTestData(5,5,2,3, "menu")86 menuBackend.modelData = appMenuData.generateTestData(5,5,2,3, "menu")
87 activatedSpy.clear();87 activatedSpy.clear();
88 waitForRendering(menuBar);
88 }89 }
8990
90 function test_mouseNavigation() {91 function test_mouseNavigation() {

Subscribers

People subscribed via source and target branches