Merge lp:~cimi/ubuntu-system-settings/wizard.wifi-fixes into lp:ubuntu-system-settings

[Andrea Cimitan]

Various fixes for wifi wizard page

 * Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)
y
 * Did you build your software in a clean sbuild/pbuilder chroot or ppa?
n
 * Did you build your software in a clean sbuild/pbuilder armhf chroot or ppa?
n
 * Has your component "TestPlan” been executed successfully on emulator, N4?
n
 * Has a 5 minute exploratory testing run been executed on N4?
y
 * If you changed the packaging (debian), did you subscribe a core-dev to this MP?
n
 * If you changed the UI, did you subscribe the design-reviewers to this MP?
n
 * What components might get impacted by your changes?
wizard
 * Have you requested review by the teams of these owning components?
yes

[lp:~cimi/ubuntu-system-settings/wizard.wifi-fixes]

974. By Andrea Cimitan on 2014-09-02

Locale update

[Michael Terry (mterry)]

Some comments inline.

Also, I noticed that if I flick to the bottom of the wifi list, it refreshes and loses my place. Which makes it hard to select an entry near the bottom (you have to slowly scroll down and be careful not to pull list above end).

Seems to fix the bugs though, and looks more like the design mockups, thanks.

[Michael Terry (mterry)]

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:973
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~cimi/ubuntu-system-settings/wizard.wifi-fixes/+merge/233071/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1361/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4336
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3249
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/553
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4135
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5588
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5588/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12494
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2666
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3542
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3542/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1361/rebuild

review: Needs Fixing (continuous-integration)

[lp:~cimi/ubuntu-system-settings/wizard.wifi-fixes]

975. By Andrea Cimitan on 2014-09-02

using repeater

[Andrea Cimitan (cimi)]

Comments inline

[lp:~cimi/ubuntu-system-settings/wizard.wifi-fixes]

976. By Andrea Cimitan on 2014-09-02

Use null component

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:974
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1363/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4346
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3254
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/555
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4141
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5598
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5598/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12500
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2670
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3547
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3547/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1363/rebuild

review: Approve (continuous-integration)

[Michael Terry (mterry)]

It's really hard to have a conversation inline when LP clears the slate every commit. Feels weird to go back to an old commit just to reply. So I'll just reply here.

About anchors: I've certainly seen the block format before, but I've never liked it. It wastes two whole lines for no benefit. I won't block this MP on that change, but I'm not a fan, and it's certainly not related to your other code changes.

About the list: I feel like it flashes a little bit when loading. And changes around a lot when you select an entry. Is that something we can do anything about on our end?

Also, you can't deselect an entry. Not necessarily something a user will want to do often, but since there is a checkbox on the right, it feels weird that they don't act like checkboxes. The design has just a check, but not a checkbox. Could we make it look like that?

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:976
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1364/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4352
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3256
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/556
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4145
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5604
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5604/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12506
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2673
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3550
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3550/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1364/rebuild

review: Approve (continuous-integration)

[lp:~cimi/ubuntu-system-settings/wizard.wifi-fixes]

977. By Andrea Cimitan on 2014-09-03

Review comments

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:977
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1367/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4381
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3282
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/559
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4169
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5633
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5633/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12551
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2694
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3576
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3576/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1367/rebuild

review: Needs Fixing (continuous-integration)

[Michael Terry (mterry)]

You didn't answer my deselect question.

And the list is still jumpy. When I select an entry, the design visuals do not involve that entry jumping to the top. If you are scrolled down in a big list, it makes it look like the entry disappears after clicking it.

[Andrea Cimitan (cimi)]

> You didn't answer my deselect question.
>
> And the list is still jumpy. When I select an entry, the design visuals do
> not involve that entry jumping to the top. If you are scrolled down in a big
> list, it makes it look like the entry disappears after clicking it.
Both those issues cannot be implemented now, as they are related to the backend. We have in fact the same issues on unity and system settings

[Michael Terry (mterry)]

OK. You filed bug 1349371 about the jumping bit, thanks!

review: Approve

[Michael Terry (mterry)]

 * Are any changes against your component pending/needed to land the MP under review in a functional state and are those called out explicitly by the submitter?
 NA

 * Did you do exploratory testing related to the component you own with the MP changeset included?
 Yes

 * Has the submitter requested review by all the relevant teams/reviewers?
 Yes

 * If you are the reviewer owning the component the MP is against, have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?
 Yes

=== modified file 'po/it.po'
--- po/it.po	2014-09-03 07:03:52 +0000
+++ po/it.po	2014-09-03 09:51:49 +0000
@@ -1748,8 +1748,8 @@
 msgstr "Sistema"
 
 #: ../wizard/qml/Pages//20-wifi.qml:27
-msgid "Connect to Wi-Fi"
-msgstr "Connetti alla rete Wi-Fi"
+msgid "Connect to Wi‑Fi"
+msgstr "Connetti alla rete Wi‑Fi"
 
 #: ../wizard/qml/Pages//20-wifi.qml:66
 msgid "Available networks"

