Merge lp:~tpeeters/ubuntu-ui-toolkit/listitem-textarea into lp:ubuntu-ui-toolkit

Proposed by Tim Peeters
Status: Merged
Approved by: Florian Boucault
Approved revision: 422
Merged at revision: 419
Proposed branch: lp:~tpeeters/ubuntu-ui-toolkit/listitem-textarea
Merge into: lp:ubuntu-ui-toolkit
Diff against target: 93 lines (+11/-14)
4 files modified
CHANGES (+2/-1)
modules/Ubuntu/Components/AbstractButton.qml (+4/-4)
modules/Ubuntu/Components/ListItems/SingleControl.qml (+1/-1)
modules/Ubuntu/Components/ListItems/Standard.qml (+4/-8)
To merge this branch: bzr merge lp:~tpeeters/ubuntu-ui-toolkit/listitem-textarea
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Tim Peeters Needs Information
Florian Boucault (community) Needs Fixing
Michał Sawicz Disapprove
Zsombor Egri Pending
Review via email: mp+158456@code.launchpad.net

Commit message

Remove mouse parameters from AbstractButton's signals.

Description of the change

Remove mouse parameters from AbstractButton's signals.

To post a comment you must log in.
Revision history for this message
Tim Peeters (tpeeters) wrote :

See linked bug why I made this change. We are not using keyboard handling anywhere, do we really need it now? Also, the clicked() signal currently needs a mouse parameter that the keyboard events cannot give it.

review: Needs Information
Revision history for this message
Michał Sawicz (saviq) wrote :

Even if not right now, we need support for keyboard navigation really soon. The core apps that are being developed might be as useful on the Ubuntu desktop, and IMO instead of removing this, we should fix it, if it poses problems.

The fix, IMO, should be that ListItem.Empty should not inherit AbstractButton.

review: Disapprove
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Zsombor Egri (zsombi) wrote :

I must agree with saviq. But why an abstract button's clicked() signal must bring a mouse event with it? It is a component, the signal is emitted when the mouse click happens on button area, and I guess noone really cares the details of the click. If they really do, they can connect to __mouseArea to get that.

Revision history for this message
Tim Peeters (tpeeters) wrote :

Removing the var mouse from the clicked events seems to work fine everywhere and it removes the error messages. But still the TextArea forwards the key events to its parent, so they get eaten by the ListItem.Empty instead of added inside the TextArea.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Florian Boucault (fboucault) wrote :

I'm happy with that API change but it must be documented in CHANGES.

review: Needs Fixing
Revision history for this message
Florian Boucault (fboucault) wrote :

Note that the actual bug #1166840 is being fixed by Zsombor separately.

Revision history for this message
Zsombor Egri (zsombi) wrote :

The bug was taken away from this MR. Tim, please proceed with the MR.

Revision history for this message
Tim Peeters (tpeeters) wrote :

Is there still a good reason to remove the mouse parameter?

review: Needs Information
421. By Tim Peeters

Empty commit to figure out some launchpad weirdness

422. By Tim Peeters

remove mouse parameter in remaining clicked() and pressAndHold() calls

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Tim Peeters (tpeeters) wrote :

The mouse parameter for the signals of AbstractButton were removed (and CHANGES was updated), but the Keys are no longer touched in this MR.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'CHANGES'
--- CHANGES 2013-04-12 12:09:26 +0000
+++ CHANGES 2013-04-15 18:51:29 +0000
@@ -15,7 +15,8 @@
15API Changes15API Changes
16***********16***********
1717
18* None18* clicked() and pressAndHold() signals of AbstractButton no longer take a mouse
19 parameter as input.
1920
20SDK 0.1.4221SDK 0.1.42
21##########22##########
2223
=== modified file 'modules/Ubuntu/Components/AbstractButton.qml'
--- modules/Ubuntu/Components/AbstractButton.qml 2013-03-15 18:20:36 +0000
+++ modules/Ubuntu/Components/AbstractButton.qml 2013-04-15 18:51:29 +0000
@@ -35,7 +35,7 @@
35 This handler is called when there is a mouse click on the button35 This handler is called when there is a mouse click on the button
36 and the button is not disabled.36 and the button is not disabled.
37 */37 */
38 signal clicked(var mouse)38 signal clicked()
3939
40 Keys.onEnterPressed: clicked()40 Keys.onEnterPressed: clicked()
41 Keys.onReturnPressed: clicked()41 Keys.onReturnPressed: clicked()
@@ -44,7 +44,7 @@
44 \preliminary44 \preliminary
45 This handler is called when there is a long press.45 This handler is called when there is a long press.
46 */46 */
47 signal pressAndHold(var mouse)47 signal pressAndHold()
4848
49 /*!49 /*!
50 \preliminary50 \preliminary
@@ -80,12 +80,12 @@
8080
81 onClicked: {81 onClicked: {
82 if (button.__acceptEvents) {82 if (button.__acceptEvents) {
83 button.clicked(mouse)83 button.clicked()
84 }84 }
85 }85 }
86 onPressAndHold: {86 onPressAndHold: {
87 if (button.__acceptEvents) {87 if (button.__acceptEvents) {
88 button.pressAndHold(mouse)88 button.pressAndHold()
89 }89 }
90 }90 }
91 }91 }
9292
=== modified file 'modules/Ubuntu/Components/ListItems/SingleControl.qml'
--- modules/Ubuntu/Components/ListItems/SingleControl.qml 2013-03-20 19:15:15 +0000
+++ modules/Ubuntu/Components/ListItems/SingleControl.qml 2013-04-15 18:51:29 +0000
@@ -55,7 +55,7 @@
55 property AbstractButton control55 property AbstractButton control
5656
57 /*! \internal */57 /*! \internal */
58 onClicked: control.clicked(mouse)58 onClicked: control.clicked()
59 pressed: __mouseArea.pressed || control.__mouseArea.pressed59 pressed: __mouseArea.pressed || control.__mouseArea.pressed
60 /*! \internal */60 /*! \internal */
61 onPressedChanged: control.pressed = singleControlListItem.pressed61 onPressedChanged: control.pressed = singleControlListItem.pressed
6262
=== modified file 'modules/Ubuntu/Components/ListItems/Standard.qml'
--- modules/Ubuntu/Components/ListItems/Standard.qml 2013-03-20 18:35:06 +0000
+++ modules/Ubuntu/Components/ListItems/Standard.qml 2013-04-15 18:51:29 +0000
@@ -249,21 +249,17 @@
249249
250 onClicked: {250 onClicked: {
251 if (control && __mouseArea.mouseX < progressionHelper.x) {251 if (control && __mouseArea.mouseX < progressionHelper.x) {
252 control.clicked(mouse)252 control.clicked();
253 mouse.accepted = true
254 } else {253 } else {
255 listItem.clicked(mouse)254 listItem.clicked();
256 mouse.accepted = true
257 }255 }
258 }256 }
259257
260 onPressAndHold: {258 onPressAndHold: {
261 if (control && __mouseArea.mouseX < progressionHelper.x) {259 if (control && __mouseArea.mouseX < progressionHelper.x) {
262 control.pressAndHold(mouse)260 control.pressAndHold();
263 mouse.accepted = true
264 } else {261 } else {
265 listItem.pressAndHold(mouse)262 listItem.pressAndHold();
266 mouse.accepted = true
267 }263 }
268 }264 }
269 }265 }

Subscribers

People subscribed via source and target branches

to status/vote changes: