Merge lp://qastaging/~uriboni/unity-2d/launcher-autoscroll into lp://qastaging/unity-2d/3.0

Proposed by Florian Boucault
Status: Merged
Approved by: Florian Boucault
Approved revision: 396
Merged at revision: 392
Proposed branch: lp://qastaging/~uriboni/unity-2d/launcher-autoscroll
Merge into: lp://qastaging/unity-2d/3.0
Diff against target: 198 lines (+146/-5)
2 files modified
launcher/AutoScrollingListView.qml (+132/-0)
launcher/Launcher.qml (+14/-5)
To merge this branch: bzr merge lp://qastaging/~uriboni/unity-2d/launcher-autoscroll
Reviewer Review Type Date Requested Status
Florian Boucault (community) Approve
Review via email: mp+49990@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2011-02-14.

Description of the change

[launcher] Adds an autoscrolling feature to the launcher. It works by having two sensitive zones at the top and bottom of the launcher. When the mouse enters these zones, the laucher will scroll up or down if needed, until the mouse leaves the zone.

To post a comment you must log in.
Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

* First few pixels at the top are not triggering the autoscroll.
* Autoscrolling is active even though there is not enough content to fill the ListView (for example if you have around 5-8 applications in a medium resolution monitor, the trash will not be at the bottom though if you stick the mouse cursor at the bottom of the launcher, the autoscrolling is triggered).

review: Needs Fixing (functional)
Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

HACK about the mouse coordinates should be FIXME.

review: Needs Fixing
Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

"Implements a ListView that have two mouse sensitive areas"
should read
"ListView that has two mouse sensitive areas"

review: Needs Fixing
Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

After testing I think it would be better to have 'autoScrollSize' to be itemHeight / 2 instead of itemHeight / 3

review: Needs Fixing
Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

Sometimes the following WARNING is issued in the console:

AutoScrollingListView.qml:105 Insufficient Arguments

Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

While the workaround for the QML bug is essentially fine, it would have been simpler to write using property binding.

We do not have time to simplify it now, so let's keep it as is.

Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

Why do you need an extra property 'spacingSize'? Using 'spacing' is enough.

review: Needs Fixing
Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

"""
"Implements a ListView that have two mouse sensitive areas"
should read
"ListView that has two mouse sensitive areas"
"""

Quoting my previous comment here. You will notice that not only have I replaced the 'have' with 'has' but also I removed the 'Implements a'. It's a new class, we *know* it implements something.

review: Needs Fixing
Revision history for this message
Ugo Riboni (uriboni) wrote : Posted in a previous version of this proposal

Fixed the last remarks and made sure the autoscroll happens only when it needs to.
Also have a look at the padding stuff in commit 394, which to me seems a quite clean way of doing it, but comments always welcome.

Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

Good to go!

review: Approve
Revision history for this message
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal

No proposals found for merge of lp:~unity-2d-team/unity-2d/launcher-layout into lp:unity-2d.

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

Good to go, again.

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

to all changes: