Ubuntu UI Toolkit

Merge lp:~daker/ubuntu-ui-toolkit/fix.1333228 into lp:ubuntu-ui-toolkit/staging

  1. fix.1333228
  2. Merge into staging
Proposed by Adnane Belmadiaf on 2017-02-25
Status: Merged
Approved by: Christian Dywan on 2017-03-01
Approved revision: 2190
Merged at revision: 2183
Proposed branch: lp:~daker/ubuntu-ui-toolkit/fix.1333228
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 163 lines (+108/-3)
3 files modified
examples/ubuntu-ui-toolkit-gallery/Toggles.qml (+43/-0)
src/imports/Components/Themes/Ambiance/1.3/CheckBoxStyle.qml (+17/-3)
tests/unit/visual/tst_toggles.13.qml (+48/-0)
To merge this branch: bzr merge lp:~daker/ubuntu-ui-toolkit/fix.1333228
Reviewer Review Type Date Requested Status
ubuntu-sdk-build-bot continuous-integration 2017-02-25 Approve on 2017-03-01
Christian Dywan Approve on 2017-03-01
Review via email: mp+318311@code.launchpad.net

Commit Message

Add support for CheckBox label when set
Add more tests for checkbox

Description of the Change

Add support for CheckBox label when set
Add support for multiline label
Add more tests for checkbox

To post a comment you must log in.
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Christian Dywan (kalikiana) wrote :

> text: "This a checkbox \n a multiline label \n with long texte"

This reads a bit awkward. How about:

"This is a checkbox\nwith a label\nspanning several lines"

> property int checkedState: checked ? Qt.Checked : Qt.Unchecked

Please drop this from the branch. We essentially don't want to introduce any new features to existing components that will be superseded by QQC2 components eventually. In this case http://doc.qt.io/qt-5/qml-qtquick-controls-checkbox.html#checkedState-prop .

> + count = styledItem.text.split("\n").length

How about using http://doc.qt.io/qt-5/qml-qtquick-text.html#lineCount-prop here. Or better yet, max(contentHeight, units.gu(2))?

review: Needs Fixing
Adnane Belmadiaf (daker) wrote :

I am not sure why but the label don't wrap

lp:~daker/ubuntu-ui-toolkit/fix.1333228 updated on 2017-03-01
2188. By Adnane Belmadiaf on 2017-03-01

Apply fixes from Christian

2189. By Adnane Belmadiaf on 2017-03-01

Revert small change

2190. By Adnane Belmadiaf on 2017-03-01

Remove clip

Christian Dywan (kalikiana) wrote :

Nice. Hit two bugs with one stone. Let's get this in.

