Ubuntu Settings Components

Merge lp:~pete-woods/ubuntu-settings-components/add-ethernet-item into lp:ubuntu-settings-components

add-ethernet-item
Merge into trunk

Proposed by Pete Woods on 2016-11-22

Status:	Rejected
Rejected by:	Pete Woods on 2016-11-30
Proposed branch:	lp:~pete-woods/ubuntu-settings-components/add-ethernet-item
Merge into:	lp:ubuntu-settings-components
Diff against target:	264 lines (+225/-0) 5 files modified debian/changelog (+6/-0) plugins/Ubuntu/Settings/Menus/EthernetItem.qml (+85/-0) plugins/Ubuntu/Settings/Menus/qmldir (+1/-0) tests/qmltests/CMakeLists.txt (+1/-0) tests/qmltests/Menus/tst_EthernetItem.qml (+132/-0)
To merge this branch:	bzr merge lp:~pete-woods/ubuntu-settings-components/add-ethernet-item
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Marco Trevisan (Treviño)		2016-11-28	Disapprove on 2016-12-01
Michał Sawicz		2016-11-22	Abstain on 2016-11-25
Unity8 CI Bot	continuous-integration		Approve on 2016-11-25
Review via email: mp+311503@code.launchpad.net

Commit Message

Add Menus.EthernetItem in support of indicator-network

Description of the Change

* Are there any related MPs required for this MP to build/function as expected? Please list.

N/A

* Did you perform an exploratory manual test run of your code change and any related functionality?

Tested on laptop. Phone/tablet devices don't have ethernet support.

* 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?

Yes, implementation was carried out with design team oversight.

* If you changed localized strings, has the POT file been updated?

N/A

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-11-24:

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

review: Approve (continuous-integration)

Michał Sawicz (saviq) wrote on 2016-11-24:

Please bump changelog here so you can depend on the new item.

review: Needs Fixing

lp:~pete-woods/ubuntu-settings-components/add-ethernet-item updated on 2016-11-25

174. By Pete Woods on 2016-11-25: Dump debian version

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-11-25:

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

review: Approve (continuous-integration)

Michał Sawicz (saviq) wrote on 2016-11-25:

Thanks!

review: Abstain

Marco Trevisan (Treviño) (3v1n0) wrote on 2016-11-28:

Looks good, but I think you can just reuse the BaseLayoutMenu with some "hacks".
And I'd avoid to send the triggered event when tapping on the item only.

So please, use this http://paste.ubuntu.com/23548192/ and you can just update the tests.
Also, please include an item in examples/OtherComponents.qml.

I'm also refactoring these components so that we don't have to use many typed items, but having a such checkable item that can be more generic.

Anyway it's fine for now... I was just wondering where's the design of the statusIcon item, as I didn't see it in the docs I've.

PS: if you'd need to add this to unity8, you also could add a breaks line to debian control to the bumped unity8 version, not to make autopkgtests angry on upgrade.

Marco Trevisan (Treviño) (3v1n0) wrote on 2016-11-28:

