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
1=== modified file 'qml/ApplicationMenus/MenuBar.qml'
2--- qml/ApplicationMenus/MenuBar.qml 2017-02-16 13:43:18 +0000
3+++ qml/ApplicationMenus/MenuBar.qml 2017-02-23 13:20:34 +0000
4@@ -96,6 +96,11 @@
5 onModelReset: d.firstInvisibleIndex = undefined
6 }
7
8+ Component {
9+ id: menuComponent
10+ MenuPopup { }
11+ }
12+
13 Repeater {
14 id: rowRepeater
15
16@@ -119,7 +124,14 @@
17
18 function show() {
19 if (!__popup) {
20- __popup = menuComponent.createObject(root, { objectName: visualItem.objectName + "-menu" });
21+ __popup = menuComponent.createObject(root,
22+ {
23+ objectName: visualItem.objectName + "-menu",
24+ desiredX: Qt.binding(function() { return visualItem.x - units.gu(1); }),
25+ desiredY: Qt.binding(function() { return root.height; }),
26+ unityMenuModel: Qt.binding(function() { return root.unityMenuModel.submenu(visualItem.__ownIndex); })
27+ });
28+ __popup.reset();
29 __popup.childActivated.connect(dismiss);
30 // force the current item to be the newly popped up menu
31 } else {
32@@ -162,17 +174,6 @@
33 onDismissAll: visualItem.dismiss()
34 }
35
36- Component {
37- id: menuComponent
38- MenuPopup {
39- desiredX: visualItem.x - units.gu(1)
40- desiredY: parent.height
41- unityMenuModel: root.unityMenuModel.submenu(visualItem.__ownIndex)
42-
43- Component.onCompleted: reset();
44- }
45- }
46-
47 RowLayout {
48 id: column
49 spacing: units.gu(1)
50
51=== modified file 'qml/ApplicationMenus/MenuItem.qml'
52--- qml/ApplicationMenus/MenuItem.qml 2017-02-07 14:10:41 +0000
53+++ qml/ApplicationMenus/MenuItem.qml 2017-02-23 13:20:34 +0000
54@@ -34,7 +34,7 @@
55 val += units.gu(1) + title.contentWidth;
56 if (hasSubmenu) {
57 val += units.gu(1) + chevronIcon.width;
58- } else if (shortcut != undefined) {
59+ } else if (menuData && menuData.shortcut != undefined) {
60 val += units.gu(3) + shortcutLabel.contentWidth;
61 }
62 return val + units.gu(1);
63@@ -49,9 +49,9 @@
64 enabled: root.enabled
65
66 // FIXME - SDK Action:text modifies menu text with html underline for mnemonic
67- text: menuData.label.replace("_", "&").replace("<u>", "&").replace("</u>", "")
68- checkable: menuData.isCheck || menuData.isRadio
69- checked: menuData.isToggled
70+ text: menuData ? menuData.label.replace("_", "&").replace("<u>", "&").replace("</u>", "") : ""
71+ checkable: menuData && (menuData.isCheck || menuData.isRadio)
72+ checked: menuData && menuData.isToggled
73 }
74
75 width: {
76@@ -127,8 +127,8 @@
77 color: enabled ? theme.palette.normal.overlaySecondaryText :
78 theme.palette.disabled.overlaySecondaryText
79
80- visible: menuData.shortcut != undefined && !root.hasSubmenu && QuickUtils.keyboardAttached
81- text: menuData.shortcut ? menuData.shortcut : ""
82+ visible: menuData && menuData.shortcut != undefined && !root.hasSubmenu && QuickUtils.keyboardAttached
83+ text: menuData && menuData.shortcut ? menuData.shortcut : ""
84 }
85 }
86
87
88=== modified file 'qml/ApplicationMenus/MenuPopup.qml'
89--- qml/ApplicationMenus/MenuPopup.qml 2017-01-31 15:24:06 +0000
90+++ qml/ApplicationMenus/MenuPopup.qml 2017-02-23 13:20:34 +0000
91@@ -276,6 +276,79 @@
92 }
93 }
94
95+ Component {
96+ id: separatorComponent
97+ ListItems.ThinDivider {
98+ // Parent will be loader
99+ objectName: parent.objectName + "-separator"
100+ implicitHeight: units.dp(2)
101+ }
102+ }
103+
104+ Component {
105+ id: menuItemComponent
106+ MenuItem {
107+ // Parent will be loader
108+ id: menuItem
109+ menuData: parent.__menuData
110+ objectName: parent.objectName + "-actionItem"
111+
112+ width: MathUtils.clamp(implicitWidth, d.__minimumWidth, d.__maximumWidth)
113+
114+ action.onTriggered: {
115+ submenuHoverTimer.stop();
116+
117+ d.currentItem = parent;
118+
119+ if (hasSubmenu) {
120+ if (!popup) {
121+ var model = root.unityMenuModel.submenu(__ownIndex);
122+ popup = submenuComponent.createObject(focusScope, {
123+ objectName: parent.objectName + "-",
124+ unityMenuModel: model,
125+ substractWidth: true,
126+ desiredX: Qt.binding(function() { return root.width }),
127+ desiredY: Qt.binding(function() {
128+ var dummy = listView.contentY; // force a recalc on contentY change.
129+ return mapToItem(container, 0, y).y;
130+ })
131+ });
132+ popup.retreat.connect(function() {
133+ popup.destroy();
134+ popup = null;
135+ menuItem.forceActiveFocus();
136+ });
137+ popup.childActivated.connect(function() {
138+ popup.destroy();
139+ popup = null;
140+ root.childActivated();
141+ });
142+ } else if (popup) {
143+ popup.visible = true;
144+ }
145+ } else {
146+ root.unityMenuModel.activate(__ownIndex);
147+ root.childActivated();
148+ }
149+ }
150+
151+ Connections {
152+ target: d
153+ onCurrentIndexChanged: {
154+ if (popup && d.currentIndex != __ownIndex) {
155+ popup.visible = false;
156+ }
157+ }
158+ onDismissAll: {
159+ if (popup) {
160+ popup.destroy();
161+ popup = null;
162+ }
163+ }
164+ }
165+ }
166+ }
167+
168 ColumnLayout {
169 id: menuColumn
170 spacing: 0
171@@ -289,6 +362,8 @@
172 id: loader
173 objectName: root.objectName + "-item" + __ownIndex
174
175+ property Item popup: null
176+ property var __menuData: model
177 property int __ownIndex: index
178 property bool __isSeparator: model.isSeparator
179
180@@ -301,80 +376,7 @@
181 return menuItemComponent;
182 }
183
184- property Item popup: null
185-
186 Layout.fillWidth: true
187-
188- Component {
189- id: menuItemComponent
190- MenuItem {
191- id: menuItem
192- menuData: model
193- objectName: loader.objectName + "-actionItem"
194-
195- width: MathUtils.clamp(implicitWidth, d.__minimumWidth, d.__maximumWidth)
196-
197- action.onTriggered: {
198- submenuHoverTimer.stop();
199-
200- d.currentItem = loader;
201-
202- if (hasSubmenu) {
203- if (!popup) {
204- var model = root.unityMenuModel.submenu(__ownIndex);
205- popup = submenuComponent.createObject(focusScope, {
206- objectName: loader.objectName + "-",
207- unityMenuModel: model,
208- substractWidth: true,
209- desiredX: Qt.binding(function() { return root.width }),
210- desiredY: Qt.binding(function() {
211- var dummy = listView.contentY; // force a recalc on contentY change.
212- return mapToItem(container, 0, y).y;
213- })
214- });
215- popup.retreat.connect(function() {
216- popup.destroy();
217- popup = null;
218- menuItem.forceActiveFocus();
219- });
220- popup.childActivated.connect(function() {
221- popup.destroy();
222- popup = null;
223- root.childActivated();
224- });
225- } else if (popup) {
226- popup.visible = true;
227- }
228- } else {
229- root.unityMenuModel.activate(__ownIndex);
230- root.childActivated();
231- }
232- }
233-
234- Connections {
235- target: d
236- onCurrentIndexChanged: {
237- if (popup && d.currentIndex != __ownIndex) {
238- popup.visible = false;
239- }
240- }
241- onDismissAll: {
242- if (popup) {
243- popup.destroy();
244- popup = null;
245- }
246- }
247- }
248- }
249- }
250-
251- Component {
252- id: separatorComponent
253- ListItems.ThinDivider {
254- objectName: loader.objectName + "-separator"
255- implicitHeight: units.dp(2)
256- }
257- }
258 }
259
260 }
261
262=== modified file 'tests/qmltests/ApplicationMenus/tst_MenuBar.qml'
263--- tests/qmltests/ApplicationMenus/tst_MenuBar.qml 2017-02-07 14:10:41 +0000
264+++ tests/qmltests/ApplicationMenus/tst_MenuBar.qml 2017-02-23 13:20:34 +0000
265@@ -85,6 +85,7 @@
266 menuBar.dismiss();
267 menuBackend.modelData = appMenuData.generateTestData(5,5,2,3, "menu")
268 activatedSpy.clear();
269+ waitForRendering(menuBar);
270 }
271
272 function test_mouseNavigation() {

Subscribers

People subscribed via source and target branches