Unity

Merge lp://staging/~brandontschaefer/unity/lp.1131646-fix into lp://staging/unity

lp.1131646-fix
Merge into trunk

Proposed by Brandon Schaefer on 2013-06-14

Status:

Merged

Approved by:

Marco Trevisan (Treviño) on 2013-06-18

Approved revision:

no longer in the source branch.

Merged at revision:

3378

Proposed branch:

lp://staging/~brandontschaefer/unity/lp.1131646-fix

Merge into:

lp://staging/unity

Diff against target:

734 lines (+484/-24)

13 files modified

launcher/SwitcherController.cpp (+72/-0)
launcher/SwitcherController.h (+3/-0)
launcher/SwitcherControllerImpl.h (+5/-0)
launcher/SwitcherModel.cpp (+97/-0)
launcher/SwitcherModel.h (+13/-0)
launcher/SwitcherView.cpp (+1/-0)
plugins/unityshell/src/unityshell.cpp (+6/-18)
plugins/unityshell/src/unityshell.h (+2/-2)
tests/test_layout_system.cpp (+48/-0)
tests/test_switcher_controller.cpp (+71/-0)
tests/test_switcher_model.cpp (+150/-0)
unity-shared/LayoutSystem.cpp (+13/-2)
unity-shared/LayoutSystem.h (+3/-2)

To merge this branch:

bzr merge lp://staging/~brandontschaefer/unity/lp.1131646-fix

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Approve on 2013-06-18
Marco Trevisan (Treviño)		2013-06-14	Approve on 2013-06-18
Review via email: mp+169475@code.staging.launchpad.net

Commit message

When in detail mode, you are now allowed to use UP/DOWN keys to move around.

Description of the change

When in detail mode, you are now allowed to use UP/DOWN keys to move around.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-06-14:

PASSED: Continuous integration, rev:3368
http://jenkins.qa.ubuntu.com/job/unity-ci/134/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/22

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/134/rebuild

review: Approve (continuous-integration)

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2013-06-17:

Nice work, overall it looks fine, just few things:

143 - if (detail_selection_index >= (unsigned int) 1)
232 + return (detail_selection_index > 0);

doesn't compile on raring g++... Probably we should wait a little.

478 + std::vector<LayoutWindow::Vector> rows = GetRows(windows, max_bounds);
235 +void SwitcherModel::SetRowSizes(std::vector<int> row_sizes)

Use const& please.

174 + for (unsigned int i = 0; i <= n; i++)

++i is nicer. Also you should make sure that n is < row_sizes_.size().

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-06-18:

PASSED: Continuous integration, rev:3372
http://jenkins.qa.ubuntu.com/job/unity-ci/140/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/28
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/28
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/28

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/140/rebuild

review: Approve (continuous-integration)

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2013-06-18:

+bool Controller::HandleStartInitiateEvent()
+bool Controller::HandleStopInitiateEvent()

What about using different naming? Such {Start,Stop}DetailMode or better?

Probably it would be better to change the compiz option names also, since they generates functions that doesn't represent anymore what they say. Don't you agree?

Would be now possible to test the controller also?

133 + void NextDetailRow();
134 + void PrevDetailRow();
135 + bool HasNextDetailRow() const;
136 + bool HasPrevDetailRow() const;

Are probably not needed anymore, right (also in the impl)?

Revision history for this message

Brandon Schaefer (brandontschaefer) wrote on 2013-06-18:

Yeeah, but it still starts/stops detail mode. Ill try to think of a different name :). Yes, I don't have to expose those function publicily anymore, and yes it will be possible to get tests on that 2 new function. What I was going to do this morning :)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-06-18:

PASSED: Continuous integration, rev:3373
http://jenkins.qa.ubuntu.com/job/unity-ci/142/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/30
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/30
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/30