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

Proposed by Andrea Cimitan
Status: Merged
Approved by: Michael Terry
Approved revision: 977
Merged at revision: 985
Proposed branch: lp:~cimi/ubuntu-system-settings/wizard.wifi-fixes
Merge into: lp:ubuntu-system-settings
Diff against target: 286 lines (+139/-79)
4 files modified
po/it.po (+2/-2)
wizard/qml/Components/Page.qml (+1/-1)
wizard/qml/Pages/20-wifi.qml (+125/-76)
wizard/qml/main.qml (+11/-0)
To merge this branch: bzr merge lp:~cimi/ubuntu-system-settings/wizard.wifi-fixes
Reviewer Review Type Date Requested Status
Michael Terry (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+233071@code.launchpad.net

Commit message

Various fixes for wifi wizard page

Description of the change

 * 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

To post a comment you must log in.
974. By Andrea Cimitan

Locale update

Revision history for this message
Michael Terry (mterry) wrote :

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.

Revision history for this message
Michael Terry (mterry) :
review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
975. By Andrea Cimitan

using repeater

Revision history for this message
Andrea Cimitan (cimi) wrote :

Comments inline

976. By Andrea Cimitan

Use null component

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michael Terry (mterry) wrote :

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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
977. By Andrea Cimitan

Review comments

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Terry (mterry) wrote :

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.

Revision history for this message
Andrea Cimitan (cimi) wrote :

> 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

Revision history for this message
Michael Terry (mterry) wrote :

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

review: Approve
Revision history for this message
Michael Terry (mterry) wrote :

 * 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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1748msgstr "Sistema"1748msgstr "Sistema"
17491749
1750#: ../wizard/qml/Pages//20-wifi.qml:271750#: ../wizard/qml/Pages//20-wifi.qml:27
1751msgid "Connect to Wi-Fi"1751msgid "Connect to Wi‑Fi"
1752msgstr "Connetti alla rete Wi-Fi"1752msgstr "Connetti alla rete Wi‑Fi"
17531753
1754#: ../wizard/qml/Pages//20-wifi.qml:661754#: ../wizard/qml/Pages//20-wifi.qml:66
1755msgid "Available networks"1755msgid "Available networks"
17561756
=== 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 @@
25 readonly property real topMargin: units.gu(8)25 readonly property real topMargin: units.gu(8)
26 readonly property real leftMargin: units.gu(2)26 readonly property real leftMargin: units.gu(2)
27 readonly property real rightMargin: units.gu(2)27 readonly property real rightMargin: units.gu(2)
28 readonly property real bottomMargin: backButton.height + buttonMargin * 328 readonly property real bottomMargin: backButton.height + buttonMargin * 2
2929
30 property bool hasBackButton: true30 property bool hasBackButton: true
31 property bool customBack: false31 property bool customBack: false
3232
=== 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 @@
17import QtQuick 2.017import QtQuick 2.0
18import QMenuModel 0.1 as QMenuModel18import QMenuModel 0.1 as QMenuModel
19import Ubuntu.Components 0.119import Ubuntu.Components 0.1
20import Ubuntu.SystemSettings.Wifi 1.020import Ubuntu.Components.ListItems 0.1 as ListItem
21import Ubuntu.SystemSettings.Wizard.Utils 0.121import Ubuntu.SystemSettings.Wizard.Utils 0.1
22import Ubuntu.Settings.Menus 0.1 as Menus22import Ubuntu.Settings.Menus 0.1 as Menus
23import "../Components" as LocalComponents23import "../Components" as LocalComponents
2424
25LocalComponents.Page {25LocalComponents.Page {
26 id: wifiPage26 id: wifiPage
27 title: i18n.tr("Connect to Wi-Fi")27 title: i18n.tr("Connect to Wi‑Fi")
28 forwardButtonSourceComponent: forwardButton28 forwardButtonSourceComponent: forwardButton
2929
30 readonly property bool connected: mainMenu.connectedAPs === 130 readonly property bool connected: mainMenu.connectedAPs === 1
@@ -36,15 +36,95 @@
36 return defaultValue;36 return defaultValue;
37 }37 }
3838
39 SortFilterProxyModel {39 QMenuModel.UnityMenuModel {
40 id: menuModel40 id: menuModel
41 filterRole: 5 // UnityMenuModel.TypeRole should really be exported from unitymenumodel41 busName: "com.canonical.indicator.network"
42 filterRegExp: RegExp("unity.widgets.systemsettings.tablet.accesspoint")42 actions: { "indicator": "/com/canonical/indicator/network" }
43 model: QMenuModel.UnityMenuModel {43 menuObjectPath: "/com/canonical/indicator/network/phone_wifi_settings"
44 id: unitymenumodel44 }
45 busName: "com.canonical.indicator.network"45
46 actions: { "indicator": "/com/canonical/indicator/network" }46 Component {
47 menuObjectPath: "/com/canonical/indicator/network/phone_wifi_settings"47 id: accessPointComponent
48 ListItem.Standard {
49 id: accessPoint
50 objectName: "accessPoint"
51
52 property QtObject menuData: null
53 property var unityMenuModel: menuModel
54 property var extendedData: menuData && menuData.ext || undefined
55 property var strengthAction: QMenuModel.UnityMenuAction {
56 model: unityMenuModel
57 index: menuIndex
58 name: getExtendedProperty(extendedData, "xCanonicalWifiApStrengthAction", "")
59 }
60 property bool checked: menuData && menuData.isToggled || false
61 property bool secure: getExtendedProperty(extendedData, "xCanonicalWifiApIsSecure", false)
62 property bool adHoc: getExtendedProperty(extendedData, "xCanonicalWifiApIsAdhoc", false)
63 property int signalStrength: strengthAction.valid ? strengthAction.state : 0
64 property int menuIndex: -1
65
66 function loadAttributes() {
67 if (!unityMenuModel || menuIndex == -1) return;
68 unityMenuModel.loadExtendedAttributes(menuIndex, {'x-canonical-wifi-ap-is-adhoc': 'bool',
69 'x-canonical-wifi-ap-is-secure': 'bool',
70 'x-canonical-wifi-ap-strength-action': 'string'});
71 }
72
73 signal activate()
74
75 text: menuData && menuData.label || ""
76 enabled: menuData && menuData.sensitive || false
77 iconName: {
78 var imageName = "nm-signal-100";
79
80 if (adHoc) {
81 imageName = "nm-adhoc";
82 } else if (signalStrength == 0) {
83 imageName = "nm-signal-00";
84 } else if (signalStrength <= 25) {
85 imageName = "nm-signal-25";
86 } else if (signalStrength <= 50) {
87 imageName = "nm-signal-50";
88 } else if (signalStrength <= 75) {
89 imageName = "nm-signal-75";
90 }
91
92 if (secure) {
93 imageName += "-secure";
94 }
95 return imageName;
96 }
97 iconFrame: false
98 control: CheckBox {
99 id: checkBoxActive
100
101 onClicked: {
102 accessPoint.activate();
103 }
104 }
105 style: Rectangle {
106 color: "#4c000000"
107 }
108
109 Component.onCompleted: {
110 loadAttributes();
111 }
112 onUnityMenuModelChanged: {
113 loadAttributes();
114 }
115 onMenuIndexChanged: {
116 loadAttributes();
117 }
118 onCheckedChanged: {
119 // Can't rely on binding. Checked is assigned on click.
120 checkBoxActive.checked = checked;
121 if (checked) {
122 mainMenu.connectedAPs++
123 } else {
124 mainMenu.connectedAPs--
125 }
126 }
127 onActivate: unityMenuModel.activate(menuIndex);
48 }128 }
49 }129 }
50130
@@ -66,76 +146,45 @@
66 text: i18n.tr("Available networks")146 text: i18n.tr("Available networks")
67 }147 }
68148
69 ListView {149 Flickable {
70 id: mainMenu150 anchors.left: parent.left
71151 anchors.right: parent.right
72 property int connectedAPs: 0
73
74 model: menuModel
75 height: column.height - label.height - column.spacing152 height: column.height - label.height - column.spacing
76 anchors.left: parent.left153 contentHeight: contentItem.childrenRect.height
77 anchors.right: parent.right
78 clip: true154 clip: true
79155 flickDeceleration: 1500 * units.gridUnit / 8
80 // Ensure all delegates are cached in order to improve smoothness of scrolling156 maximumFlickVelocity: 2500 * units.gridUnit / 8
81 cacheBuffer: 10000157 boundsBehavior: (contentHeight > height) ? Flickable.DragAndOvershootBounds : Flickable.StopAtBounds
82158
83 // Only allow flicking if the content doesn't fit on the page159 Column {
84 boundsBehavior: (contentHeight > mainMenu.height) ?160 anchors.left: parent.left
85 Flickable.DragAndOvershootBounds :161 anchors.right: parent.right
86 Flickable.StopAtBounds162
87163 Repeater {
88 currentIndex: -1164 id: mainMenu
89 delegate: Menus.AccessPointMenu {165
90 id: menuDelegate166 // FIXME Check connection better https://bugs.launchpad.net/indicator-network/+bug/1349371
91167 property int connectedAPs: 0
92 property int menuIndex: menuModel.mapRowToSource(index)168
93 property var extendedData: model.ext169 model: menuModel
94 property var serverToggle: model.isToggled170
95 property var strengthAction: QMenuModel.UnityMenuAction {171 delegate: Loader {
96 model: unitymenumodel172 id: loader
97 index: menuIndex173
98 name: getExtendedProperty(extendedData, "xCanonicalWifiApStrengthAction", "")174 readonly property bool isAccessPoint: model.type === "unity.widgets.systemsettings.tablet.accesspoint"
99 }175
100176 anchors.left: parent.left
101 anchors {177 anchors.right: parent.right
102 left: parent.left178 height: isAccessPoint ? units.gu(6) : 0
103 right: parent.right179 asynchronous: true
104 }180 sourceComponent: isAccessPoint ? accessPointComponent : null
105 text: model.label181
106 enabled: model.sensitive182 onLoaded: {
107 secure: getExtendedProperty(extendedData, "xCanonicalWifiApIsSecure", false)183 item.menuData = Qt.binding(function() { return model; });
108 adHoc: getExtendedProperty(extendedData, "xCanonicalWifiApIsAdhoc", false)184 item.menuIndex = Qt.binding(function() { return index; });
109 signalStrength: strengthAction.valid ? strengthAction.state : 0185 }
110
111 style: Rectangle {
112 color: "black"
113 opacity: 0.3
114 }
115
116 onCheckedChanged: {
117 if (checked) {
118 mainMenu.connectedAPs++
119 } else {
120 mainMenu.connectedAPs--
121 }186 }
122 }187 }
123 onMenuIndexChanged: {
124 loadAttributes();
125 }
126 onServerToggleChanged: {
127 checked = serverToggle;
128 }
129 onTriggered: {
130 unitymenumodel.activate(menuIndex);
131 }
132
133 function loadAttributes() {
134 if (!unitymenumodel || menuIndex == -1) return;
135 unitymenumodel.loadExtendedAttributes(menuIndex, {'x-canonical-wifi-ap-is-adhoc': 'bool',
136 'x-canonical-wifi-ap-is-secure': 'bool',
137 'x-canonical-wifi-ap-strength-action': 'string'});
138 }
139 }188 }
140 }189 }
141 }190 }
142191
=== 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 @@
108 }108 }
109 }109 }
110110
111 Rectangle {
112 id: modalNotificationBackground
113 visible: notifications.useModal && (notifications.state == "narrow")
114 anchors.fill: parent
115 color: "#80000000"
116
117 MouseArea {
118 anchors.fill: parent
119 }
120 }
121
111 InputMethod {122 InputMethod {
112 anchors.fill: parent123 anchors.fill: parent
113 }124 }

Subscribers

People subscribed via source and target branches