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
=== modified file 'examples/ubuntu-ui-toolkit-gallery/Toggles.qml'
--- examples/ubuntu-ui-toolkit-gallery/Toggles.qml 2015-04-25 08:18:45 +0000
+++ examples/ubuntu-ui-toolkit-gallery/Toggles.qml 2017-03-01 14:50:51 +0000
@@ -55,6 +55,49 @@
55 checked: true55 checked: true
56 }56 }
57 }57 }
58
59 TemplateRow {
60 title: i18n.tr("Checkbox with label")
61
62 CheckBox {
63 objectName: "checkbox_checked_lbl"
64 checked: true
65 text: "This a checkbox label"
66 }
67 }
68
69 TemplateRow {
70 title: i18n.tr("Disabled checkbox with label")
71
72 CheckBox {
73 objectName: "checkbox_disabled_checked_lbl"
74 checked: true
75 enabled: false
76 text: "This a checkbox label"
77 }
78 }
79
80 TemplateRow {
81 title: i18n.tr("Disabled checkbox with label")
82
83 CheckBox {
84 objectName: "checkbox_disabled_checked_lbl"
85 checked: false
86 enabled: false
87 text: "This a checkbox label"
88 }
89 }
90
91 TemplateRow {
92 title: i18n.tr("Checkbox with multiline label")
93
94 CheckBox {
95 objectName: "checkbox_checked_lbl"
96 checked: true
97 text: "This is a checkbox with a built-in label spanning several lines that won't be ellipsized but increase in height instead"
98 width: parent.width
99 }
100 }
58 }101 }
59102
60103
61104
=== modified file 'src/imports/Components/Themes/Ambiance/1.3/CheckBoxStyle.qml'
--- src/imports/Components/Themes/Ambiance/1.3/CheckBoxStyle.qml 2016-01-27 15:17:56 +0000
+++ src/imports/Components/Themes/Ambiance/1.3/CheckBoxStyle.qml 2017-03-01 14:50:51 +0000
@@ -57,7 +57,7 @@
57 property real iconPadding: units.gu(0.4)57 property real iconPadding: units.gu(0.4)
5858
59 implicitWidth: units.gu(2)59 implicitWidth: units.gu(2)
60 implicitHeight: units.gu(2)60 implicitHeight: Math.max(checkBoxLbl.contentHeight, units.gu(2))
6161
62 FocusShape {62 FocusShape {
63 }63 }
@@ -65,10 +65,12 @@
65 UbuntuShape {65 UbuntuShape {
66 id: background66 id: background
67 anchors {67 anchors {
68 fill: parent
69 margins: checkBoxStyle.backgroundPadding68 margins: checkBoxStyle.backgroundPadding
70 }69 }
71 property real iconSize: Math.min(width, height) - 2*checkBoxStyle.iconPadding70 width: units.gu(2)
71 height: units.gu(2)
72
73 property real iconSize: units.gu(2) - 2*checkBoxStyle.iconPadding
7274
73 Icon {75 Icon {
74 color: checkBoxStyle.iconColor76 color: checkBoxStyle.iconColor
@@ -164,4 +166,16 @@
164 }166 }
165 ]167 ]
166 }168 }
169
170 Label {
171 id: checkBoxLbl
172 text: styledItem.text
173 anchors.left: background.right
174 anchors.leftMargin: units.gu(1)
175 anchors.right: parent.right
176 height: parent.height
177 enabled: styledItem.enabled
178 visible: styledItem.text
179 wrapMode: Text.WordWrap
180 }
167}181}
168182
=== modified file 'tests/unit/visual/tst_toggles.13.qml'
--- tests/unit/visual/tst_toggles.13.qml 2016-06-15 13:46:51 +0000
+++ tests/unit/visual/tst_toggles.13.qml 2017-03-01 14:50:51 +0000
@@ -39,6 +39,23 @@
39 property bool checkedNow: true39 property bool checkedNow: true
40 onClicked: checkedNow = checked40 onClicked: checkedNow = checked
41 }41 }
42
43 CheckBox {
44 id: testCheckLbl
45 checked: true
46 text: "This a checkbox label"
47 property bool checkedNow: true
48 onClicked: checkedNow = checked
49 }
50
51 CheckBox {
52 id: testCheckLblDisabled
53 checked: false
54 enabled: false
55 text: "This a checkbox label"
56 property bool checkedNow: false
57 onClicked: checkedNow = checked
58 }
42 }59 }
4360
44 UbuntuTestCase {61 UbuntuTestCase {
@@ -75,6 +92,37 @@
75 clickedSpy.wait(400);92 clickedSpy.wait(400);
76 compare(data.testItem.checkedNow, data.testItem.checked);93 compare(data.testItem.checkedNow, data.testItem.checked);
77 }94 }
95
96
97 function test_toggle_lbl_checked_data() {
98 return [
99 {tag: "CheckBox", testItem: testCheckLbl, mouse: true},
100 ];
101 }
102
103 function test_toggle_lbl_checked(data) {
104 data.testItem.checkedNow = data.testItem.checked;
105 data.testItem.forceActiveFocus();
106 clickedSpy.target = data.testItem;
107 mouseClick(data.testItem, centerOf(data.testItem).x, centerOf(data.testItem).y);
108 clickedSpy.wait(400);
109 compare(data.testItem.checkedNow, data.testItem.checked);
110 }
111
112 function test_toggle_lbl_checked_disabled_data() {
113 return [
114 {tag: "CheckBox", testItem: testCheckLblDisabled, mouse: true},
115 ];
116 }
117
118 function test_toggle_lbl_checked_disabled(data) {
119 data.testItem.checkedNow = data.testItem.checked;
120 data.testItem.forceActiveFocus();
121 clickedSpy.target = data.testItem;
122 mouseClick(data.testItem, centerOf(data.testItem).x, centerOf(data.testItem).y);
123 compare(clickedSpy.count, 0);
124 compare(data.testItem.checkedNow, data.testItem.checked);
125 }
78 }126 }
79}127}
80128

Subscribers

People subscribed via source and target branches