Merge lp:~nik90/checkbox/clean-qml-code into lp:~zyga/checkbox/touch-app

Proposed by Nekhelesh Ramananthan
Status: Merged
Merge reported by: Zygmunt Krynicki
Merged at revision: not available
Proposed branch: lp:~nik90/checkbox/clean-qml-code
Merge into: lp:~zyga/checkbox/touch-app
Diff against target: 111 lines (+33/-32)
3 files modified
checkbox-touch/components/WelcomePage.qml (+28/-27)
checkbox-touch/main.qml (+4/-4)
checkbox-touch/manifest.json (+1/-1)
To merge this branch: bzr merge lp:~nik90/checkbox/clean-qml-code
Reviewer Review Type Date Requested Status
Zygmunt Krynicki Pending
Review via email: mp+230991@code.launchpad.net

Commit message

Made some small improvements to the QML Code.

Description of the change

Made some small improvements to the QML Code.

To post a comment you must log in.
Revision history for this message
Zygmunt Krynicki (zyga) wrote :

Hi

I've merged your changes in locally. Due to the way we work I cannot just bzr merge it directly as I want to preserve the logical flow of patches in the target branch.

Thank you for your time and for improving checkbox :-)

I have only one question:

92 - WelcomePage {
93 - id: welcomePage
94 - visible: false
95 + Component.onCompleted: {
96 + push(Qt.resolvedUrl("components/WelcomePage.qml"))
97 }
98 - Component.onCompleted: push(welcomePage)

Why is that better? I realize it's going to push a new component each time, right? (and only once we actually click on the start testing button)

Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

I do that personally for several reasons,

1. It makes your main.qml file simple and precise.

2. Dynamic loading of components. Since welcomePage is in a separate file, it gets loaded only when you push it into the pagestack. Once you pop it (when moving to the next page), it gets unloaded from memory automatically.

This doesn't happen if you instantiate the welcomePage object in the main.qml file like it was done before.

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

Thanks.

I'll mark this merge request as merged now :-)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'checkbox-touch/components/WelcomePage.qml'
--- checkbox-touch/components/WelcomePage.qml 2014-08-15 14:30:38 +0000
+++ checkbox-touch/components/WelcomePage.qml 2014-08-15 15:45:49 +0000
@@ -23,42 +23,43 @@
23import QtQuick 2.023import QtQuick 2.0
24import Ubuntu.Components 1.124import Ubuntu.Components 1.1
2525
26
27Page {26Page {
27 id: welcomePage
28
28 signal startTestingTriggered();29 signal startTestingTriggered();
2930
30 title: i18n.tr("System Testing")31 title: i18n.tr("System Testing")
31 anchors.bottomMargin: units.gu(1)32 visible: false
32 Column {33
33 anchors.margins: units.gu(1)34 Label {
34 spacing: units.gu(1)35 id: welcomeText
35 id: column136
36 anchors {37 anchors {
37 fill: parent38 top: parent.top
38 }39 left: parent.left
39 Text {40 right: parent.right
40 id: welcomeText41 bottom: startTestButton.top
41 text: i18n.tr("Welcome text")42 }
42 wrapMode: Text.WrapAtWordBoundaryOrAnywhere43
43 width: parent.width44 text: i18n.tr("Welcome text")
44 height: parent.height45 font.pixelSize: units.gu(4)
45 verticalAlignment: Text.AlignVCenter46 verticalAlignment: Text.AlignVCenter
46 horizontalAlignment: Text.AlignHCenter47 horizontalAlignment: Text.AlignHCenter
47 font.pixelSize: units.gu(4)48 wrapMode: Text.WrapAtWordBoundaryOrAnywhere
48 }
49 }49 }
50
50 Button {51 Button {
52 id: startTestButton
53
54 anchors {
55 left: parent.left
56 right: parent.right
57 bottom: parent.bottom
58 margins: units.gu(2)
59 }
60
51 color: UbuntuColors.green61 color: UbuntuColors.green
52 text: i18n.tr("Start Testing")62 text: i18n.tr("Start Testing")
53 /* This is just to add some spacing around the button. I'm not sure if
54 * that's *the* way to do it though */
55 anchors.right: parent.right
56 anchors.rightMargin: units.gu(2)
57 anchors.left: parent.left
58 anchors.leftMargin: units.gu(2)
59 anchors.bottom: parent.bottom
60 anchors.bottomMargin: units.gu(1)
61 transformOrigin: Item.Center
62 onClicked: startTestingTriggered();63 onClicked: startTestingTriggered();
63 }64 }
64}65}
6566
=== modified file 'checkbox-touch/main.qml'
--- checkbox-touch/main.qml 2014-08-15 14:30:38 +0000
+++ checkbox-touch/main.qml 2014-08-15 15:45:49 +0000
@@ -44,6 +44,8 @@
44 width: units.gu(100)44 width: units.gu(100)
45 height: units.gu(75)45 height: units.gu(75)
4646
47 useDeprecatedToolbar: false
48
47 // High-level object representing the full checkbox testing stack49 // High-level object representing the full checkbox testing stack
48 CheckboxStack {50 CheckboxStack {
49 onStackReady: {51 onStackReady: {
@@ -55,10 +57,8 @@
5557
56 PageStack {58 PageStack {
57 id: pageStack59 id: pageStack
58 WelcomePage {60 Component.onCompleted: {
59 id: welcomePage61 push(Qt.resolvedUrl("components/WelcomePage.qml"))
60 visible: false
61 }62 }
62 Component.onCompleted: push(welcomePage)
63 }63 }
64}64}
6565
=== modified file 'checkbox-touch/manifest.json'
--- checkbox-touch/manifest.json 2014-08-12 19:01:04 +0000
+++ checkbox-touch/manifest.json 2014-08-15 15:45:49 +0000
@@ -12,4 +12,4 @@
12 "name": "com.canonical.certification.checkbox-touch",12 "name": "com.canonical.certification.checkbox-touch",
13 "title": "Checkbox Touch",13 "title": "Checkbox Touch",
14 "version": "0.2"14 "version": "0.2"
15}15}
16\ No newline at end of file16\ No newline at end of file

Subscribers

People subscribed via source and target branches