Unity

manual-tests/SuperTab.txt (+33/-0)
plugins/unityshell/src/Launcher.cpp (+36/-7)
plugins/unityshell/src/Launcher.h (+1/-0)
plugins/unityshell/src/unityshell.cpp (+54/-3)
plugins/unityshell/src/unityshell.h (+2/-0)

To merge this branch:

bzr merge lp://qastaging/~3v1n0/unity/super-tab-improvements

Related bugs:

Bug #891620: Add SUPER+TAB switching mode that enables the user to switch applications via the Launcher	Medium	Fix Released
Bug #919018: SUPER+TAB switcher should be circular	Medium	Fix Released
Bug #919019: It should be possible to escape from SUPER+TAB with Escape key	Low	Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Sam Spilsbury (community)		2012-01-20	Approve on 2012-01-20
Review via email: mp+89371@code.qastaging.launchpad.net

Description of the change

Improved the Super+Tab switcher making possible to escape from it using the Esc key and making the selection circular.

This branch mostly fixes bug #919018 and bug #919019 as requested by design on recent updates of the main bug #891620

Revision history for this message

Sam Spilsbury (smspillaz) wrote on 2012-01-20:

49 - if ((*it)->GetCenter().y + - _icon_size/ 2 < GetGeometry().y)
50 + if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
51 + {

That should test for > 0. In the diff it initially looked like you were changing a + to a - :)

52 _launcher_drag_delta += (_icon_size + _space_between_icons);
53 + }
54 + else if ((*it)->GetCenter().y + _icon_size / 2 > GetGeometry().height)
55 + {
56 + _launcher_drag_delta -= (*it)->GetCenter().y + _icon_size/2 +
57 + _space_between_icons - GetGeometry().height;
58 + }
59 }

Other than that. Approve

review: Approve

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-01-20:

> 49 - if ((*it)->GetCenter().y + - _icon_size/ 2 < GetGeometry().y)
> 50 + if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
> 51 + {
>
> That should test for > 0. In the diff it initially looked like you were
> changing a + to a - :)

What do you mean? You'd prefer to move GetGeometry().y to the left side?
The old code contained a copy&paste typo because there was a "+ -" operator that is just a "-" in fact.

Revision history for this message

Sam Spilsbury (smspillaz) wrote on 2012-01-21:

> > 49 - if ((*it)->GetCenter().y + - _icon_size/ 2 < GetGeometry().y)
> > 50 + if ((*it)->GetCenter().y - _icon_size / 2 < GetGeometry().y)
> > 51 + {
> >
> > That should test for > 0. In the diff it initially looked like you were
> > changing a + to a - :)
>
> What do you mean? You'd prefer to move GetGeometry().y to the left side?
> The old code contained a copy&paste typo because there was a "+ -" operator
> that is just a "-" in fact.

Oh whoops, looks like I didn't see the <.

The typo threw me off :)

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-01-27:

This branch has been merged on lp:~unity-team/unity/unity.multi-launcher so let's close this review.