Ubuntu UI Toolkit

Merge lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent into lp:ubuntu-ui-toolkit/staging

  1. focusPassesFromChildToParent
  2. Merge into staging
Proposed by Christian Dywan on 2015-11-19
Status: Merged
Approved by: Zsombor Egri on 2015-11-30
Approved revision: 1725
Merged at revision: 1732
Proposed branch: lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent
Merge into: lp:ubuntu-ui-toolkit/staging
Prerequisite: lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/enoFocusPriSecItem
Diff against target: 243 lines (+60/-11)
8 files modified
components.api (+1/-0)
src/Ubuntu/Components/1.3/TextArea.qml (+9/-1)
src/Ubuntu/Components/1.3/TextField.qml (+12/-1)
src/Ubuntu/Components/plugin/ucabstractbutton.cpp (+1/-0)
src/Ubuntu/Components/plugin/ucstyleditembase.cpp (+10/-1)
src/Ubuntu/Components/plugin/ucstyleditembase.h (+9/-0)
tests/unit_x11/tst_components/tst_focus.qml (+4/-3)
tests/unit_x11/tst_components/tst_textinput_common13.qml (+14/-5)
To merge this branch: bzr merge lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration 2015-11-19 Approve on 2015-11-30
Zsombor Egri (community) 2015-11-19 Approve on 2015-11-30
Review via email: mp+277962@code.launchpad.net

This proposal supersedes a proposal from 2015-11-18.

Commit Message

Don't set activeFocusOnPress on TextField but on child only

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:1719
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/~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent/+merge/277815/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2501/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1229
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1231
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1231/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1228

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2501/rebuild

review: Needs Fixing (continuous-integration)
Zsombor Egri (zsombi) wrote :

A small comment, otherwise it's good to go from my side!!!

review: Needs Fixing
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent updated on 2015-11-19
1720. By Christian Dywan on 2015-11-19

More solid tests and more robust fix

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent updated on 2015-11-23
1721. By Christian Dywan on 2015-11-23

Add qdoc comment for activeFocusOnPress

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent updated on 2015-11-25
1722. By Christian Dywan on 2015-11-25

Consistently apply activeFocusOnTab everywhere

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent updated on 2015-11-26
1723. By Christian Dywan on 2015-11-26

Merge lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/staging

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent updated on 2015-11-30
1724. By Christian Dywan on 2015-11-30

Make the must_not_grab_focus test clearer

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/focusPassesFromChildToParent updated on 2015-11-30
1725. By Christian Dywan on 2015-11-30

Make the test actually work

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Zsombor Egri (zsombi) wrote :

Still like it :)

review: Approve
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'components.api'
2--- components.api 2015-11-27 11:00:06 +0000
3+++ components.api 2015-11-30 10:37:31 +0000
4@@ -1037,6 +1037,7 @@
5 property bool ignoreUnknownProperties
6 Ubuntu.Components.StyledItem 1.3 1.3 1.1 1.0 0.1: Item
7 property bool activeFocusOnPress 1.3
8+ signal activeFocusOnTabChanged2() 1.3
9 function bool requestFocus(Qt.FocusReason reason) 1.3
10 function bool requestFocus() 1.3
11 property Component style
12
13=== modified file 'src/Ubuntu/Components/1.3/TextArea.qml'
14--- src/Ubuntu/Components/1.3/TextArea.qml 2015-09-28 14:36:54 +0000
15+++ src/Ubuntu/Components/1.3/TextArea.qml 2015-11-30 10:37:31 +0000
16@@ -538,6 +538,13 @@
17 */
18 property alias wrapMode:editor.wrapMode
19
20+ /*!
21+ Whether the TextArea should gain active focus on a mouse press. By default
22+ this is set to true.
23+ \qmlproperty bool activeFocusOnPress
24+ */
25+ property alias activeFocusOnPress: editor.activeFocusOnPress
26+
27 // signals
28 /*!
29 This handler is called when the user clicks on a link embedded in the text.
30@@ -752,6 +759,7 @@
31
32 opacity: enabled ? 1.0 : 0.3
33 activeFocusOnPress: true
34+ activeFocusOnTab: true
35
36 /*!\internal */
37 onVisibleChanged: {
38@@ -858,7 +866,7 @@
39 wrapMode: TextEdit.WrapAtWordBoundaryOrAnywhere
40 mouseSelectionMode: TextEdit.SelectWords
41 selectByMouse: true
42- activeFocusOnPress: control.activeFocusOnPress
43+ activeFocusOnPress: true
44 onActiveFocusChanged: if (!activeFocus && inputHandler.popover) PopupUtils.close(inputHandler.popover)
45 cursorDelegate: TextCursor {
46 handler: inputHandler
47
48=== modified file 'src/Ubuntu/Components/1.3/TextField.qml'
49--- src/Ubuntu/Components/1.3/TextField.qml 2015-11-17 15:17:20 +0000
50+++ src/Ubuntu/Components/1.3/TextField.qml 2015-11-30 10:37:31 +0000
51@@ -172,6 +172,13 @@
52 property alias acceptableInput: editor.acceptableInput
53
54 /*!
55+ Whether the TextField should gain active focus on a mouse press. By default
56+ this is set to true.
57+ \qmlproperty bool activeFocusOnPress
58+ */
59+ property alias activeFocusOnPress: editor.activeFocusOnPress
60+
61+ /*!
62 Whether the TextField should scroll when the text is longer than the width.
63 By default this is set to true.
64
65@@ -810,6 +817,7 @@
66
67 opacity: enabled ? 1.0 : 0.3
68 activeFocusOnPress: true
69+ activeFocusOnTab: true
70
71 /*! \internal */
72 onVisibleChanged: {
73@@ -871,6 +879,7 @@
74 children[i].parent = leftPane;
75 children[i].anchors.verticalCenter = verticalCenter;
76 children[i].activeFocusOnPress = false;
77+ children[i].activeFocusOnTab = false;
78 }
79 }
80 }
81@@ -894,6 +903,7 @@
82 children[i].parent = rightPane;
83 children[i].anchors.verticalCenter = verticalCenter;
84 children[i].activeFocusOnPress = false;
85+ children[i].activeFocusOnTab = false;
86 }
87 }
88 }
89@@ -902,6 +912,7 @@
90 id: clearButton
91 objectName: "clear_button"
92 activeFocusOnPress: false
93+ activeFocusOnTab: false
94
95 anchors {
96 top: parent.top
97@@ -995,7 +1006,7 @@
98
99 // overrides
100 selectByMouse: true
101- activeFocusOnPress: control.activeFocusOnPress
102+ activeFocusOnPress: true
103 onActiveFocusChanged: if (!activeFocus && inputHandler.popover) PopupUtils.close(inputHandler.popover)
104
105 // input selection and navigation handling
106
107=== modified file 'src/Ubuntu/Components/plugin/ucabstractbutton.cpp'
108--- src/Ubuntu/Components/plugin/ucabstractbutton.cpp 2015-10-28 15:49:24 +0000
109+++ src/Ubuntu/Components/plugin/ucabstractbutton.cpp 2015-11-30 10:37:31 +0000
110@@ -55,6 +55,7 @@
111 , m_pressAndHoldConnected(false)
112 {
113 setActiveFocusOnPress(true);
114+ setActiveFocusOnTab(true);
115 }
116
117 bool UCAbstractButton::isPressAndHoldConnected()
118
119=== modified file 'src/Ubuntu/Components/plugin/ucstyleditembase.cpp'
120--- src/Ubuntu/Components/plugin/ucstyleditembase.cpp 2015-10-20 11:32:09 +0000
121+++ src/Ubuntu/Components/plugin/ucstyleditembase.cpp 2015-11-30 10:37:31 +0000
122@@ -34,6 +34,15 @@
123 {
124 }
125
126+bool UCStyledItemBase::activeFocusOnTab2() const
127+{
128+ return activeFocusOnTab();
129+}
130+void UCStyledItemBase::setActiveFocusOnTab2(bool v)
131+{
132+ setActiveFocusOnTab(v);
133+}
134+
135 UCStyledItemBasePrivate::~UCStyledItemBasePrivate()
136 {
137 }
138@@ -42,6 +51,7 @@
139 {
140 Q_Q(UCStyledItemBase);
141 q->setFlag(QQuickItem::ItemIsFocusScope);
142+ QObject::connect(q, &QQuickItem::activeFocusOnTabChanged, q, &UCStyledItemBase::activeFocusOnTabChanged2);
143 }
144
145
146@@ -202,7 +212,6 @@
147 return;
148 d->activeFocusOnPress = value;
149 d->setFocusable(d->activeFocusOnPress);
150- setActiveFocusOnTab(value);
151 Q_EMIT activeFocusOnPressChanged();
152 }
153
154
155=== modified file 'src/Ubuntu/Components/plugin/ucstyleditembase.h'
156--- src/Ubuntu/Components/plugin/ucstyleditembase.h 2015-10-20 10:19:10 +0000
157+++ src/Ubuntu/Components/plugin/ucstyleditembase.h 2015-11-30 10:37:31 +0000
158@@ -32,6 +32,12 @@
159 Q_PROPERTY(bool activeFocusOnPress
160 READ activefocusOnPress WRITE setActiveFocusOnPress
161 NOTIFY activeFocusOnPressChanged REVISION 1)
162+ // FIXME Re-expose property that would be inaccessible due to a QML bug
163+ // https://bugs.launchpad.net/ubuntu/+source/qtdeclarative-opensource-src/+bug/1389721
164+ Q_PROPERTY(bool activeFocusOnTab
165+ READ activeFocusOnTab2
166+ WRITE setActiveFocusOnTab2
167+ NOTIFY activeFocusOnTabChanged2 FINAL)
168 Q_PRIVATE_PROPERTY(UCStyledItemBase::d_func(), QQmlComponent *style READ style WRITE setStyle RESET resetStyle NOTIFY styleChanged FINAL DESIGNABLE false)
169 Q_PRIVATE_PROPERTY(UCStyledItemBase::d_func(), QQuickItem *__styleInstance READ styleInstance NOTIFY styleInstanceChanged FINAL DESIGNABLE false)
170 Q_PRIVATE_PROPERTY(UCStyledItemBase::d_func(), QString styleName READ styleName WRITE setStyleName NOTIFY styleNameChanged FINAL REVISION 2)
171@@ -41,6 +47,8 @@
172
173 bool activefocusOnPress() const;
174 void setActiveFocusOnPress(bool value);
175+ bool activeFocusOnTab2() const;
176+ void setActiveFocusOnTab2(bool value);
177
178 public Q_SLOTS:
179 Q_REVISION(1) bool requestFocus(Qt::FocusReason reason = Qt::OtherFocusReason);
180@@ -49,6 +57,7 @@
181 void styleChanged();
182 void styleInstanceChanged();
183 Q_REVISION(1) void activeFocusOnPressChanged();
184+ Q_REVISION(1) void activeFocusOnTabChanged2();
185 Q_REVISION(2) void themeChanged();
186 Q_REVISION(2) void styleNameChanged();
187
188
189=== modified file 'tests/unit_x11/tst_components/tst_focus.qml'
190--- tests/unit_x11/tst_components/tst_focus.qml 2015-07-29 03:47:05 +0000
191+++ tests/unit_x11/tst_components/tst_focus.qml 2015-11-30 10:37:31 +0000
192@@ -192,10 +192,11 @@
193 }
194 function test_tab_focus(data) {
195 data.from.forceActiveFocus();
196- verify(data.from.focus, "Source component is not focused");
197+ verify(data.from.activeFocus, "Source component is not focused");
198 keyClick(data.key);
199- waitForRendering(data.to, 200);
200- verify(data.to.focus, "Target component is not focused");
201+ waitForRendering(data.to, 500);
202+ verify(!data.from.activeFocus, "Source component still keeps focus");
203+ verify(data.to.activeFocus, "Target component is not focused");
204 }
205
206 function test_hide_osk_when_pickerpanel_opens() {
207
208=== modified file 'tests/unit_x11/tst_components/tst_textinput_common13.qml'
209--- tests/unit_x11/tst_components/tst_textinput_common13.qml 2015-11-23 20:19:31 +0000
210+++ tests/unit_x11/tst_components/tst_textinput_common13.qml 2015-11-30 10:37:31 +0000
211@@ -490,18 +490,27 @@
212 verify(availableHeight <= expectedHeight, 'Dialog did not shrink (%1 > %2)'.arg(availableHeight).arg(expectedHeight));
213 }
214
215- function test_secondaryItem_must_not_grab_focus() {
216- var textField = customTextField;
217+ function test_secondaryItem_must_not_grab_focus_data() {
218+ return [
219+ { tag: 'same', input: textField },
220+ { tag: 'other', input: customTextField },
221+ ];
222+ }
223+
224+ function test_secondaryItem_must_not_grab_focus(data) {
225 textField.forceActiveFocus();
226 compare(textField.focus, true, 'TextField is focused');
227
228 var clearButton = findChild(textField, "clear_button")
229 mouseClick(clearButton, clearButton.width/2, clearButton.height/2);
230- compare(textField.focus, true, 'TextField remains focused');
231+ waitForRendering(data.input, 500);
232+ compare(textField.focus, true, 'TextField no longer focused');
233 mouseClick(primaryButton, primaryButton.width/2, primaryButton.height/2);
234- compare(textField.focus, true, 'TextField remains focused');
235+ waitForRendering(data.input, 500);
236+ compare(textField.focus, true, 'TextField no longer focused');
237 mouseClick(secondaryButton, secondaryButton.width/2, secondaryButton.height/2);
238- compare(textField.focus, true, 'TextField remains focused');
239+ waitForRendering(data.input, 500);
240+ compare(textField.focus, true, 'TextField no longer focused');
241 }
242 }
243 }

Subscribers

People subscribed via source and target branches