Merge lp://qastaging/~mardy/unity-2d/treat-double-clicks-as-single into lp://qastaging/unity-2d/3.0

Proposed by Alberto Mardegan
Status: Merged
Approved by: Florian Boucault
Approved revision: 580
Merged at revision: 577
Proposed branch: lp://qastaging/~mardy/unity-2d/treat-double-clicks-as-single
Merge into: lp://qastaging/unity-2d/3.0
Diff against target: 233 lines (+134/-27)
6 files modified
launcher/ListViewDragAndDrop.qml (+26/-9)
panel/applets/CMakeLists.txt (+1/-0)
panel/applets/homebutton/homebutton.cpp (+61/-0)
panel/applets/homebutton/homebutton.h (+43/-0)
panel/applets/homebutton/homebuttonapplet.cpp (+1/-14)
panel/applets/homebutton/homebuttonapplet.h (+2/-4)
To merge this branch: bzr merge lp://qastaging/~mardy/unity-2d/treat-double-clicks-as-single
Reviewer Review Type Date Requested Status
Florian Boucault (community) Approve
Review via email: mp+62344@code.qastaging.launchpad.net

Commit message

[panel & launcher] Consider double clicks as single clicks for launcher items and the home button of the panel (bfb).

For the panel created HomeButton class: specialized subclass of QToolButton, handling double clicks as single clicks.
For the launcher, ignored DoubleClicked signal and added a workaround that separates click resulting from the end of a drag and drop from regular clicks.

Description of the change

Proposed fix for bug 766776.

For the BFB, use a QTime instance to track the elapsed time since the last click. I initially tried hacking around a reimplementation of the virtual mouseDoubleClickEvent(), just to find out that it's never emitted because QAbstractButton is reimplementing the handling of mouse events in order to suppress it.

For the launcher, I wasted some time in figuring out that the onClicked() slot in the MouseArea in LauncherItem.qml is never emitted (should we add a comment next to it, to avoid someone else to spend time in figuring out that as well?): this is because the LauncherItem is part of a bigger MouseArea (the one in ListViewDragAndDrop.qml) which eats all the mouse events.
I initially tried to set the mouse.accepted property to false in the outer MouseArea, and I could fix the double-click problem in LauncherItem very nicely. But this broke the DnD, because after setting the mouse.accepted property to false, the ListViewDragAndDrop would not be notified for the next mouse move and release events.
So, though quite ugly, the ignoreNextClick flag is the only workaround I could think of. QtQuick 2.0 will probably help in solving this better [http://bugreports.qt.nokia.com/browse/QTBUG-13007].

To post a comment you must log in.
572. By Didier Roche-Tolomelli

replace the generating pot target with the generic make unity-2d.pot one

573. By Florian Boucault

Added compilation flags to help detecting use of deprecated GTK APIs and direct access to object fields.
See:
- https://live.gnome.org/GnomeGoals/RemoveDeprecatedSymbols/GTK+
- https://live.gnome.org/GnomeGoals/UseGseal

574. By Didier Roche-Tolomelli

[packaging]
 * debian/control:
   - remove deprecated transitional packages (only from maverick ppa or early
     natty alpha)
   - don't dep on gnome-session-bin
   - remove the -dev package, uneeded for a private library internal to this
     source
   - remove bzr build-dep, we will refresh the pot with dh_translations
 * debian/rules:
   - build with dh7 now
   - buid with --fail-missing and purging installed but not shipped private
     headers
 * debian/unity-2d-launcher.preinst:
   - removed, deprecated as it was for the ppa migration
 * debian/libunity-2d-private0.post*
   - removed: ldconfig is generated directly by dh_makeshlibs as we don't pass
     -n anymore

575. By Didier Roche-Tolomelli

update changelog

576. By Didier Roche-Tolomelli

releasing version 3.8.6-0ubuntu1

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

Functionally, the intended behaviour of ignoring double clicks works fine both in the launcher and for the home button.
However the home button exhibits an issue. Its visual state is not always synchronised with the state of the dash: sometimes the dash is shown but the button is visually not pressed. Steps to reproduce:
1. launch the panel
2. click on the home button
3. wait just enough time for the dash to appear but less than the doubleClickInterval (looks like around 500 ms here)
4. click again on the home button

Result: the home button is visually unpressed but the dash stays.
Debugging a little shows that the second clicked is caught and properly ignored. However the checked state of the button is toggled. This behaviour is internal to QAbstractButton and should be overridable by reimplementing QAbstractButton::nextCheckState.

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

The technique used to ignore double clicks on the home button seems to be the simplest one around. The code looks good.

Moving on to reviewing the code for ignoring double clicks on the launcher.

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

The ignoreNextClick workaround is fine and works reliably. Though its purpose was not obvious at first. A small explanation in the code saying that it is used to ignore clicks that happen when a drag has just finished would be welcome.

Everything else is great!

review: Needs Fixing
579. By Alberto Mardegan

[launcher] Add explanatory comment for ignoreNextClick flag

580. By Alberto Mardegan

[panel] Create HomeButton class: specialized subclass of QToolButton,
handling double clicks as single clicks.

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

Excellent job. Functionally and code wise.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches