Merge lp:~nick-dedekind/unity/lp1043808.instant-preview-feedback into lp:unity
- lp1043808.instant-preview-feedback
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Gord Allott |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2783 |
Proposed branch: | lp:~nick-dedekind/unity/lp1043808.instant-preview-feedback |
Merge into: | lp:unity |
Diff against target: |
674 lines (+157/-147) 15 files modified
dash/CoverflowResultView.cpp (+25/-13) dash/CoverflowResultView.h (+3/-0) dash/DashController.cpp (+0/-1) dash/DashView.cpp (+37/-40) dash/DashView.h (+3/-4) dash/LensView.cpp (+15/-9) dash/LensView.h (+2/-2) dash/PreviewStateMachine.cpp (+1/-4) dash/ResultView.cpp (+1/-0) dash/ResultView.h (+3/-1) dash/ResultViewGrid.cpp (+39/-61) dash/ResultViewGrid.h (+2/-0) dash/previews/PreviewContainer.cpp (+9/-8) manual-tests/Preview.txt (+17/-0) unity-shared/UBusMessages.h (+0/-4) |
To merge this branch: | bzr merge lp:~nick-dedekind/unity/lp1043808.instant-preview-feedback |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gord Allott (community) | Approve | ||
Michal Hruby (community) | Needs Fixing | ||
Brandon Schaefer (community) | Approve | ||
Review via email: mp+125981@code.launchpad.net |
Commit message
Immediate feedback for dash preview activation.
Description of the change
Preview of dash results immediately open preview window, and show spinner if the preview result takes longer than 300ms to arrive. Preview state machine now keeps track of preview position between preview activations. Fixes problem with preview response lag causing current index corruption.
Michal Hruby (mhr3) wrote : | # |
66 + glib::Variant data(g_
407 + glib::Variant data(g_
The StealRef() here doesn't look right, g_variant_new returns a floating ref, so the Variant wrapper should sink it first (which it doesn't with StealRef).
Nick Dedekind (nick-dedekind) wrote : | # |
Fixed glib variant reference.
All types of result activation now include additional URI info so that lenses can activate previews from direct activations.
Nick Dedekind (nick-dedekind) wrote : | # |
Added manual test.
Gord Allott (gordallott) wrote : | # |
Code looks fine, we should probably rearchtect this system sometime though, its doing things it wasn't designed to do as it was built before we really knew what the design was.
Preview Diff
1 | === modified file 'dash/CoverflowResultView.cpp' |
2 | --- dash/CoverflowResultView.cpp 2012-09-18 11:55:33 +0000 |
3 | +++ dash/CoverflowResultView.cpp 2012-09-27 16:36:23 +0000 |
4 | @@ -18,6 +18,7 @@ |
5 | */ |
6 | |
7 | #include "CoverflowResultView.h" |
8 | +#include <UnityCore/Variant.h> |
9 | #include "unity-shared/IconLoader.h" |
10 | #include "unity-shared/IconTexture.h" |
11 | #include "unity-shared/DashStyle.h" |
12 | @@ -124,18 +125,18 @@ |
13 | |
14 | void CoverflowResultItem::Activate(int button) |
15 | { |
16 | + int index = Index(); |
17 | + int size = model_->Items().size(); |
18 | + |
19 | + glib::Variant data(g_variant_new("(iiii)", 0, 0, index, size - index)); |
20 | + |
21 | //Left and right click take you to previews. |
22 | if (button == 1 || button == 3) |
23 | - parent_->UriActivated.emit(result_.uri, ResultView::ActivateType::PREVIEW); |
24 | + parent_->Activate(result_.uri, index, ResultView::ActivateType::PREVIEW); |
25 | //Scroll click opens up music player. |
26 | else if (button == 2) |
27 | - parent_->UriActivated.emit(result_.uri, ResultView::ActivateType::DIRECT); |
28 | - |
29 | - int index = Index(); |
30 | - int size = model_->Items().size(); |
31 | - |
32 | - ubus_.SendMessage(UBUS_DASH_PREVIEW_INFO_PAYLOAD, |
33 | - g_variant_new("(iiii)", 0, 0, index, size - index)); |
34 | + parent_->Activate(result_.uri, index, ResultView::ActivateType::DIRECT); |
35 | + |
36 | } |
37 | |
38 | CoverflowResultView::Impl::Impl(CoverflowResultView *parent) |
39 | @@ -189,11 +190,8 @@ |
40 | |
41 | if (nav_mode) |
42 | { |
43 | - int left_results = current_index; |
44 | - int right_results = num_results ? (num_results - current_index) - 1 : 0; |
45 | - parent_->UriActivated.emit(GetUriForIndex(current_index), ActivateType::PREVIEW); |
46 | - ubus_.SendMessage(UBUS_DASH_PREVIEW_INFO_PAYLOAD, |
47 | - g_variant_new("(iiii)", 0, 0, left_results, right_results)); |
48 | + std::string uri = GetUriForIndex(current_index); |
49 | + parent_->Activate(uri, current_index, ActivateType::PREVIEW); |
50 | } |
51 | }); |
52 | } |
53 | @@ -309,5 +307,19 @@ |
54 | } |
55 | |
56 | |
57 | +void CoverflowResultView::Activate(std::string const& uri, int index, ResultView::ActivateType type) |
58 | +{ |
59 | + unsigned num_results = pimpl->coverflow_->model()->Items().size(); |
60 | + |
61 | + int left_results = index; |
62 | + int right_results = num_results ? (num_results - index) - 1 : 0; |
63 | + int row_y = GetRootGeometry().y; |
64 | + int row_height = renderer_->height; |
65 | + |
66 | + glib::Variant data(g_variant_new("(iiii)", row_y, row_height, left_results, right_results)); |
67 | + UriActivated.emit(uri, type, data); |
68 | +} |
69 | + |
70 | + |
71 | } |
72 | } |
73 | |
74 | === modified file 'dash/CoverflowResultView.h' |
75 | --- dash/CoverflowResultView.h 2012-08-20 19:28:46 +0000 |
76 | +++ dash/CoverflowResultView.h 2012-09-27 16:36:23 +0000 |
77 | @@ -39,12 +39,15 @@ |
78 | |
79 | virtual void AddResult(Result& result); |
80 | virtual void RemoveResult(Result& result); |
81 | + |
82 | + virtual void Activate(std::string const& uri, int index, ActivateType type); |
83 | |
84 | protected: |
85 | virtual void Draw(nux::GraphicsEngine& GfxContext, bool force_draw); |
86 | virtual void DrawContent(nux::GraphicsEngine& GfxContext, bool force_draw); |
87 | virtual long ComputeContentSize(); |
88 | |
89 | + |
90 | private: |
91 | struct Impl; |
92 | Impl* pimpl; |
93 | |
94 | === modified file 'dash/DashController.cpp' |
95 | --- dash/DashController.cpp 2012-09-20 10:18:18 +0000 |
96 | +++ dash/DashController.cpp 2012-09-27 16:36:23 +0000 |
97 | @@ -228,7 +228,6 @@ |
98 | if (check_monitor) |
99 | { |
100 | monitor_ = CLAMP(GetIdealMonitor(), 0, static_cast<int>(UScreen::GetDefault()->GetMonitors().size()-1)); |
101 | - printf("relayout on monitor:%d, monitor count:%d\n", monitor_, static_cast<int>(UScreen::GetDefault()->GetMonitors().size())); |
102 | } |
103 | |
104 | nux::Geometry geo = GetIdealWindowGeometry(); |
105 | |
106 | === modified file 'dash/DashView.cpp' |
107 | --- dash/DashView.cpp 2012-09-20 10:18:18 +0000 |
108 | +++ dash/DashView.cpp 2012-09-27 16:36:23 +0000 |
109 | @@ -165,6 +165,7 @@ |
110 | animation_.Start(); |
111 | } |
112 | |
113 | + preview_navigation_mode_ = previews::Navigation::NONE; |
114 | preview_displaying_ = false; |
115 | |
116 | // re-focus dash view component. |
117 | @@ -201,6 +202,39 @@ |
118 | } |
119 | } |
120 | |
121 | +void DashView::OnUriActivated(ResultView::ActivateType type, std::string const& uri, GVariant* data, std::string const& unique_id) |
122 | +{ |
123 | + last_activated_uri_ = uri; |
124 | + stored_activated_unique_id_ = unique_id; |
125 | + |
126 | + if (data) |
127 | + { |
128 | + // Update positioning information. |
129 | + int position = -1; |
130 | + int row_height = 0; |
131 | + int results_to_the_left = 0; |
132 | + int results_to_the_right = 0; |
133 | + g_variant_get(data, "(iiii)", &position, &row_height, &results_to_the_left, &results_to_the_right); |
134 | + preview_state_machine_.SetSplitPosition(SplitPosition::CONTENT_AREA, position); |
135 | + preview_state_machine_.left_results = results_to_the_left; |
136 | + preview_state_machine_.right_results = results_to_the_right; |
137 | + |
138 | + if (opening_row_y_ == -1) |
139 | + { |
140 | + // Update only when opening the previews |
141 | + opening_row_y_ = position; |
142 | + } |
143 | + opening_row_height_ = row_height; |
144 | + } |
145 | + |
146 | + // we want immediate preview reaction on first opening. |
147 | + if (type == ResultView::ActivateType::DIRECT && !preview_displaying_) |
148 | + { |
149 | + BuildPreview(Preview::Ptr(nullptr)); |
150 | + } |
151 | +} |
152 | + |
153 | + |
154 | void DashView::BuildPreview(Preview::Ptr model) |
155 | { |
156 | if (!preview_displaying_) |
157 | @@ -239,21 +273,19 @@ |
158 | |
159 | // connect to nav left/right signals to request nav left/right movement. |
160 | preview_container_->navigate_left.connect([&] () { |
161 | - preview_state_machine_.Reset(); |
162 | preview_navigation_mode_ = previews::Navigation::LEFT; |
163 | |
164 | // sends a message to all result views, sending the the uri of the current preview result |
165 | // and the unique id of the result view that should be handling the results |
166 | - ubus_manager_.SendMessage(UBUS_DASH_PREVIEW_NAVIGATION_REQUEST, g_variant_new("(iss)", -1, stored_preview_uri_identifier_.c_str(), stored_preview_unique_id_.c_str())); |
167 | + ubus_manager_.SendMessage(UBUS_DASH_PREVIEW_NAVIGATION_REQUEST, g_variant_new("(iss)", -1, last_activated_uri_.c_str(), stored_activated_unique_id_.c_str())); |
168 | }); |
169 | |
170 | preview_container_->navigate_right.connect([&] () { |
171 | - preview_state_machine_.Reset(); |
172 | preview_navigation_mode_ = previews::Navigation::RIGHT; |
173 | |
174 | // sends a message to all result views, sending the the uri of the current preview result |
175 | // and the unique id of the result view that should be handling the results |
176 | - ubus_manager_.SendMessage(UBUS_DASH_PREVIEW_NAVIGATION_REQUEST, g_variant_new("(iss)", 1, stored_preview_uri_identifier_.c_str(), stored_preview_unique_id_.c_str())); |
177 | + ubus_manager_.SendMessage(UBUS_DASH_PREVIEW_NAVIGATION_REQUEST, g_variant_new("(iss)", 1, last_activated_uri_.c_str(), stored_activated_unique_id_.c_str())); |
178 | }); |
179 | |
180 | preview_container_->request_close.connect([&] () { ClosePreview(); }); |
181 | @@ -372,11 +404,7 @@ |
182 | content_layout_->AddView(lenses_layout_, 1, nux::MINOR_POSITION_LEFT); |
183 | |
184 | home_view_ = new LensView(home_lens_, nullptr); |
185 | - home_view_->uri_preview_activated.connect([&] (std::string const& uri, std::string const& unique_id) |
186 | - { |
187 | - stored_preview_unique_id_ = unique_id; |
188 | - stored_preview_uri_identifier_ = uri; |
189 | - }); |
190 | + home_view_->uri_activated.connect(sigc::mem_fun(this, &DashView::OnUriActivated)); |
191 | |
192 | AddChild(home_view_); |
193 | active_lens_view_ = home_view_; |
194 | @@ -393,26 +421,6 @@ |
195 | { |
196 | ubus_manager_.RegisterInterest(UBUS_PLACE_ENTRY_ACTIVATE_REQUEST, |
197 | sigc::mem_fun(this, &DashView::OnActivateRequest)); |
198 | - |
199 | - ubus_manager_.RegisterInterest(UBUS_DASH_PREVIEW_INFO_PAYLOAD, [&] (GVariant *data) |
200 | - { |
201 | - int position = -1; |
202 | - int row_height = 0; |
203 | - int results_to_the_left = 0; |
204 | - int results_to_the_right = 0; |
205 | - g_variant_get(data, "(iiii)", &position, &row_height, &results_to_the_left, &results_to_the_right); |
206 | - preview_state_machine_.SetSplitPosition(SplitPosition::CONTENT_AREA, position); |
207 | - preview_state_machine_.left_results = results_to_the_left; |
208 | - preview_state_machine_.right_results = results_to_the_right; |
209 | - |
210 | - if (opening_row_y_ == -1) |
211 | - { |
212 | - // Update only when opening the previews |
213 | - opening_row_y_ = position; |
214 | - } |
215 | - opening_row_height_ = row_height; |
216 | - |
217 | - }); |
218 | } |
219 | |
220 | long DashView::PostLayoutManagement (long LayoutResult) |
221 | @@ -891,12 +899,6 @@ |
222 | AddChild(view); |
223 | view->SetVisible(false); |
224 | view->uri_activated.connect(sigc::mem_fun(this, &DashView::OnUriActivated)); |
225 | - view->uri_preview_activated.connect([&] (std::string const& uri, std::string const& unique_id) |
226 | - { |
227 | - LOG_DEBUG(logger) << "got unique id from preview activation: " << unique_id; |
228 | - stored_preview_unique_id_ = unique_id; |
229 | - stored_preview_uri_identifier_ = uri; |
230 | - }); |
231 | |
232 | lenses_layout_->AddView(view, 1); |
233 | lens_views_[lens->id] = view; |
234 | @@ -1016,11 +1018,6 @@ |
235 | } |
236 | } |
237 | |
238 | -void DashView::OnUriActivated(std::string const& uri) |
239 | -{ |
240 | - last_activated_uri_ = uri; |
241 | -} |
242 | - |
243 | void DashView::OnUriActivatedReply(std::string const& uri, HandledType type, Lens::Hints const&) |
244 | { |
245 | // We don't want to close the dash if there was another activation pending |
246 | |
247 | === modified file 'dash/DashView.h' |
248 | --- dash/DashView.h 2012-09-19 13:58:01 +0000 |
249 | +++ dash/DashView.h 2012-09-27 16:36:23 +0000 |
250 | @@ -93,7 +93,7 @@ |
251 | void DrawContent(nux::GraphicsEngine& gfx_context, bool force_draw); |
252 | virtual long PostLayoutManagement (long LayoutResult); |
253 | nux::Area* FindAreaUnderMouse(const nux::Point& mouse_position, nux::NuxEventType event_type); |
254 | - |
255 | + |
256 | void BuildPreview(Preview::Ptr model); |
257 | void OnMouseButtonDown(int x, int y, unsigned long button, unsigned long key); |
258 | void OnBackgroundColorChanged(GVariant* args); |
259 | @@ -104,7 +104,7 @@ |
260 | void OnSearchFinished(Lens::Hints const& hints); |
261 | void OnGlobalSearchFinished(Lens::Hints const& hints); |
262 | void OnAppsGlobalSearchFinished(Lens::Hints const& hints); |
263 | - void OnUriActivated(std::string const& uri); |
264 | + void OnUriActivated(ResultView::ActivateType type, std::string const& uri, GVariant* data, std::string const& unique_id); |
265 | void OnUriActivatedReply(std::string const& uri, HandledType type, Lens::Hints const&); |
266 | bool DoFallbackActivation(std::string const& uri); |
267 | bool LaunchApp(std::string const& appname); |
268 | @@ -130,8 +130,7 @@ |
269 | PreviewStateMachine preview_state_machine_; |
270 | previews::PreviewContainer::Ptr preview_container_; |
271 | bool preview_displaying_; |
272 | - std::string stored_preview_unique_id_; |
273 | - std::string stored_preview_uri_identifier_; |
274 | + std::string stored_activated_unique_id_; |
275 | dash::previews::Navigation preview_navigation_mode_; |
276 | |
277 | nux::VLayout* layout_; |
278 | |
279 | === modified file 'dash/LensView.cpp' |
280 | --- dash/LensView.cpp 2012-09-19 18:45:56 +0000 |
281 | +++ dash/LensView.cpp 2012-09-27 16:36:23 +0000 |
282 | @@ -344,18 +344,17 @@ |
283 | grid->expanded = false; |
284 | |
285 | group->SetRendererName(renderer_name.c_str()); |
286 | - grid->UriActivated.connect(sigc::bind([&] (std::string const& uri, ResultView::ActivateType type, std::string const& view_id) |
287 | + grid->UriActivated.connect(sigc::bind([&] (std::string const& uri, ResultView::ActivateType type, GVariant* data, std::string const& view_id) |
288 | { |
289 | + uri_activated.emit(type, uri, data, view_id); |
290 | switch (type) |
291 | { |
292 | case ResultView::ActivateType::DIRECT: |
293 | { |
294 | - uri_activated.emit(uri); |
295 | lens_->Activate(uri); |
296 | } break; |
297 | case ResultView::ActivateType::PREVIEW: |
298 | { |
299 | - uri_preview_activated.emit(uri, view_id); |
300 | lens_->Preview(uri); |
301 | } break; |
302 | default: break; |
303 | @@ -452,6 +451,13 @@ |
304 | return static_cast<ResultViewGrid*>(group->GetChildView()); |
305 | } |
306 | |
307 | +ResultView* LensView::GetResultViewForCategory(unsigned category_index) |
308 | +{ |
309 | + if (category_index >= categories_.size()) return nullptr; |
310 | + PlacesGroup* group = categories_.at(category_index); |
311 | + return static_cast<ResultView*>(group->GetChildView()); |
312 | +} |
313 | + |
314 | void LensView::OnResultAdded(Result const& result) |
315 | { |
316 | try { |
317 | @@ -720,22 +726,22 @@ |
318 | for (unsigned int i = 0; i < category_order.size(); i++) |
319 | { |
320 | unsigned cat_index = category_order.at(i); |
321 | - ResultViewGrid* grid = GetGridForCategory(cat_index); |
322 | - if (grid == nullptr) continue; |
323 | - auto it = grid->GetIteratorAtRow(0); |
324 | + ResultView* result_view = GetResultViewForCategory(cat_index); |
325 | + if (result_view == nullptr) continue; |
326 | + auto it = result_view->GetIteratorAtRow(0); |
327 | if (!it.IsLast()) |
328 | { |
329 | Result result(*it); |
330 | - uri_activated(result.uri); |
331 | - lens_->Activate(result.uri); |
332 | + result_view->Activate(result.uri, result_view->GetIndexForUri(result.uri), ResultView::ActivateType::DIRECT); |
333 | return; |
334 | } |
335 | } |
336 | + |
337 | // Fallback |
338 | Result result = results->RowAtIndex(0); |
339 | if (result.uri != "") |
340 | { |
341 | - uri_activated(result.uri); |
342 | + uri_activated.emit(ResultView::ActivateType::DIRECT, result.uri, nullptr, ""); |
343 | lens_->Activate(result.uri); |
344 | } |
345 | } |
346 | |
347 | === modified file 'dash/LensView.h' |
348 | --- dash/LensView.h 2012-09-19 18:45:56 +0000 |
349 | +++ dash/LensView.h 2012-09-27 16:36:23 +0000 |
350 | @@ -68,8 +68,7 @@ |
351 | nux::Property<ViewType> view_type; |
352 | nux::Property<bool> can_refine_search; |
353 | |
354 | - sigc::signal<void, std::string const&> uri_activated; |
355 | - sigc::signal<void, std::string const&, std::string const&> uri_preview_activated; |
356 | + sigc::signal<void, ResultView::ActivateType, std::string const&, GVariant*, std::string const&> uri_activated; |
357 | |
358 | void PerformSearch(std::string const& search_query); |
359 | void CheckNoResults(Lens::Hints const& hints); |
360 | @@ -95,6 +94,7 @@ |
361 | void OnViewTypeChanged(ViewType view_type); |
362 | bool ReinitializeFilterModels(); |
363 | ResultViewGrid* GetGridForCategory(unsigned category_index); |
364 | + ResultView* GetResultViewForCategory(unsigned category_index); |
365 | |
366 | void BuildPreview(std::string const& uri, Preview::Ptr model); |
367 | |
368 | |
369 | === modified file 'dash/PreviewStateMachine.cpp' |
370 | --- dash/PreviewStateMachine.cpp 2012-08-15 09:54:22 +0000 |
371 | +++ dash/PreviewStateMachine.cpp 2012-09-27 16:36:23 +0000 |
372 | @@ -51,8 +51,6 @@ |
373 | { |
374 | stored_preview_ = preview; |
375 | CheckPreviewRequirementsFulfilled(); |
376 | - left_results = -1; |
377 | - right_results = -1; |
378 | requires_activation_ = true; |
379 | } |
380 | |
381 | @@ -66,8 +64,7 @@ |
382 | |
383 | void PreviewStateMachine::ClosePreview() |
384 | { |
385 | - stored_preview_ = nullptr; |
386 | - preview_active = true; |
387 | + Reset(); |
388 | SetSplitPosition(SplitPosition::CONTENT_AREA, -1); |
389 | } |
390 | |
391 | |
392 | === modified file 'dash/ResultView.cpp' |
393 | --- dash/ResultView.cpp 2012-09-02 20:34:37 +0000 |
394 | +++ dash/ResultView.cpp 2012-09-27 16:36:23 +0000 |
395 | @@ -28,6 +28,7 @@ |
396 | #include <Nux/VLayout.h> |
397 | #include <Nux/Button.h> |
398 | #include <NuxCore/Logger.h> |
399 | +#include <UnityCore/Variant.h> |
400 | |
401 | namespace unity |
402 | { |
403 | |
404 | === modified file 'dash/ResultView.h' |
405 | --- dash/ResultView.h 2012-09-02 20:34:37 +0000 |
406 | +++ dash/ResultView.h 2012-09-27 16:36:23 +0000 |
407 | @@ -61,13 +61,15 @@ |
408 | nux::Property<bool> expanded; |
409 | nux::Property<int> results_per_row; |
410 | nux::Property<std::string> unique_id; |
411 | - sigc::signal<void, std::string const&, ActivateType> UriActivated; |
412 | + sigc::signal<void, std::string const&, ActivateType, GVariant*> UriActivated; |
413 | |
414 | std::string GetName() const; |
415 | ResultIterator GetIteratorAtRow(unsigned row); |
416 | void AddProperties(GVariantBuilder* builder); |
417 | IntrospectableList GetIntrospectableChildren(); |
418 | |
419 | + virtual void Activate(std::string const& uri, int index, ActivateType type) = 0; |
420 | + |
421 | protected: |
422 | virtual void Draw(nux::GraphicsEngine& GfxContext, bool force_draw); |
423 | virtual void DrawContent(nux::GraphicsEngine& GfxContext, bool force_draw); |
424 | |
425 | === modified file 'dash/ResultViewGrid.cpp' |
426 | --- dash/ResultViewGrid.cpp 2012-09-24 16:47:33 +0000 |
427 | +++ dash/ResultViewGrid.cpp 2012-09-27 16:36:23 +0000 |
428 | @@ -28,6 +28,7 @@ |
429 | #include <gdk/gdk.h> |
430 | #include <unity-protocol.h> |
431 | |
432 | +#include <UnityCore/Variant.h> |
433 | #include "unity-shared/IntrospectableWrappers.h" |
434 | #include "unity-shared/Timer.h" |
435 | #include "unity-shared/ubus-server.h" |
436 | @@ -75,7 +76,7 @@ |
437 | key_nav_focus_change.connect(sigc::mem_fun(this, &ResultViewGrid::OnKeyNavFocusChange)); |
438 | key_nav_focus_activate.connect([&] (nux::Area *area) |
439 | { |
440 | - UriActivated.emit (focused_uri_, ResultView::ActivateType::DIRECT); |
441 | + Activate(focused_uri_, selected_index_, ResultView::ActivateType::DIRECT); |
442 | }); |
443 | key_down.connect(sigc::mem_fun(this, &ResultViewGrid::OnKeyDown)); |
444 | mouse_move.connect(sigc::mem_fun(this, &ResultViewGrid::MouseMove)); |
445 | @@ -140,30 +141,9 @@ |
446 | { |
447 | activated_uri_ = GetUriForIndex(current_index); |
448 | LOG_DEBUG(logger) << "activating preview for index: " |
449 | - << "(" << current_index << ")" |
450 | - << " " << activated_uri_; |
451 | - int left_results = current_index; |
452 | - int right_results = num_results ? (num_results - current_index) - 1 : 0; |
453 | - |
454 | - int row_y = padding + GetRootGeometry().y; |
455 | - int row_size = renderer_->height + vertical_spacing; |
456 | - int row_height = row_size; |
457 | - |
458 | - if (GetItemsPerRow()) |
459 | - { |
460 | - int num_row = GetNumResults() / GetItemsPerRow(); |
461 | - if (GetNumResults() % GetItemsPerRow()) |
462 | - { |
463 | - ++num_row; |
464 | - } |
465 | - int row_index = current_index / GetItemsPerRow(); |
466 | - |
467 | - row_y += row_index * row_size; |
468 | - } |
469 | - |
470 | - ubus_.SendMessage(UBUS_DASH_PREVIEW_INFO_PAYLOAD, |
471 | - g_variant_new("(iiii)", row_y, row_height, left_results, right_results)); |
472 | - UriActivated.emit(activated_uri_, ActivateType::PREVIEW); |
473 | + << "(" << current_index << ")" |
474 | + << " " << activated_uri_; |
475 | + Activate(activated_uri_, current_index, ActivateType::PREVIEW); |
476 | } |
477 | |
478 | } |
479 | @@ -176,6 +156,33 @@ |
480 | SetDndEnabled(true, false); |
481 | } |
482 | |
483 | +void ResultViewGrid::Activate(std::string const& uri, int index, ResultView::ActivateType type) |
484 | +{ |
485 | + unsigned num_results = GetNumResults(); |
486 | + |
487 | + int left_results = index; |
488 | + int right_results = num_results ? (num_results - index) - 1 : 0; |
489 | + //FIXME - just uses y right now, needs to use the absolute position of the bottom of the result |
490 | + // (jay) Here is the fix: Compute the y position of the row where the item is located. |
491 | + int row_y = padding + GetRootGeometry().y; |
492 | + int row_height = renderer_->height + vertical_spacing; |
493 | + |
494 | + if (GetItemsPerRow()) |
495 | + { |
496 | + int num_row = GetNumResults() / GetItemsPerRow(); |
497 | + if (GetNumResults() % GetItemsPerRow()) |
498 | + { |
499 | + ++num_row; |
500 | + } |
501 | + int row_index = index / GetItemsPerRow(); |
502 | + |
503 | + row_y += row_index * row_height; |
504 | + } |
505 | + |
506 | + glib::Variant data(g_variant_new("(iiii)", row_y, row_height, left_results, right_results)); |
507 | + UriActivated.emit(uri, type, data); |
508 | +} |
509 | + |
510 | void ResultViewGrid::QueueLazyLoad() |
511 | { |
512 | lazy_load_source_.reset(new glib::Idle(glib::Source::Priority::DEFAULT)); |
513 | @@ -483,12 +490,7 @@ |
514 | |
515 | if (event_type == nux::NUX_KEYDOWN && event_keysym == XK_Menu) |
516 | { |
517 | - UriActivated.emit (focused_uri_, ResultView::ActivateType::PREVIEW); |
518 | - int left_results = selected_index_; |
519 | - int right_results = (num_results - selected_index_) - 1; |
520 | - //FIXME - just uses y right now, needs to use the absolute position of the bottom of the result |
521 | - ubus_.SendMessage(UBUS_DASH_PREVIEW_INFO_PAYLOAD, |
522 | - g_variant_new("(iii)", mouse_last_y_, left_results, right_results)); |
523 | + Activate(focused_uri_, selected_index_, ActivateType::PREVIEW); |
524 | } |
525 | } |
526 | |
527 | @@ -715,36 +717,12 @@ |
528 | Result result = *it; |
529 | selected_index_ = index; |
530 | focused_uri_ = result.uri; |
531 | - if (nux::GetEventButton(button_flags) == nux::MouseButton::MOUSE_BUTTON3) |
532 | - { |
533 | - activated_uri_ = result.uri(); |
534 | - UriActivated.emit(result.uri, ResultView::ActivateType::PREVIEW); |
535 | - int left_results = index; |
536 | - int right_results = (num_results - index) - 1; |
537 | - //FIXME - just uses y right now, needs to use the absolute position of the bottom of the result |
538 | - // (jay) Here is the fix: Compute the y position of the row where the item is located. |
539 | - int row_y = padding + GetRootGeometry().y; |
540 | - int row_size = renderer_->height + vertical_spacing; |
541 | - int row_height = row_size; |
542 | - |
543 | - if (GetItemsPerRow()) |
544 | - { |
545 | - int num_row = GetNumResults() / GetItemsPerRow(); |
546 | - if (GetNumResults() % GetItemsPerRow()) |
547 | - { |
548 | - ++num_row; |
549 | - } |
550 | - int row_index = index / GetItemsPerRow(); |
551 | - |
552 | - row_y += row_index * row_size; |
553 | - } |
554 | - ubus_.SendMessage(UBUS_DASH_PREVIEW_INFO_PAYLOAD, |
555 | - g_variant_new("(iiii)", row_y, row_height, left_results, right_results)); |
556 | - } |
557 | - else |
558 | - { |
559 | - UriActivated.emit(result.uri, ResultView::ActivateType::DIRECT); |
560 | - } |
561 | + |
562 | + ActivateType type = nux::GetEventButton(button_flags) == nux::MouseButton::MOUSE_BUTTON3 ? ResultView::ActivateType::PREVIEW : |
563 | + ResultView::ActivateType::DIRECT; |
564 | + |
565 | + activated_uri_ = result.uri(); |
566 | + Activate(activated_uri_, index, type); |
567 | } |
568 | } |
569 | |
570 | |
571 | === modified file 'dash/ResultViewGrid.h' |
572 | --- dash/ResultViewGrid.h 2012-09-17 12:16:29 +0000 |
573 | +++ dash/ResultViewGrid.h 2012-09-27 16:36:23 +0000 |
574 | @@ -52,6 +52,8 @@ |
575 | |
576 | int GetSelectedIndex(); |
577 | virtual unsigned GetIndexAtPosition(int x, int y); |
578 | + |
579 | + virtual void Activate(std::string const& uri, int index, ActivateType type); |
580 | |
581 | protected: |
582 | void MouseMove(int x, int y, int dx, int dy, unsigned long button_flags, unsigned long key_flags); |
583 | |
584 | === modified file 'dash/previews/PreviewContainer.cpp' |
585 | --- dash/previews/PreviewContainer.cpp 2012-09-19 14:40:41 +0000 |
586 | +++ dash/previews/PreviewContainer.cpp 2012-09-27 16:36:23 +0000 |
587 | @@ -47,11 +47,8 @@ |
588 | { |
589 | nux::logging::Logger logger("unity.dash.previews.previewcontainer"); |
590 | |
591 | - |
592 | - |
593 | -const int ANIM_DURATION_SHORT_SHORT = 100; |
594 | -const int ANIM_DURATION = 200; |
595 | const int ANIM_DURATION_LONG = 500; |
596 | +const int PREVIEW_SPINNER_WAIT = 300; |
597 | |
598 | const std::string ANIMATION_IDLE = "animation-idle"; |
599 | } |
600 | @@ -99,11 +96,10 @@ |
601 | |
602 | void PushPreview(previews::Preview::Ptr preview, Navigation direction) |
603 | { |
604 | - preview_initiate_count_++; |
605 | - StopPreviewWait(); |
606 | - |
607 | if (preview) |
608 | { |
609 | + preview_initiate_count_++; |
610 | + StopPreviewWait(); |
611 | // the parents layout will not change based on the previews. |
612 | preview->SetReconfigureParentLayoutOnGeometryChange(false); |
613 | |
614 | @@ -111,6 +107,11 @@ |
615 | AddView(preview.GetPointer()); |
616 | preview->SetVisible(false); |
617 | } |
618 | + else |
619 | + { |
620 | + // if we push a null preview, then start waiting. |
621 | + StartPreviewWait(); |
622 | + } |
623 | PreviewSwipe swipe; |
624 | swipe.direction = direction; |
625 | swipe.preview = preview; |
626 | @@ -227,7 +228,7 @@ |
627 | |
628 | void StartPreviewWait() |
629 | { |
630 | - preview_wait_timer_.reset(new glib::Timeout(300, [&] () { |
631 | + preview_wait_timer_.reset(new glib::Timeout(PREVIEW_SPINNER_WAIT, [&] () { |
632 | |
633 | if (waiting_preview_) |
634 | return false; |
635 | |
636 | === modified file 'manual-tests/Preview.txt' |
637 | --- manual-tests/Preview.txt 2012-09-19 17:49:17 +0000 |
638 | +++ manual-tests/Preview.txt 2012-09-27 16:36:23 +0000 |
639 | @@ -170,3 +170,20 @@ |
640 | Expected Result: |
641 | The button A must not be highlighted. |
642 | The button B must be highlighted. |
643 | + |
644 | + |
645 | +Preview Open Wait |
646 | +------------------------------ |
647 | +This tests that if a preview takes time to open, the preview will open with a spinner |
648 | +while it waits for the preview info to return to unity. |
649 | + |
650 | +Setup: |
651 | +#. Open dash. |
652 | +#. Open Application Lens. |
653 | + |
654 | +Actions: |
655 | +#. Right-click an arbitrary result |
656 | + |
657 | +Expected Result: |
658 | + The preview window will immediately display, and if the preview takes longer |
659 | + than 300ms to open, it will display a spinner while waiting. |
660 | |
661 | === modified file 'unity-shared/UBusMessages.h' |
662 | --- unity-shared/UBusMessages.h 2012-09-18 13:59:06 +0000 |
663 | +++ unity-shared/UBusMessages.h 2012-09-27 16:36:23 +0000 |
664 | @@ -82,10 +82,6 @@ |
665 | // FIXME - fix the nux focus api so we don't need this |
666 | #define UBUS_RESULT_VIEW_KEYNAV_CHANGED "RESULT_VIEW_KEYNAV_CHANGED" |
667 | |
668 | -// for communicating positions to the preview state machine (iiii) |
669 | -// (split y coord in absolute geometry, height of row where icon resides, results to the left, results to the right) |
670 | -#define UBUS_DASH_PREVIEW_INFO_PAYLOAD "DASH_PREVIEW_INFO_PAYLOAD" |
671 | - |
672 | // called when previews wish to navigate left/right or close (is) |
673 | // -1 = left, 0 = close, 1 = right, |
674 | // string is the uri string that last result activated was |
Confirmed working, and looks good to me.
I think at lease a manual test should be added to show that the spinner shows when right clicking a shopping lens results.