=== modified file 'wizard/qml/Components/Page.qml'
--- wizard/qml/Components/Page.qml	2014-08-26 21:19:55 +0000
+++ wizard/qml/Components/Page.qml	2014-09-03 09:51:49 +0000
@@ -25,7 +25,7 @@
     readonly property real topMargin: units.gu(8)
     readonly property real leftMargin: units.gu(2)
     readonly property real rightMargin: units.gu(2)
-    readonly property real bottomMargin: backButton.height + buttonMargin * 3
+    readonly property real bottomMargin: backButton.height + buttonMargin * 2
 
     property bool hasBackButton: true
     property bool customBack: false

=== modified file 'wizard/qml/Pages/20-wifi.qml'
--- wizard/qml/Pages/20-wifi.qml	2014-08-26 21:19:55 +0000
+++ wizard/qml/Pages/20-wifi.qml	2014-09-03 09:51:49 +0000
@@ -17,14 +17,14 @@
 import QtQuick 2.0
 import QMenuModel 0.1 as QMenuModel
 import Ubuntu.Components 0.1
-import Ubuntu.SystemSettings.Wifi 1.0
+import Ubuntu.Components.ListItems 0.1 as ListItem
 import Ubuntu.SystemSettings.Wizard.Utils 0.1
 import Ubuntu.Settings.Menus 0.1 as Menus
 import "../Components" as LocalComponents
 
 LocalComponents.Page {
     id: wifiPage
-    title: i18n.tr("Connect to Wi-Fi")
+    title: i18n.tr("Connect to Wi‑Fi")
     forwardButtonSourceComponent: forwardButton
 
     readonly property bool connected: mainMenu.connectedAPs === 1
@@ -36,15 +36,95 @@
         return defaultValue;
     }
 