review: Approve
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/ubuntu-ui-toolkit-gallery/Toggles.qml'
2--- examples/ubuntu-ui-toolkit-gallery/Toggles.qml 2015-04-25 08:18:45 +0000
3+++ examples/ubuntu-ui-toolkit-gallery/Toggles.qml 2017-03-01 14:50:51 +0000
4@@ -55,6 +55,49 @@
5 checked: true
6 }
7 }
8+
9+ TemplateRow {
10+ title: i18n.tr("Checkbox with label")
11+
12+ CheckBox {
13+ objectName: "checkbox_checked_lbl"
14+ checked: true
15+ text: "This a checkbox label"
16+ }
17+ }
18+
19+ TemplateRow {
20+ title: i18n.tr("Disabled checkbox with label")
21+
22+ CheckBox {
23+ objectName: "checkbox_disabled_checked_lbl"
24+ checked: true
25+ enabled: false
26+ text: "This a checkbox label"
27+ }
28+ }
29+
30+ TemplateRow {
31+ title: i18n.tr("Disabled checkbox with label")
32+
33+ CheckBox {
34+ objectName: "checkbox_disabled_checked_lbl"
35+ checked: false
36+ enabled: false
37+ text: "This a checkbox label"
38+ }
39+ }
40+
41+ TemplateRow {
42+ title: i18n.tr("Checkbox with multiline label")
43+
44+ CheckBox {
45+ objectName: "checkbox_checked_lbl"
46+ checked: true
47+ text: "This is a checkbox with a built-in label spanning several lines that won't be ellipsized but increase in height instead"
48+ width: parent.width
49+ }
50+ }
51 }
52
53
54
55=== modified file 'src/imports/Components/Themes/Ambiance/1.3/CheckBoxStyle.qml'
56--- src/imports/Components/Themes/Ambiance/1.3/CheckBoxStyle.qml 2016-01-27 15:17:56 +0000
57+++ src/imports/Components/Themes/Ambiance/1.3/CheckBoxStyle.qml 2017-03-01 14:50:51 +0000
58@@ -57,7 +57,7 @@
59 property real iconPadding: units.gu(0.4)
60
61 implicitWidth: units.gu(2)
62- implicitHeight: units.gu(2)
63+ implicitHeight: Math.max(checkBoxLbl.contentHeight, units.gu(2))
64
65 FocusShape {
66 }
67@@ -65,10 +65,12 @@
68 UbuntuShape {
69 id: background
70 anchors {
71- fill: parent
72 margins: checkBoxStyle.backgroundPadding
73 }
74- property real iconSize: Math.min(width, height) - 2*checkBoxStyle.iconPadding
75+ width: units.gu(2)
76+ height: units.gu(2)
77+
78+ property real iconSize: units.gu(2) - 2*checkBoxStyle.iconPadding
79
80 Icon {
81 color: checkBoxStyle.iconColor
82@@ -164,4 +166,16 @@
83 }
84 ]
85 }
86+
87+ Label {
88+ id: checkBoxLbl
89+ text: styledItem.text
90+ anchors.left: background.right
91+ anchors.leftMargin: units.gu(1)
92+ anchors.right: parent.right
93+ height: parent.height
94+ enabled: styledItem.enabled
95+ visible: styledItem.text
96+ wrapMode: Text.WordWrap
97+ }
98 }
99
100=== modified file 'tests/unit/visual/tst_toggles.13.qml'
101--- tests/unit/visual/tst_toggles.13.qml 2016-06-15 13:46:51 +0000
102+++ tests/unit/visual/tst_toggles.13.qml 2017-03-01 14:50:51 +0000
103@@ -39,6 +39,23 @@
104 property bool checkedNow: true
105 onClicked: checkedNow = checked
106 }
107+
108+ CheckBox {
109+ id: testCheckLbl
110+ checked: true
111+ text: "This a checkbox label"
112+ property bool checkedNow: true
113+ onClicked: checkedNow = checked
114+ }
115+
116+ CheckBox {
117+ id: testCheckLblDisabled
118+ checked: false
119+ enabled: false
120+ text: "This a checkbox label"
121+ property bool checkedNow: false
122+ onClicked: checkedNow = checked
123+ }
124 }
125
126 UbuntuTestCase {
127@@ -75,6 +92,37 @@
128 clickedSpy.wait(400);
129 compare(data.testItem.checkedNow, data.testItem.checked);
130 }
131+
132+
133+ function test_toggle_lbl_checked_data() {
134+ return [
135+ {tag: "CheckBox", testItem: testCheckLbl, mouse: true},
136+ ];
137+ }
138+
139+ function test_toggle_lbl_checked(data) {
140+ data.testItem.checkedNow = data.testItem.checked;
141+ data.testItem.forceActiveFocus();
142+ clickedSpy.target = data.testItem;
143+ mouseClick(data.testItem, centerOf(data.testItem).x, centerOf(data.testItem).y);
144+ clickedSpy.wait(400);
145+ compare(data.testItem.checkedNow, data.testItem.checked);
146+ }
147+
148+ function test_toggle_lbl_checked_disabled_data() {
149+ return [
150+ {tag: "CheckBox", testItem: testCheckLblDisabled, mouse: true},
151+ ];
152+ }
153+
154+ function test_toggle_lbl_checked_disabled(data) {
155+ data.testItem.checkedNow = data.testItem.checked;
156+ data.testItem.forceActiveFocus();
157+ clickedSpy.target = data.testItem;
158+ mouseClick(data.testItem, centerOf(data.testItem).x, centerOf(data.testItem).y);
159+ compare(clickedSpy.count, 0);
160+ compare(data.testItem.checkedNow, data.testItem.checked);
161+ }
162 }
163 }
164

Subscribers

People subscribed via source and target branches