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

Proposed by Adnane Belmadiaf
Status: Merged
Approved by: Cris Dywan
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 Approve
Cris Dywan Approve
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.
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Cris 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
Revision history for this message
Adnane Belmadiaf (daker) wrote :

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

2188. By Adnane Belmadiaf

Apply fixes from Christian

2189. By Adnane Belmadiaf

Revert small change

2190. By Adnane Belmadiaf

Remove clip

Revision history for this message
Cris Dywan (kalikiana) wrote :

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

review: Approve
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
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