Merge lp:~brandontschaefer/unity/alt-grave-ordering-fix into lp:unity

Proposed by Brandon Schaefer on 2012-10-09
Status: Merged
Approved by: Marco Trevisan (Treviño) on 2012-10-11
Approved revision: 2819
Merged at revision: 2827
Proposed branch: lp:~brandontschaefer/unity/alt-grave-ordering-fix
Merge into: lp:unity
Diff against target: 85 lines (+52/-2)
3 files modified
launcher/SwitcherModel.cpp (+6/-2)
tests/autopilot/unity/tests/test_switcher.py (+13/-0)
tests/test_switcher_model.cpp (+33/-0)
To merge this branch: bzr merge lp:~brandontschaefer/unity/alt-grave-ordering-fix
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve on 2012-10-10
Thomi Richards (community) quality 2012-10-09 Approve on 2012-10-09
jenkins continuous-integration Pending
Review via email: mp+128823@code.launchpad.net

Commit message

Changes the swapping order of the detail mode. Now the currently active window gets moved to the end, instead of getting swapped with the last used window.

Description of the change

=== Problem ===
When pressing Alt+grave twice it would focus the active window going into detail mode. It should focus the third window, not the first.

=== Fix ===
Instead of swapping the first and second window, move the first all the way to the end (it being a vector means moving everything over one left)

=== Test ===
AP test

So with this change:

Window order, where 1 is the current focused window:
1,2,3,4,5,6

Pressing Alt+`

Desired result order:
2,3,4,5,6,1

Current result order:
2,1,3,4,5,6

To post a comment you must log in.
Brandon Schaefer (brandontschaefer) wrote :

Running:

autopilot run unity.tests.test_switcher

Ran 54 tests in 461.457s
OK

2818. By Brandon Schaefer on 2012-10-09

* Better assert

review: Approve (quality)
Marco Trevisan (Treviño) (3v1n0) wrote :

Good!

review: Approve
2819. By Brandon Schaefer on 2012-10-10

* Added a unit test

Marco Trevisan (Treviño) (3v1n0) wrote :

Cool thanks for the unit test... Globally approve!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'launcher/SwitcherModel.cpp'
2--- launcher/SwitcherModel.cpp 2012-10-04 07:10:39 +0000
3+++ launcher/SwitcherModel.cpp 2012-10-10 21:23:21 +0000
4@@ -162,9 +162,13 @@
5
6 std::sort(results.begin(), results.end(), compare_windows_by_active);
7
8- // swap so we focus the last focused window first
9 if (Selection() == _last_active_icon && results.size () > 1)
10- std::swap (results[0], results[1]);
11+ {
12+ for (unsigned int i = 0; i < results.size()-1; i++)
13+ {
14+ std::swap (results[i], results[i+1]);
15+ }
16+ }
17
18 return results;
19 }
20
21=== modified file 'tests/autopilot/unity/tests/test_switcher.py'
22--- tests/autopilot/unity/tests/test_switcher.py 2012-09-17 10:37:02 +0000
23+++ tests/autopilot/unity/tests/test_switcher.py 2012-10-10 21:23:21 +0000
24@@ -350,6 +350,19 @@
25
26 self.assertProperty(char_win1, is_focused=True)
27
28+ def test_detail_mode_selects_third_window(self):
29+ """Pressing Alt+` twice must select the third last used window.
30+ LP:1061229
31+ """
32+ char_win1, char_win2, char_win3 = self.start_applications("Character Map", "Character Map", "Character Map")
33+ self.assertVisibleWindowStack([char_win3, char_win2, char_win1])
34+
35+ self.switcher.initiate(SwitcherMode.DETAIL)
36+ self.switcher.next_detail()
37+
38+ self.switcher.select()
39+ self.assertVisibleWindowStack([char_win1, char_win3, char_win2])
40+
41
42 class SwitcherWorkspaceTests(SwitcherTestCase):
43 """Test Switcher behavior with respect to multiple workspaces."""
44
45=== modified file 'tests/test_switcher_model.cpp'
46--- tests/test_switcher_model.cpp 2012-03-14 06:24:18 +0000
47+++ tests/test_switcher_model.cpp 2012-10-10 21:23:21 +0000
48@@ -100,4 +100,37 @@
49 EXPECT_EQ(model->LastSelection(), third);
50 }
51
52+TEST(TestSwitcherModel, TestActiveDetailWindowSort)
53+{
54+ std::vector<AbstractLauncherIcon::Ptr> detail_icons;
55+ AbstractLauncherIcon::Ptr detail(new MockLauncherIcon());
56+ detail->SetQuirk(AbstractLauncherIcon::Quirk::ACTIVE, true);
57+ detail_icons.push_back(detail);
58+
59+ // Set up a list with out an active icon, so we can assert
60+ // the first detail icon == to the last xid of detailed list
61+ std::vector<AbstractLauncherIcon::Ptr> icons;
62+ AbstractLauncherIcon::Ptr normal(new MockLauncherIcon());
63+ icons.push_back(normal);
64+
65+ SwitcherModel::Ptr model_detail_active(new SwitcherModel(detail_icons));
66+ model_detail_active->detail_selection = true;
67+
68+ SwitcherModel::Ptr model_detail(new SwitcherModel(icons));
69+ model_detail->detail_selection = true;
70+
71+ EXPECT_TRUE(model_detail_active->DetailXids().size() > 2);
72+ EXPECT_TRUE(model_detail_active->DetailSelectionWindow() != model_detail->DetailSelectionWindow());
73+
74+ // Move to the last detailed window
75+ for (unsigned int i = 0; i < model_detail_active->DetailXids().size() - 1; i++)
76+ model_detail_active->NextDetail();
77+
78+ Window sorted, unsorted;
79+ sorted = model_detail_active->DetailSelectionWindow();
80+ unsorted = model_detail->DetailSelectionWindow();
81+
82+ EXPECT_EQ(sorted, unsorted);
83+}
84+
85 }