Merge lp:~stolowski/unity/expand-2nd-home-category into lp:unity

Proposed by Paweł Stołowski
Status: Rejected
Rejected by: Andrea Azzarone
Proposed branch: lp:~stolowski/unity/expand-2nd-home-category
Merge into: lp:unity
Diff against target: 161 lines (+59/-18)
4 files modified
dash/DashView.cpp (+11/-0)
dash/LensView.cpp (+30/-15)
dash/LensView.h (+1/-1)
manual-tests/Dash.txt (+17/-2)
To merge this branch: bzr merge lp:~stolowski/unity/expand-2nd-home-category
Reviewer Review Type Date Requested Status
Andrea Azzarone (community) Disapprove
Michal Hruby (community) Needs Fixing
Neil J. Patel Pending
Review via email: mp+126453@code.launchpad.net

Commit message

Dash Home Lens: expand 2nd category (Files & Folders by default) if search is empty.

Description of the change

Dash Home Lens: expand 2nd category (Files & Folders by default) if search is empty.

To post a comment you must log in.
Revision history for this message
Andrea Azzarone (azzar1) wrote :

Do we really need to hardcode it? A category cannot expand itself using libunity etc.?

Revision history for this message
Andrea Azzarone (azzar1) :
review: Needs Information
Revision history for this message
Paweł Stołowski (stolowski) wrote :

Unfortunately there is no way to do this from lenses. Ideally, we should have a new expand-in-home-on-empty-search lens property in libunity Lens class, that would be passed via existing as{v} hints over DBus. I discussed this with Neil and he suggested handling it as an edge case on unity side only, as it's sanest for 12.10.

Revision history for this message
Michal Hruby (mhr3) wrote :

Can we at least do this when the search actually finishes and not arbitrarily after adding a few results?

Otherwise the category might get expanded, but as another lens finishes search and pushes more results it will again get unexpanded... Or is that what what we actually want?

> lens_->view_type == ViewType::LENS_VIEW

That doesn't look right, we should be using HOME_VIEW in home lens (if that's not the case the id check should suffice)

review: Needs Fixing
Revision history for this message
Andrea Azzarone (azzar1) wrote :

> Unfortunately there is no way to do this from lenses. Ideally, we should have
> a new expand-in-home-on-empty-search lens property in libunity Lens class,
> that would be passed via existing as{v} hints over DBus. I discussed this with
> Neil and he suggested handling it as an edge case on unity side only, as it's
> sanest for 12.10.

Got it.

2756. By Paweł Stołowski

Perform category expansion on search finished.

2757. By Paweł Stołowski

Merged trunk.

2758. By Paweł Stołowski

Added manual test for 2nd category expansion in dash home view.

Revision history for this message
Andrea Azzarone (azzar1) :
review: Disapprove

Unmerged revisions

2758. By Paweł Stołowski

Added manual test for 2nd category expansion in dash home view.

2757. By Paweł Stołowski

Merged trunk.

2756. By Paweł Stołowski

Perform category expansion on search finished.

2755. By Paweł Stołowski

Dash Home Lens: expand 2nd category (Files & Folders by default) if search is empty.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'dash/DashView.cpp'
--- dash/DashView.cpp 2012-10-29 09:34:54 +0000
+++ dash/DashView.cpp 2012-11-05 10:17:23 +0000
@@ -1013,6 +1013,17 @@
1013 if (active_lens_view_ == NULL) return;1013 if (active_lens_view_ == NULL) return;
10141014
1015 active_lens_view_->CheckNoResults(hints);1015 active_lens_view_->CheckNoResults(hints);
1016
1017 if (active_lens_view_ == home_view_ && active_lens_view_->lens()->last_search_string == "")
1018 {
1019 active_lens_view_->CheckCategoryExpansion(2);
1020 }
1021 else
1022 {
1023 // Check if all results so far are from one category
1024 // If so, then expand that category.
1025 active_lens_view_->CheckCategoryExpansion(1);
1026 }
1016 std::string const& search_string = search_bar_->search_string;1027 std::string const& search_string = search_bar_->search_string;
10171028
1018 if (active_lens_view_->search_string == search_string)1029 if (active_lens_view_->search_string == search_string)
10191030
=== modified file 'dash/LensView.cpp'
--- dash/LensView.cpp 2012-10-29 09:34:54 +0000
+++ dash/LensView.cpp 2012-11-05 10:17:23 +0000
@@ -483,16 +483,15 @@
483 CheckNoResults(Lens::Hints());483 CheckNoResults(Lens::Hints());
484 }484 }
485485
486 if (!model_updated_timeout_)486 /* Disabled for now, enable back if needed
487 if (!model_updated_timeout_)
487 {488 {
488 model_updated_timeout_.reset(new glib::Idle([&] () {489 model_updated_timeout_.reset(new glib::Idle([&] () {
489 // Check if all results so far are from one category
490 // If so, then expand that category.
491 CheckCategoryExpansion();
492 model_updated_timeout_.reset();490 model_updated_timeout_.reset();
493 return false;491 return false;
494 }, glib::Source::Priority::HIGH));492 }, glib::Source::Priority::HIGH));
495 }493 }
494 */
496 } catch (std::out_of_range& oor) {495 } catch (std::out_of_range& oor) {
497 LOG_WARN(logger) << "Result does not have a valid category index: "496 LOG_WARN(logger) << "Result does not have a valid category index: "
498 << boost::lexical_cast<unsigned int>(result.category_index)497 << boost::lexical_cast<unsigned int>(result.category_index)
@@ -566,7 +565,9 @@
566 }565 }
567}566}
568567
569void LensView::CheckCategoryExpansion()568
569/* Expand last category if number of categories with results is less than or equal to max_categories */
570void LensView::CheckCategoryExpansion(int max_categories)
570{571{
571 int number_of_displayed_categories = 0;572 int number_of_displayed_categories = 0;
572573
@@ -574,23 +575,37 @@
574 // If so, collapse it for now575 // If so, collapse it for now
575 if (last_expanded_group_ != nullptr)576 if (last_expanded_group_ != nullptr)
576 {577 {
577 last_expanded_group_->SetExpanded(false);578 last_expanded_group_->SetExpanded(false);
578 last_expanded_group_ = nullptr;579 last_expanded_group_ = nullptr;
579 }580 }
580581
581 // Cycle through all categories582 const std::vector<unsigned> order(lens_->GetCategoriesOrder());
583 unsigned last_category_index = 0;
584
585 // Cycle through all categories, find last category to expand based on order
582 for (auto category : categories_)586 for (auto category : categories_)
583 {587 {
584 if (counts_[category] > 0) {588 if (counts_[category] > 0) {
585 number_of_displayed_categories++;589 number_of_displayed_categories++;
586 last_expanded_group_ = category;590 for (unsigned i = 0; i < order.size(); i++)
591 {
592 if (order[i] == category->GetCategoryIndex())
593 {
594 if (i >= last_category_index)
595 {
596 last_category_index = i;
597 last_expanded_group_ = category;
598 }
599 break;
600 }
587 }601 }
602 }
588 }603 }
589604
590 if (number_of_displayed_categories == 1 && last_expanded_group_ != nullptr)605 if (number_of_displayed_categories <= max_categories && last_expanded_group_ != nullptr)
591 last_expanded_group_->SetExpanded(true);606 last_expanded_group_->SetExpanded(true);
592 else607 else
593 last_expanded_group_ = nullptr;608 last_expanded_group_ = nullptr;
594}609}
595610
596void LensView::HideResultsMessage()611void LensView::HideResultsMessage()
597612
=== modified file 'dash/LensView.h'
--- dash/LensView.h 2012-10-11 01:44:15 +0000
+++ dash/LensView.h 2012-11-05 10:17:23 +0000
@@ -72,7 +72,7 @@
7272
73 void PerformSearch(std::string const& search_query);73 void PerformSearch(std::string const& search_query);
74 void CheckNoResults(Lens::Hints const& hints);74 void CheckNoResults(Lens::Hints const& hints);
75 void CheckCategoryExpansion();75 void CheckCategoryExpansion(int max_categories);
76 void HideResultsMessage();76 void HideResultsMessage();
7777
78private:78private:
7979
=== modified file 'manual-tests/Dash.txt'
--- manual-tests/Dash.txt 2012-10-11 01:44:15 +0000
+++ manual-tests/Dash.txt 2012-11-05 10:17:23 +0000
@@ -90,7 +90,7 @@
90 * Open the dash home view90 * Open the dash home view
91 * Search for something on your system that returns multiple results (more than the number that can fit in a row)91 * Search for something on your system that returns multiple results (more than the number that can fit in a row)
92 yet all results are in one category only. This differs per system; try searching for "gnome" in the home lens, or "kde"92 yet all results are in one category only. This differs per system; try searching for "gnome" in the home lens, or "kde"
93 in the app lens. 93 in the app lens.
94 * Multiple results should be returned in both the cases, but if searching for "gnome" on the home lens returns folders in addition to apps94 * Multiple results should be returned in both the cases, but if searching for "gnome" on the home lens returns folders in addition to apps
95 or "kde" returns results from two categories in the apps lens, then keep experimenting with other search strings.95 or "kde" returns results from two categories in the apps lens, then keep experimenting with other search strings.
96 * When you have at least one situation in which results from only one category are returned, and the number of results are enough to overflow one single row,96 * When you have at least one situation in which results from only one category are returned, and the number of results are enough to overflow one single row,
@@ -101,6 +101,21 @@
101 header expands automatically101 header expands automatically
102102
103103
104Dash home category expand test
105------------------------------
106This tests whether the dash home expands 2nd category (Files & Folders by default) if there is no search query.
107https://bugs.launchpad.net/unity/+bug/1052914
108
109Setup:
110Make sure you've at least 6 files indexed by zeitgeist - just create some empty files in nautilus if needed.
111
112Actions:
113 * Open the dash home view, make sure search query is empty.
114
115Expected result:
116 * 'Files & Folders' category header is expanded.
117
118
104Test the Panel does not lose track of when the Dash is opened then closed.119Test the Panel does not lose track of when the Dash is opened then closed.
105--------------------------------------------------------------------------120--------------------------------------------------------------------------
106This tests shows the panel will not think the Dash is opened when it is closed.121This tests shows the panel will not think the Dash is opened when it is closed.
@@ -307,7 +322,7 @@
307 2758 ? Sl 0:00 /usr/lib/unity-lens-music/unity-music-daemon322 2758 ? Sl 0:00 /usr/lib/unity-lens-music/unity-music-daemon
308 2760 ? Sl 0:00 /usr/bin/python /usr/lib/unity-lens-video/unity-lens-video323 2760 ? Sl 0:00 /usr/bin/python /usr/lib/unity-lens-video/unity-lens-video
309 2816 ? Sl 0:00 /usr/lib/unity-lens-music/unity-musicstore-daemon324 2816 ? Sl 0:00 /usr/lib/unity-lens-music/unity-musicstore-daemon
310 325
311Wrong Result:326Wrong Result:
312 #2 and #4 both produce non-empty result which means the lenses were not327 #2 and #4 both produce non-empty result which means the lenses were not
313 lazy-loaded but they were running before the first dash use.328 lazy-loaded but they were running before the first dash use.