Merge lp:~stolowski/unity/no-click-preview-for-apps into lp:unity

Proposed by Paweł Stołowski on 2013-07-03
Status: Merged
Approved by: Christopher Townsend on 2013-07-08
Approved revision: 3400
Merged at revision: 3412
Proposed branch: lp:~stolowski/unity/no-click-preview-for-apps
Merge into: lp:unity
Diff against target: 135 lines (+68/-3)
4 files modified
dash/ResultView.h (+1/-0)
dash/ResultViewGrid.cpp (+1/-1)
dash/ScopeView.cpp (+10/-1)
tests/autopilot/unity/tests/test_dash.py (+56/-1)
To merge this branch: bzr merge lp:~stolowski/unity/no-click-preview-for-apps
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2013-07-08
Brandon Schaefer (community) 2013-07-03 Approve on 2013-07-05
Unity Team 2013-07-03 Pending
Review via email: mp+172787@code.launchpad.net

Commit message

Activate (installed) applications on single left-click in Home and Applications scopes view.

Description of the change

Activate (installed) applications on single left-click in Home and Applications scopes view. Applications which are not installed will still be previewed first (this is handled on Application scope side which has the final say about how to handle activation request).

Note: we had a discussion in scopes team whether to expose an API to make it full controllable by scope developers, and decided not to do so, as Apps are the only exception for the default behavior and we want more control over this behavior.

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Brandon Schaefer (brandontschaefer) wrote :

111 + scope = self.unity.dash.view.get_scope view_by_name("home.scope")

This is causing some error for me while trying to run the test_dash.py AP tests.

Failed to import test module: unity.tests.test_dash
Traceback (most recent call last):
  File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests
    module = self._get_module_from_name(name)
  File "/usr/lib/python2.7/unittest/loader.py", line 230, in _get_module_from_name
    __import__(name)
  File "/home/bschaefer/src/tmp-unity/tests/autopilot/unity/tests/test_dash.py", line 837
    scope = self.unity.dash.view.get_scope view_by_name("home.scope")
                                                       ^
SyntaxError: invalid syntax

    unittest.loader.ModuleImportFailure.unity.tests.test_dash

review: Needs Fixing
Brandon Schaefer (brandontschaefer) wrote :

But other then the tests, this works like a charm :)

Paweł Stołowski (stolowski) wrote :

> But other then the tests, this works like a charm :)

Ah, seems I accidentally pressed space before commiting... Thanks for catching, updated.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Brandon Schaefer (brandontschaefer) wrote :

Nice.

Looks good to me, tests working, and AP tests :)

review: Approve
Christopher Townsend (townsend) wrote :

Re-approving due to some weird jenkins publickey failures.

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'dash/ResultView.h'
2--- dash/ResultView.h 2013-07-01 21:19:33 +0000
3+++ dash/ResultView.h 2013-07-04 08:04:42 +0000
4@@ -61,6 +61,7 @@
5 typedef enum ActivateType_
6 {
7 DIRECT,
8+ PREVIEW_LEFT_BUTTON,
9 PREVIEW
10 } ActivateType;
11
12
13=== modified file 'dash/ResultViewGrid.cpp'
14--- dash/ResultViewGrid.cpp 2013-05-13 19:24:26 +0000
15+++ dash/ResultViewGrid.cpp 2013-07-04 08:04:42 +0000
16@@ -812,7 +812,7 @@
17 {
18 // delay activate for single left click. (for double click check)
19 activate_timer_.reset(new glib::Timeout(DOUBLE_CLICK_SPEED, [this, index]() {
20- Activate(activated_result_, index, ResultView::ActivateType::PREVIEW);
21+ Activate(activated_result_, index, ResultView::ActivateType::PREVIEW_LEFT_BUTTON);
22 return false;
23 }));
24 }
25
26=== modified file 'dash/ScopeView.cpp'
27--- dash/ScopeView.cpp 2013-07-04 06:35:46 +0000
28+++ dash/ScopeView.cpp 2013-07-04 08:04:42 +0000
29@@ -474,15 +474,23 @@
30
31 if (scope_)
32 {
33+ const std::string category_id = category.id();
34 std::string unique_id = category.name() + scope_->name();
35 results_view->unique_id = unique_id;
36 results_view->expanded = false;
37
38- results_view->ResultActivated.connect([this, unique_id] (LocalResult const& local_result, ResultView::ActivateType type, GVariant* data)
39+ results_view->ResultActivated.connect([this, unique_id, category_id] (LocalResult const& local_result, ResultView::ActivateType type, GVariant* data)
40 {
41 if (g_str_has_prefix(local_result.uri.c_str(), "x-unity-no-preview"))
42 type = ResultView::ActivateType::DIRECT;
43
44+ // Applications scope results should be activated on left-click (instead of preview). Note that app scope can still
45+ // respond with preview for activation request (the case for uninstalled apps).
46+ bool is_app_scope_result = (scope_->id() == "applications.scope" || (scope_->id() == "home.scope" && category_id == "applications.scope"));
47+
48+ if (is_app_scope_result && type == ResultView::ActivateType::PREVIEW_LEFT_BUTTON)
49+ type = ResultView::ActivateType::DIRECT;
50+
51 result_activated.emit(type, local_result, data, unique_id);
52 switch (type)
53 {
54@@ -490,6 +498,7 @@
55 {
56 scope_->Activate(local_result, nullptr, cancellable_);
57 } break;
58+ case ResultView::ActivateType::PREVIEW_LEFT_BUTTON:
59 case ResultView::ActivateType::PREVIEW:
60 {
61 scope_->Preview(local_result, nullptr, cancellable_);
62
63=== modified file 'tests/autopilot/unity/tests/test_dash.py'
64--- tests/autopilot/unity/tests/test_dash.py 2013-06-14 16:30:01 +0000
65+++ tests/autopilot/unity/tests/test_dash.py 2013-07-04 08:04:42 +0000
66@@ -794,13 +794,68 @@
67
68 result = results[0]
69 # result.preview handles finding xy co-ords and right mouse-click
70- result.preview()
71+ result.preview(button=2)
72 self.assertThat(self.unity.dash.preview_displaying, Eventually(Equals(True)))
73
74 self.keyboard.press_and_release("Escape")
75
76 self.assertThat(self.unity.dash.preview_displaying, Eventually(Equals(False)))
77
78+ def test_app_scope_lmb_installed_app(self):
79+ """Left-clicking on an application scope result in 'Installed' category
80+ must activate it.
81+ """
82+ gettext.install("unity-scope-applications", unicode=True)
83+ scope = self.unity.dash.reveal_application_scope()
84+
85+ self.addCleanup(self.process_manager.close_all_app, "Character Map")
86+ self.addCleanup(self.unity.dash.ensure_hidden)
87+
88+ self.keyboard.type("Character")
89+ # wait for "Installed" category
90+ category = self.wait_for_category(scope, _("Installed"))
91+
92+ # wait for some results
93+ self.assertThat(lambda: len(category.get_results()), Eventually(GreaterThan(0), timeout=20))
94+ results = category.get_results()
95+
96+ result = results[0]
97+ # result.preview handles finding xy co-ords and right mouse-click
98+ result.activate(double_click=False)
99+
100+ # make sure it started
101+ self.assertThat(lambda: len(self.process_manager.get_open_windows_by_application("Character Map")), Eventually(Equals(1), timeout=10))
102+
103+ self.assertThat(self.unity.dash.preview_displaying, Eventually(Equals(False)))
104+
105+ def test_home_scope_lmb_app(self):
106+ """Left-clicking on an application scope result in 'Application' category of Home
107+ must activate it.
108+ """
109+ gettext.install("unity-scope-applications", unicode=True)
110+ self.unity.dash.ensure_visible()
111+ scope = self.unity.dash.view.get_scopeview_by_name("home.scope")
112+
113+ self.addCleanup(self.process_manager.close_all_app, "Character Map")
114+ self.addCleanup(self.unity.dash.ensure_hidden)
115+
116+ self.keyboard.type("Character Map")
117+ # wait for "Applications" category
118+ category = self.wait_for_category(scope, _("Applications"))
119+
120+ # wait for some results
121+ self.assertThat(lambda: len(category.get_results()), Eventually(GreaterThan(0), timeout=20))
122+ results = category.get_results()
123+
124+ result = results[0]
125+ # result.preview handles finding xy co-ords and right mouse-click
126+ result.activate(double_click=False)
127+
128+ # make sure it started
129+ self.assertThat(lambda: len(self.process_manager.get_open_windows_by_application("Character Map")), Eventually(Equals(1), timeout=10))
130+
131+ self.assertThat(self.unity.dash.preview_displaying, Eventually(Equals(False)))
132+
133 def test_files_scope_preview_open_close(self):
134 """Right-clicking on a files scope result must show its
135 preview.