-    SortFilterProxyModel {
+    QMenuModel.UnityMenuModel {
         id: menuModel
-        filterRole: 5 // UnityMenuModel.TypeRole should really be exported from unitymenumodel
-        filterRegExp: RegExp("unity.widgets.systemsettings.tablet.accesspoint")
-        model: QMenuModel.UnityMenuModel {
-            id: unitymenumodel
-            busName: "com.canonical.indicator.network"
-            actions: { "indicator": "/com/canonical/indicator/network" }
-            menuObjectPath: "/com/canonical/indicator/network/phone_wifi_settings"
+        busName: "com.canonical.indicator.network"
+        actions: { "indicator": "/com/canonical/indicator/network" }
+        menuObjectPath: "/com/canonical/indicator/network/phone_wifi_settings"
+    }
+
+    Component {
+        id: accessPointComponent
+        ListItem.Standard {
+            id: accessPoint
+            objectName: "accessPoint"
+
+            property QtObject menuData: null
+            property var unityMenuModel: menuModel
+            property var extendedData: menuData && menuData.ext || undefined
+            property var strengthAction: QMenuModel.UnityMenuAction {
+                model: unityMenuModel
+                index: menuIndex
+                name: getExtendedProperty(extendedData, "xCanonicalWifiApStrengthAction", "")
+            }
+            property bool checked: menuData && menuData.isToggled || false
+            property bool secure: getExtendedProperty(extendedData, "xCanonicalWifiApIsSecure", false)
+            property bool adHoc: getExtendedProperty(extendedData, "xCanonicalWifiApIsAdhoc", false)
+            property int signalStrength: strengthAction.valid ? strengthAction.state : 0
+            property int menuIndex: -1
+
+            function loadAttributes() {
+                if (!unityMenuModel || menuIndex == -1) return;
+                unityMenuModel.loadExtendedAttributes(menuIndex, {'x-canonical-wifi-ap-is-adhoc': 'bool',
+                                                                  'x-canonical-wifi-ap-is-secure': 'bool',
+                                                                  'x-canonical-wifi-ap-strength-action': 'string'});
+            }
+
+            signal activate()
+
+            text: menuData && menuData.label || ""
+            enabled: menuData && menuData.sensitive || false
+            iconName: {
+                var imageName = "nm-signal-100";
+
+                if (adHoc) {
+                    imageName = "nm-adhoc";
+                } else if (signalStrength == 0) {
+                    imageName = "nm-signal-00";
+                } else if (signalStrength <= 25) {
+                    imageName = "nm-signal-25";
+                } else if (signalStrength <= 50) {
+                    imageName = "nm-signal-50";
+                } else if (signalStrength <= 75) {
+                    imageName = "nm-signal-75";
+                }
+
+                if (secure) {
+                    imageName += "-secure";
+                }
+                return imageName;
+            }
+            iconFrame: false
+            control: CheckBox {
+                id: checkBoxActive
+
+                onClicked: {
+                    accessPoint.activate();
+                }
+            }
+            style: Rectangle {
+                color: "#4c000000"
+            }
+
+            Component.onCompleted: {
+                loadAttributes();
+            }
+            onUnityMenuModelChanged: {
+                loadAttributes();
+            }
+            onMenuIndexChanged: {
+                loadAttributes();
+            }
+            onCheckedChanged: {
+                // Can't rely on binding. Checked is assigned on click.
+                checkBoxActive.checked = checked;
+                if (checked) {
+                    mainMenu.connectedAPs++
+                } else {
+                    mainMenu.connectedAPs--
+                }
+            }
+            onActivate: unityMenuModel.activate(menuIndex);
         }
     }
 
@@ -66,76 +146,45 @@
             text: i18n.tr("Available networks")
         }
 
-        ListView {
-            id: mainMenu
-
-            property int connectedAPs: 0
-
-            model: menuModel
+        Flickable {
+            anchors.left: parent.left
+            anchors.right: parent.right
             height: column.height - label.height - column.spacing
-            anchors.left: parent.left
-            anchors.right: parent.right
+            contentHeight: contentItem.childrenRect.height
             clip: true
-
-            // Ensure all delegates are cached in order to improve smoothness of scrolling
-            cacheBuffer: 10000
-
-            // Only allow flicking if the content doesn't fit on the page
-            boundsBehavior: (contentHeight > mainMenu.height) ?
-                                Flickable.DragAndOvershootBounds :
-                                Flickable.StopAtBounds
-
-            currentIndex: -1
-            delegate: Menus.AccessPointMenu {
-                id: menuDelegate
-
-                property int menuIndex: menuModel.mapRowToSource(index)
-                property var extendedData: model.ext
-                property var serverToggle: model.isToggled
-                property var strengthAction: QMenuModel.UnityMenuAction {
-                    model: unitymenumodel
-                    index: menuIndex
-                    name: getExtendedProperty(extendedData, "xCanonicalWifiApStrengthAction", "")
-                }
-
-                anchors {
-                    left: parent.left
-                    right: parent.right
-                }
-                text: model.label
-                enabled: model.sensitive
-                secure: getExtendedProperty(extendedData, "xCanonicalWifiApIsSecure", false)
-                adHoc: getExtendedProperty(extendedData, "xCanonicalWifiApIsAdhoc", false)
-                signalStrength: strengthAction.valid ? strengthAction.state : 0
-
-                style: Rectangle {
-                    color: "black"
-                    opacity: 0.3
-                }
-
-                onCheckedChanged: {
-                    if (checked) {
-                        mainMenu.connectedAPs++
-                    } else {
-                        mainMenu.connectedAPs--
+            flickDeceleration: 1500 * units.gridUnit / 8
+            maximumFlickVelocity: 2500 * units.gridUnit / 8
+            boundsBehavior: (contentHeight > height) ? Flickable.DragAndOvershootBounds : Flickable.StopAtBounds
+
+            Column {
+                anchors.left: parent.left
+                anchors.right: parent.right
+
+                Repeater {
+                    id: mainMenu
+
+                    // FIXME Check connection better https://bugs.launchpad.net/indicator-network/+bug/1349371
+                    property int connectedAPs: 0
+
+                    model: menuModel
+
+                    delegate: Loader {
+                        id: loader
+
+                        readonly property bool isAccessPoint: model.type === "unity.widgets.systemsettings.tablet.accesspoint"
+
+                        anchors.left: parent.left
+                        anchors.right: parent.right
+                        height: isAccessPoint ? units.gu(6) : 0
+                        asynchronous: true
+                        sourceComponent: isAccessPoint ? accessPointComponent : null
+
+                        onLoaded: {
+                            item.menuData = Qt.binding(function() { return model; });
+                            item.menuIndex = Qt.binding(function() { return index; });
+                        }
                     }
                 }
-                onMenuIndexChanged: {
-                    loadAttributes();
-                }
-                onServerToggleChanged: {
-                    checked = serverToggle;
-                }
-                onTriggered: {
-                    unitymenumodel.activate(menuIndex);
-                }
-
-                function loadAttributes() {
-                    if (!unitymenumodel || menuIndex == -1) return;
-                    unitymenumodel.loadExtendedAttributes(menuIndex, {'x-canonical-wifi-ap-is-adhoc': 'bool',
-                                                                      'x-canonical-wifi-ap-is-secure': 'bool',
-                                                                      'x-canonical-wifi-ap-strength-action': 'string'});
-                }
             }
         }
     }

=== modified file 'wizard/qml/main.qml'
--- wizard/qml/main.qml	2014-08-26 21:19:55 +0000
+++ wizard/qml/main.qml	2014-09-03 09:51:49 +0000
@@ -108,6 +108,17 @@
         }
     }
 
+    Rectangle {
+        id: modalNotificationBackground
+        visible: notifications.useModal && (notifications.state == "narrow")
+        anchors.fill: parent
+        color: "#80000000"
+
+        MouseArea {
+            anchors.fill: parent
+        }
+    }
+
     InputMethod {
         anchors.fill: parent
     }