Ah, if instead tapping in the item should toggle the switch, do the same we do in the SwitchMenu (or just rebase on that with something like http://paste.ubuntu.com/23548269/)

Pete Woods (pete-woods) wrote on 2016-11-30:

Hmm. You've managed to make this code so trivial that it seems pointless even adding this widget at all.

Pete Woods (pete-woods) wrote on 2016-11-30:

Especially as I got feedback from design to not include the icon any more.

Marco Trevisan (Treviño) (3v1n0) wrote on 2016-12-01:

Ok, I think we can reject this then... Having more generic items is something I'd prefer to solve this kind of cases ;-).

review: Disapprove

Unmerged revisions

174. By Pete Woods on 2016-11-25: Dump debian version
173. By Pete Woods on 2016-11-22: Add EthernetItem widget

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Pete Woods

Unity Team

 === modified file 'debian/changelog'
 --- debian/changelog	2016-11-16 18:54:00 +0000
 +++ debian/changelog	2016-11-25 13:42:57 +0000
@@ -1,3 +1,9 @@
++ubuntu-settings-components (0.12-0ubuntu1) UNRELEASED; urgency=medium
++
++  * Add Menus.EthernetItem in support of indicator-network.
++
++ -- Pete <pete.woods@canonical.com>  Fri, 25 Nov 2016 13:41:29 +0000
++
  ubuntu-settings-components (0.11+17.04.20161116.1-0ubuntu1) zesty; urgency=medium
    * Menus: add Pointer and Touch styles to be used depending on the user
 === added file 'plugins/Ubuntu/Settings/Menus/EthernetItem.qml'
 --- plugins/Ubuntu/Settings/Menus/EthernetItem.qml	1970-01-01 00:00:00 +0000
 +++ plugins/Ubuntu/Settings/Menus/EthernetItem.qml	2016-11-25 13:42:57 +0000
@@ -0,0 +1,85 @@
++/*
++ * Copyright 2014-2016 Canonical Ltd.
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU Lesser General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Author: Pete Woods <pete.woods@canonical.com>
++ */
++
++import QtQuick 2.4
++import QtQuick.Layouts 1.1
++import Ubuntu.Components 1.3
++import Ubuntu.Settings.Menus.Style 0.1
++
++BaseMenu {
++    id: menu
++    property alias statusIcon: statusIcon.name
++    property alias statusText: labelStatus.text
++    property alias name: labelName.text
++    property alias checked: switcher.checked
++
++    menuHeight: slotsLayout.height
++
++    StyledSlotsLayout {
++        id: slotsLayout
++        objectName: "ethernetSlotsLayout"
++        style: menuStyle
++
++        mainSlot: ColumnLayout {
++            spacing: units.gu(0.5)
++
++            Label {
++                id: labelName
++                objectName: "labelName"
++                elide: Text.ElideRight
++                color: menu.foregroundColor
++                font.bold: true
++                font.pixelSize: menuStyle.fontSize
++            }
++
++            Row {
++                id: statusRow
++                spacing: units.gu(1)
++                height: labelStatus.height
++
++                Label {
++                    id: labelStatus
++                    objectName: "labelStatus"
++                    elide: Text.ElideRight
++                    opacity: 0.6
++                    color: menu.foregroundColor
++                    font.pixelSize: menuStyle.fontSize
++                }
++
++                Icon {
++                    id: statusIcon
++                    objectName: "statusIcon"
++                    color: menuStyle.iconColor
++
++                    height: menuStyle.iconSize
++                    width: height
++
++                    visible: name !== ""
++                }
++            }
++        }
++
++        Switch {
++            id: switcher
++            objectName: "switcher"
++            onClicked: menu.triggered(checked)
++
++            SlotsLayout.position: SlotsLayout.Trailing
++        }
++    }
++}
 === modified file 'plugins/Ubuntu/Settings/Menus/qmldir'
 --- plugins/Ubuntu/Settings/Menus/qmldir	2016-10-10 22:27:38 +0000
 +++ plugins/Ubuntu/Settings/Menus/qmldir	2016-11-25 13:42:57 +0000
@@ -8,6 +8,7 @@
  ButtonMenu 0.1 ButtonMenu.qml
  CalendarMenu 0.1 CalendarMenu.qml
  CheckableMenu 0.1 CheckableMenu.qml
++EthernetItem 0.1 EthernetItem.qml
  EventMenu 0.1 EventMenu.qml
  GroupedMessageMenu 0.1 GroupedMessageMenu.qml
  MediaPlayerMenu 0.1 MediaPlayerMenu.qml
 === modified file 'tests/qmltests/CMakeLists.txt'
 --- tests/qmltests/CMakeLists.txt	2016-10-11 13:51:11 +0000
 +++ tests/qmltests/CMakeLists.txt	2016-11-25 13:42:57 +0000
@@ -9,6 +9,7 @@
  add_usc_qmltest(Menus ButtonMenu)
  add_usc_qmltest(Menus CalendarMenu)
  add_usc_qmltest(Menus CheckableMenu)
++add_usc_qmltest(Menus EthernetItem)
  add_usc_qmltest(Menus EventMenu)
  add_usc_qmltest(Menus GroupedMessageMenu)
  add_usc_qmltest(Menus MediaPlayerMenu)
 === added file 'tests/qmltests/Menus/tst_EthernetItem.qml'
 --- tests/qmltests/Menus/tst_EthernetItem.qml	1970-01-01 00:00:00 +0000
 +++ tests/qmltests/Menus/tst_EthernetItem.qml	2016-11-25 13:42:57 +0000
@@ -0,0 +1,132 @@
++/*
++ * Copyright 2016 Canonical Ltd.
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU Lesser General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by Pete Woods <pete.woods@canonical.com>
++ */
++
++import QtQuick 2.4
++import QtTest 1.0
++import Ubuntu.Test 0.1
++import Ubuntu.Settings.Menus 0.1
++
++Item {
++    width: units.gu(42)
++    height: units.gu(75)
++
++    Column {
++        anchors.fill: parent
++
++        EthernetItem {
++            id: ethernetItem
++        }
++
++        EthernetItem {
++            id: checkedEthernetItem
++            checked: true
++        }
++
++        EthernetItem {
++            id: clickEthernetItem
++        }
++    }
++
++    SignalSpy {
++        id: signalSpy
++    }
++
++    UbuntuTestCase {
++        name: "BaseMenu"
++        when: windowShown
++
++        function getSwitchItem(item) {
++            var switchItem = findChild(item, "switcher");
++            verify(switchItem !== undefined)
++            return switchItem
++        }
++
++        function cleanup() {
++            clickEthernetItem.checked = false
++            signalSpy.clear()
++        }
++
++        function test_checked() {
++            compare(getSwitchItem(ethernetItem).checked, false, "should be unchecked by default")
++        }
++
++        function test_unChecked() {
++            compare(getSwitchItem(checkedEthernetItem).checked, true, "should be checked")
++        }
++
++        function checkedData() {
++            return [ {tag: "unchecked", checked: false}, {tag: "checked", checked: true}]
++        }
++
++        function test_changing_data() {
++            return checkedData()
++        }
++
++        function test_changing(data) {
++            clickEthernetItem.checked = data.checked
++            compare(getSwitchItem(clickEthernetItem).checked, data.checked, "switch should be %1".arg(data.tag))
++        }
++
++        function test_clickEvent_data() {
++            return checkedData()
++        }
++
++        function test_clickEvent(data) {
++            signalSpy.target = clickEthernetItem
++            signalSpy.signalName = "onClicked"
++            clickEthernetItem.checked = data.checked
++            mouseClick(clickEthernetItem, clickEthernetItem.width/2, clickEthernetItem.height/2)
++            compare(signalSpy.count, 1)
++        }
++
++        function test_onCheckedChanged_data() {
++            return checkedData()
++        }
++
++        function test_onCheckedChanged(data) {
++            clickEthernetItem.checked = data.checked
++            signalSpy.clear()
++            signalSpy.target = clickEthernetItem
++            signalSpy.signalName = "onTriggered"
++            mouseClick(clickEthernetItem, clickEthernetItem.width/2, clickEthernetItem.height/2)
++            compare(signalSpy.count, 1)
++        }
++
++        function getChild(name) {
++            var child = findChild(ethernetItem, name)
++            verify(child !== undefined)
++            return child
++        }
++
++        function test_nameStatusIcon_data() {
++            return [{name: "Ethernet (eth0)", statusText: "Connected", statusIcon: "network-wired-connected"},
++                    {name: "Ethernet", statusText: "Connecting", statusIcon: "network-wired-active"},
++                    {name: "Ethernet", statusText: "Disabled", statusIcon: "network-wired-disabled"}]
++        }
++
++        function test_nameStatusIcon(data) {
++            ethernetItem.name = data.name
++            ethernetItem.statusText = data.statusText
++            ethernetItem.statusIcon = data.statusIcon
++
++            compare(getChild("labelName").text, data.name)
++            compare(getChild("labelStatus").text, data.statusText)
++            compare(getChild("statusIcon").name, data.statusIcon)
++        }
++    }
++}