Merge lp:~uriboni/unity-2d/launcher-padding-fully-inside-items into lp:unity-2d/3.0

Proposed by Ugo Riboni
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 436
Merged at revision: 445
Proposed branch: lp:~uriboni/unity-2d/launcher-padding-fully-inside-items
Merge into: lp:unity-2d/3.0
Diff against target: 0 lines
To merge this branch: bzr merge lp:~uriboni/unity-2d/launcher-padding-fully-inside-items
Reviewer Review Type Date Requested Status
Olivier Tilloy (community) code Approve
Review via email: mp+52588@code.launchpad.net

Commit message

[launcher] Implement padding in the list entirely with padding inside the items

Description of the change

The changes in this MR basically remove the padding at top and bottom of the AutoscrollingListView, which were implemented by using an appropriately sized header and footer.

Then I put back this padding by simply using the already existing padding inside the items and just aligning the tile to the bottom of the item instead of the middle.

Everything works as before, but there's one regression. When there are more items than fit the screen and the launcher is scrolled all the way to the bottom, there's a difference in the spacing between the last item in the launcher and the first item in the shelf, as illustrated here: http://imgur.com/a/MGftH

Florian saw this and deemed the regression acceptable, especially since he may remove the shelf entirely when the accordion effect is implemented.

To post a comment you must log in.
Revision history for this message
Olivier Tilloy (osomon) wrote :

The tooltips and active arrows are not vertically aligned. Not sure which one is shifted (maybe both actually).

review: Needs Fixing (functional)
Revision history for this message
Ugo Riboni (uriboni) wrote :

They were both wrongly aligned. Should be fixed now.

432. By Ugo Riboni

Make sure the pips and the arrow are correctly centered in the tile.

Revision history for this message
Olivier Tilloy (osomon) wrote :

So the pips are now vertically centred, but the tooltips (a.k.a. quicklists) are still shifted down by a handful of pixels. Can you please fix?

review: Needs Fixing (functional)
433. By Ugo Riboni

Merge changes from trunk

434. By Ugo Riboni

Use a different logic to always center the menu arrow into with the entire menu and with the tile's active arrow

435. By Ugo Riboni

Revert the previous commit (it had issues I couldn't solve) and just adjust the magic numbers so the menu is centered.

Revision history for this message
Ugo Riboni (uriboni) wrote :

Centering of menu should be fixed too.

Revision history for this message
Olivier Tilloy (osomon) wrote :

Functional review is satisfactory: the two related bugs are fixed indeed, and no regressions but the acceptable one mentioned are to be observed.

I’ll now proceed to the code review.

review: Approve (functional)
Revision history for this message
Olivier Tilloy (osomon) wrote :

In launcher/AutoScrollingListView.qml:

+ contentY: 0

I don’t think you need to explicitly specify the value of contentY if it’s 0 (which should be the default value). Not tested, but I’m pretty sure this can be safely removed.

436. By Ugo Riboni

Remove some useless code

Revision history for this message
Olivier Tilloy (osomon) wrote :

Good to go!

review: Approve (code)

Preview Diff

Empty

Subscribers

People subscribed via source and target branches

to all changes: