From ccf707d0bb9995ee9046e4b92a666144f72d395b Mon Sep 17 00:00:00 2001 From: Andrew Lindesay Date: Mon, 30 Mar 2020 22:31:34 +1300 Subject: [PATCH] HaikuDepot: Fixes for Tabs Cleans up the code around tab-switching and also improves some logic around inserting packages into the list of 'prominent' packages by using a binary search. If Haiku is installed into an environment with no networking then it won't be able to talk to HDS and so won't know which packages are promoted. In this case switch the user to the "all packages" tab so they are not shown a blank panel by default. Relates to #14675, #14927 Change-Id: I14dd3be4af09a98245ddd0a9704bd8d53ed64a53 Reviewed-on: https://review.haiku-os.org/c/haiku/+/2478 Reviewed-by: waddlesplash --- src/apps/haikudepot/model/Captcha.cpp | 6 +- src/apps/haikudepot/model/Model.cpp | 46 ++---- src/apps/haikudepot/model/Model.h | 24 ++-- src/apps/haikudepot/model/PackageInfo.cpp | 13 ++ src/apps/haikudepot/model/PackageInfo.h | 2 + .../haikudepot/ui/FeaturedPackagesView.cpp | 96 ++++++++++--- src/apps/haikudepot/ui/FeaturedPackagesView.h | 9 ++ src/apps/haikudepot/ui/MainWindow.cpp | 133 +++++++++++++----- src/apps/haikudepot/ui/MainWindow.h | 5 +- 9 files changed, 238 insertions(+), 96 deletions(-) diff --git a/src/apps/haikudepot/model/Captcha.cpp b/src/apps/haikudepot/model/Captcha.cpp index b4b9df85c2..4bf5022818 100644 --- a/src/apps/haikudepot/model/Captcha.cpp +++ b/src/apps/haikudepot/model/Captcha.cpp @@ -21,9 +21,10 @@ Captcha::Captcha(BMessage* from) fToken(""), fPngImageData(NULL) { - if (from->FindString(KEY_TOKEN, &fToken) != B_OK) + if (from->FindString(KEY_TOKEN, &fToken) != B_OK) { printf("expected key [%s] in the message data when creating a " "Captcha\n", KEY_TOKEN); + } const void* data; ssize_t len; @@ -45,8 +46,7 @@ Captcha::Captcha() Captcha::~Captcha() { - if (fPngImageData != NULL) - delete fPngImageData; + delete fPngImageData; } diff --git a/src/apps/haikudepot/model/Model.cpp b/src/apps/haikudepot/model/Model.cpp index 25d3d9749a..4c7900cf87 100644 --- a/src/apps/haikudepot/model/Model.cpp +++ b/src/apps/haikudepot/model/Model.cpp @@ -286,19 +286,6 @@ private: }; -class IsFeaturedFilter : public PackageFilter { -public: - IsFeaturedFilter() - { - } - - virtual bool AcceptsPackage(const PackageInfoRef& package) const - { - return package.Get() != NULL && package->IsProminent(); - } -}; - - static inline bool is_source_package(const PackageInfoRef& package) { @@ -338,14 +325,12 @@ Model::Model() fCategoryFilter(PackageFilterRef(new AnyFilter(), true)), fDepotFilter(""), fSearchTermsFilter(PackageFilterRef(new AnyFilter(), true)), - fIsFeaturedFilter(), - fShowFeaturedPackages(true), + fPackageListViewMode(PROMINENT), fShowAvailablePackages(true), fShowInstalledPackages(true), fShowSourcePackages(false), fShowDevelopPackages(false) { - _UpdateIsFeaturedFilter(); } @@ -399,7 +384,6 @@ Model::MatchesFilter(const PackageInfoRef& package) const { return fCategoryFilter->AcceptsPackage(package) && fSearchTermsFilter->AcceptsPackage(package) - && fIsFeaturedFilter->AcceptsPackage(package) && (fShowAvailablePackages || package->State() != NONE) && (fShowInstalledPackages || package->State() != ACTIVATED) && (fShowSourcePackages || !is_source_package(package)) @@ -448,6 +432,18 @@ Model::SyncDepot(const DepotInfo& depot) } +bool +Model::HasAnyProminentPackages() +{ + for (int32 i = fDepots.CountItems() - 1; i >= 0; i--) { + const DepotInfo& existingDepot = fDepots.ItemAtFast(i); + if (existingDepot.HasAnyProminentPackages()) + return true; + } + return false; +} + + void Model::Clear() { @@ -543,7 +539,6 @@ Model::SetSearchTerms(const BString& searchTerms) filter = new SearchTermsFilter(searchTerms); fSearchTermsFilter.SetTo(filter, true); - _UpdateIsFeaturedFilter(); } @@ -559,10 +554,9 @@ Model::SearchTerms() const void -Model::SetShowFeaturedPackages(bool show) +Model::SetPackageListViewMode(package_list_view_mode mode) { - fShowFeaturedPackages = show; - _UpdateIsFeaturedFilter(); + fPackageListViewMode = mode; } @@ -959,16 +953,6 @@ Model::DumpExportPkgDataPath(BPath& path, } -void -Model::_UpdateIsFeaturedFilter() -{ - if (fShowFeaturedPackages && SearchTerms().IsEmpty()) - fIsFeaturedFilter = PackageFilterRef(new IsFeaturedFilter(), true); - else - fIsFeaturedFilter = PackageFilterRef(new AnyFilter(), true); -} - - void Model::_PopulatePackageScreenshot(const PackageInfoRef& package, const ScreenshotInfo& info, int32 scaledWidth, bool fromCacheOnly) diff --git a/src/apps/haikudepot/model/Model.h b/src/apps/haikudepot/model/Model.h index 2bcb4755b1..7fd161cce2 100644 --- a/src/apps/haikudepot/model/Model.h +++ b/src/apps/haikudepot/model/Model.h @@ -20,6 +20,12 @@ class BMessage; class BPath; +typedef enum package_list_view_mode { + PROMINENT, + ALL +} package_list_view_mode; + + class PackageFilter : public BReferenceable { public: virtual ~PackageFilter(); @@ -83,6 +89,7 @@ public: { return fDepots; } const DepotInfo* DepotForName(const BString& name) const; bool SyncDepot(const DepotInfo& depot); + bool HasAnyProminentPackages(); void Clear(); @@ -102,9 +109,11 @@ public: void SetSearchTerms(const BString& searchTerms); BString SearchTerms() const; - void SetShowFeaturedPackages(bool show); - bool ShowFeaturedPackages() const - { return fShowFeaturedPackages; } + void SetPackageListViewMode( + package_list_view_mode mode); + package_list_view_mode + PackageListViewMode() const + { return fPackageListViewMode; } void SetShowAvailablePackages(bool show); bool ShowAvailablePackages() const { return fShowAvailablePackages; } @@ -136,7 +145,8 @@ public: const BString& passwordClear, bool storePassword); - const WebAppInterface& GetWebAppInterface() const + const WebAppInterface& + GetWebAppInterface() const { return fWebAppInterface; } void ReplaceDepotByUrl( @@ -159,8 +169,6 @@ private: const BMessage &responsePayload, const char *sourceDescription) const; - void _UpdateIsFeaturedFilter(); - static int32 _PopulateAllPackagesEntry(void* cookie); void _PopulatePackageChangelog( @@ -191,9 +199,9 @@ private: PackageFilterRef fCategoryFilter; BString fDepotFilter; PackageFilterRef fSearchTermsFilter; - PackageFilterRef fIsFeaturedFilter; - bool fShowFeaturedPackages; + package_list_view_mode + fPackageListViewMode; bool fShowAvailablePackages; bool fShowInstalledPackages; bool fShowSourcePackages; diff --git a/src/apps/haikudepot/model/PackageInfo.cpp b/src/apps/haikudepot/model/PackageInfo.cpp index 237ce9250a..855d193d7f 100644 --- a/src/apps/haikudepot/model/PackageInfo.cpp +++ b/src/apps/haikudepot/model/PackageInfo.cpp @@ -1178,6 +1178,19 @@ DepotInfo::SyncPackages(const PackageList& otherPackages) } +bool +DepotInfo::HasAnyProminentPackages() const +{ + int32 count = fPackages.CountItems(); + for (int32 i = 0; i < count; i++) { + const PackageInfoRef& package = fPackages.ItemAtFast(i); + if (package->IsProminent()) + return true; + } + return false; +} + + void DepotInfo::SetURL(const BString& URL) { diff --git a/src/apps/haikudepot/model/PackageInfo.h b/src/apps/haikudepot/model/PackageInfo.h index 9ccaeb443a..512b6a4bb4 100644 --- a/src/apps/haikudepot/model/PackageInfo.h +++ b/src/apps/haikudepot/model/PackageInfo.h @@ -436,6 +436,8 @@ public: void SyncPackages(const PackageList& packages); + bool HasAnyProminentPackages() const; + void SetURL(const BString& URL); const BString& URL() const { return fURL; } diff --git a/src/apps/haikudepot/ui/FeaturedPackagesView.cpp b/src/apps/haikudepot/ui/FeaturedPackagesView.cpp index e8e0198052..860d17e465 100644 --- a/src/apps/haikudepot/ui/FeaturedPackagesView.cpp +++ b/src/apps/haikudepot/ui/FeaturedPackagesView.cpp @@ -380,31 +380,89 @@ FeaturedPackagesView::~FeaturedPackagesView() } +/*! This method will add the package into the list to be displayed. The + insertion will occur in alphabetical order. +*/ + void FeaturedPackagesView::AddPackage(const PackageInfoRef& package) { - // Find insertion index (alphabetical) - int32 index = 0; - for (int32 i = 0; BLayoutItem* item = fPackageListLayout->ItemAt(i); i++) { - PackageView* view = dynamic_cast(item->View()); - if (view == NULL) - break; - - BString name = view->PackageName(); - if (name == package->Name()) { - // Don't add packages more than once - return; - } - - BString title = view->PackageTitle(); - if (title.Compare(package->Title()) < 0) - index++; + int32 index = _InsertionIndex(package->Name()); + if (index != -1) { + PackageView* view = new PackageView(); + view->SetPackage(package); + fPackageListLayout->AddView(index, view); } +} - PackageView* view = new PackageView(); - view->SetPackage(package); - fPackageListLayout->AddView(index, view); +const char* +FeaturedPackagesView::_PackageNameAtIndex(int32 index) const +{ + BLayoutItem* item = fPackageListLayout->ItemAt(index); + PackageView* view = dynamic_cast(item->View()); + return (view != NULL ? view->PackageName() : NULL); + // some of the items in the GroupLayout instance are not of type + // PackageView* and it is not immediately clear where they are + // coming from. + +} + + +int32 +FeaturedPackagesView::_InsertionIndex(const BString& packageName) const +{ + int32 count = fPackageListLayout->CountItems(); + return _InsertionIndexBinary(packageName, 0, count - 1); +} + +int32 +FeaturedPackagesView::_InsertionIndexLinear(const BString& packageName, + int32 startIndex, int32 endIndex) const +{ + for (int32 i = startIndex; i <= endIndex; i++) { + const char* iPackageName = _PackageNameAtIndex(i); + if (NULL != iPackageName) { + int compare = packageName.Compare(iPackageName); + if (compare == 0) + return -1; + if (compare < 0) + return i; + } + } + return endIndex; +} + +/*! This performs a binary search to find the location at which to insert + the item. +*/ + +int32 +FeaturedPackagesView::_InsertionIndexBinary(const BString& packageName, + int32 startIndex, int32 endIndex) const +{ + if (startIndex == endIndex) + return startIndex; + + int32 endStartSpan = endIndex - startIndex; + + if (endStartSpan < 5) + return _InsertionIndexLinear(packageName, startIndex, endIndex); + + int midIndex = startIndex + (endStartSpan / 2); + const char *midPackageName = _PackageNameAtIndex(midIndex); + + if (midPackageName == NULL) + return _InsertionIndexLinear(packageName, startIndex, endIndex); + + int compare = packageName.Compare(midPackageName); + + if (compare == 0) + return -1; + // don't want to insert the same package twice. + if (compare < 0) + return _InsertionIndexBinary(packageName, startIndex, midIndex); + return _InsertionIndexBinary(packageName, midIndex, endIndex); } diff --git a/src/apps/haikudepot/ui/FeaturedPackagesView.h b/src/apps/haikudepot/ui/FeaturedPackagesView.h index 6658f3e1f9..fd88d1db84 100644 --- a/src/apps/haikudepot/ui/FeaturedPackagesView.h +++ b/src/apps/haikudepot/ui/FeaturedPackagesView.h @@ -30,6 +30,15 @@ public: static void CleanupIcons(); +private: + const char* _PackageNameAtIndex(int32 index) const; + int32 _InsertionIndex( + const BString& packageName) const; + int32 _InsertionIndexBinary(const BString& packageName, + int32 startIndex, int32 endIndex) const; + int32 _InsertionIndexLinear(const BString& packageName, + int32 startIndex, int32 endIndex) const; + private: BGroupLayout* fPackageListLayout; ScrollableGroupView* fContainerView; diff --git a/src/apps/haikudepot/ui/MainWindow.cpp b/src/apps/haikudepot/ui/MainWindow.cpp index 12e6586f1a..fe8850234d 100644 --- a/src/apps/haikudepot/ui/MainWindow.cpp +++ b/src/apps/haikudepot/ui/MainWindow.cpp @@ -68,14 +68,18 @@ enum { MSG_WORK_STATUS_CHANGE = 'wsch', MSG_WORK_STATUS_CLEAR = 'wscl', - MSG_SHOW_FEATURED_PACKAGES = 'sofp', + MSG_CHANGE_PACKAGE_LIST_VIEW_MODE = 'cplm', MSG_SHOW_AVAILABLE_PACKAGES = 'savl', MSG_SHOW_INSTALLED_PACKAGES = 'sins', MSG_SHOW_SOURCE_PACKAGES = 'ssrc', MSG_SHOW_DEVELOP_PACKAGES = 'sdvl' }; -#define KEY_ERROR_STATUS "errorStatus" +#define KEY_ERROR_STATUS "errorStatus" +#define KEY_PACKAGE_LIST_VIEW_MODE "packageListViewMode" + +#define TAB_PROMINENT_PACKAGES 0 +#define TAB_ALL_PACKAGES 1 using namespace BPackageKit; using namespace BPackageKit::BManager::BPrivate; @@ -159,7 +163,7 @@ MainWindow::MainWindow(const BMessage& settings) fPackageListView->AttachWorkStatusView(fWorkStatusView); fListTabs = new TabView(BMessenger(this), - BMessage(MSG_SHOW_FEATURED_PACKAGES), "list tabs"); + BMessage(MSG_CHANGE_PACKAGE_LIST_VIEW_MODE), "list tabs"); fListTabs->AddTab(fFeaturedPackagesView); fListTabs->AddTab(fPackageListView); @@ -186,27 +190,16 @@ MainWindow::MainWindow(const BMessage& settings) fModel.AddListener(fModelListener); - // Restore settings BMessage columnSettings; if (settings.FindMessage("column settings", &columnSettings) == B_OK) fPackageListView->LoadState(&columnSettings); - bool showOption; - if (settings.FindBool("show featured packages", &showOption) == B_OK) - fModel.SetShowFeaturedPackages(showOption); - if (settings.FindBool("show available packages", &showOption) == B_OK) - fModel.SetShowAvailablePackages(showOption); - if (settings.FindBool("show installed packages", &showOption) == B_OK) - fModel.SetShowInstalledPackages(showOption); - if (settings.FindBool("show develop packages", &showOption) == B_OK) - fModel.SetShowDevelopPackages(showOption); - if (settings.FindBool("show source packages", &showOption) == B_OK) - fModel.SetShowSourcePackages(showOption); + _RestoreModelSettings(settings); - if (fModel.ShowFeaturedPackages()) - fListTabs->Select(0); + if (fModel.PackageListViewMode() == PROMINENT) + fListTabs->Select(TAB_PROMINENT_PACKAGES); else - fListTabs->Select(1); + fListTabs->Select(TAB_ALL_PACKAGES); _RestoreNickname(settings); _UpdateAuthorization(); @@ -255,6 +248,7 @@ MainWindow::MainWindow(const BMessage& settings, const PackageInfoRef& package) // Restore settings _RestoreNickname(settings); + _UpdateAuthorization(); _RestoreWindowFrame(settings); fPackageInfoView->SetPackage(package); @@ -375,17 +369,8 @@ MainWindow::MessageReceived(BMessage* message) fFilterView->AdoptModel(fModel); break; - case MSG_SHOW_FEATURED_PACKAGES: - // check to see if we aren't already on the current tab - if (fListTabs->Selection() == - (fModel.ShowFeaturedPackages() ? 0 : 1)) - break; - { - BAutolock locker(fModel.Lock()); - fModel.SetShowFeaturedPackages( - fListTabs->Selection() == 0); - } - _AdoptModel(); + case MSG_CHANGE_PACKAGE_LIST_VIEW_MODE: + _HandleChangePackageListViewMode(); break; case MSG_SHOW_AVAILABLE_PACKAGES: @@ -661,6 +646,24 @@ MainWindow::MessageReceived(BMessage* message) } +static const char* +main_window_package_list_view_mode_str(package_list_view_mode mode) +{ + if (mode == PROMINENT) + return "PROMINENT"; + return "ALL"; +} + + +static package_list_view_mode +main_window_str_to_package_list_view_mode(const BString& str) +{ + if (str == "PROMINENT") + return PROMINENT; + return ALL; +} + + void MainWindow::StoreSettings(BMessage& settings) const { @@ -673,8 +676,9 @@ MainWindow::StoreSettings(BMessage& settings) const settings.AddMessage("column settings", &columnSettings); - settings.AddBool("show featured packages", - fModel.ShowFeaturedPackages()); + settings.AddString(KEY_PACKAGE_LIST_VIEW_MODE, + main_window_package_list_view_mode_str( + fModel.PackageListViewMode())); settings.AddBool("show available packages", fModel.ShowAvailablePackages()); settings.AddBool("show installed packages", @@ -851,6 +855,28 @@ MainWindow::_RestoreWindowFrame(const BMessage& settings) } +void +MainWindow::_RestoreModelSettings(const BMessage& settings) +{ + BString packageListViewMode; + if (settings.FindString(KEY_PACKAGE_LIST_VIEW_MODE, + &packageListViewMode) == B_OK) { + fModel.SetPackageListViewMode( + main_window_str_to_package_list_view_mode(packageListViewMode)); + } + + bool showOption; + if (settings.FindBool("show available packages", &showOption) == B_OK) + fModel.SetShowAvailablePackages(showOption); + if (settings.FindBool("show installed packages", &showOption) == B_OK) + fModel.SetShowInstalledPackages(showOption); + if (settings.FindBool("show develop packages", &showOption) == B_OK) + fModel.SetShowDevelopPackages(showOption); + if (settings.FindBool("show source packages", &showOption) == B_OK) + fModel.SetShowSourcePackages(showOption); +} + + void MainWindow::_InitWorkerThreads() { @@ -887,6 +913,9 @@ MainWindow::_InitWorkerThreads() void MainWindow::_AdoptModel() { + if (Logger::IsTraceEnabled()) + printf("adopting model to main window ui"); + { AutoLocker modelLocker(fModel.Lock()); fVisiblePackages = fModel.CreatePackageList(); @@ -906,10 +935,10 @@ MainWindow::_AdoptModel() fShowSourcePackagesItem->SetMarked(fModel.ShowSourcePackages()); fShowDevelopPackagesItem->SetMarked(fModel.ShowDevelopPackages()); - if (fModel.ShowFeaturedPackages()) - fListTabs->Select(0); + if (fModel.PackageListViewMode() == PROMINENT) + fListTabs->Select(TAB_PROMINENT_PACKAGES); else - fListTabs->Select(1); + fListTabs->Select(TAB_ALL_PACKAGES); fFilterView->AdoptModel(fModel); } @@ -971,6 +1000,19 @@ MainWindow::_BulkLoadCompleteReceived(status_t errorStatus) fRefreshRepositoriesItem->SetEnabled(true); _AdoptModel(); _UpdateAvailableRepositories(); + + // if after loading everything in, it transpires that there are no + // featured packages then the featured packages should be disabled + // and the user should be switched to the "all packages" view so + // that they are not presented with a blank window! + + bool hasProminentPackages = fModel.HasAnyProminentPackages(); + fListTabs->TabAt(TAB_PROMINENT_PACKAGES)->SetEnabled(hasProminentPackages); + if (!hasProminentPackages + && fListTabs->Selection() == TAB_PROMINENT_PACKAGES) { + fModel.SetPackageListViewMode(ALL); + fListTabs->Select(TAB_ALL_PACKAGES); + } } @@ -1562,4 +1604,27 @@ MainWindow::CoordinatorChanged(ProcessCoordinatorState& coordinatorState) if (Logger::IsInfoEnabled()) printf("! unknown process coordinator changed\n"); } +} + + +static package_list_view_mode +main_window_tab_to_package_list_view_mode(int32 tab) +{ + if (tab == TAB_PROMINENT_PACKAGES) + return PROMINENT; + return ALL; +} + + +void +MainWindow::_HandleChangePackageListViewMode() +{ + package_list_view_mode tabMode = main_window_tab_to_package_list_view_mode( + fListTabs->Selection()); + package_list_view_mode modelMode = fModel.PackageListViewMode(); + + if (tabMode != modelMode) { + BAutolock locker(fModel.Lock()); + fModel.SetPackageListViewMode(tabMode); + } } \ No newline at end of file diff --git a/src/apps/haikudepot/ui/MainWindow.h b/src/apps/haikudepot/ui/MainWindow.h index 17cca4cfa1..0dddeae636 100644 --- a/src/apps/haikudepot/ui/MainWindow.h +++ b/src/apps/haikudepot/ui/MainWindow.h @@ -81,9 +81,10 @@ private: void _BuildMenu(BMenuBar* menuBar); void _BuildUserMenu(BMenuBar* menuBar); - void _RestoreNickname(const BMessage& settings); const char* _WindowFrameName() const; + void _RestoreNickname(const BMessage& settings); void _RestoreWindowFrame(const BMessage& settings); + void _RestoreModelSettings(const BMessage& settings); void _InitWorkerThreads(); void _AdoptModel(); @@ -103,6 +104,8 @@ private: void _HandleWorkStatusChangeMessageReceived( const BMessage* message); + void _HandleChangePackageListViewMode(); + static status_t _RefreshModelThreadWorker(void* arg); static status_t _PackageActionWorker(void* arg); static status_t _PopulatePackageWorker(void* arg);