From 621200ebbd67f15ca5d0d061675409960bfe0b52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1ximo=20Casta=C3=B1eda?= Date: Sun, 28 Jan 2024 20:47:44 +0100 Subject: [PATCH] BOutlineListView: fix ItemUnderAt and accept NULL as superitem ItemUnderAt was returning items that were not under the superitem (this was fixed for EachItemUnder in hrev52210). Make a NULL superitem mean the parent of the topmost items. Despite not being explicitly documented in the BeBook, that's how BeOS works, the MenuWorld test app uses it and, well, it's handy. Change-Id: I2551e8ce874a6238c5e5fb1eb742e68e62d3928a Reviewed-on: https://review.haiku-os.org/c/haiku/+/7359 Reviewed-by: Adrien Destugues --- docs/user/interface/OutlineListView.dox | 9 +- headers/os/interface/OutlineListView.h | 2 + src/kits/interface/OutlineListView.cpp | 47 ++- .../kits/interface/OutlineListViewTest.cpp | 286 +++++++++++++++--- 4 files changed, 279 insertions(+), 65 deletions(-) diff --git a/docs/user/interface/OutlineListView.dox b/docs/user/interface/OutlineListView.dox index 5033527415..c00566f48a 100644 --- a/docs/user/interface/OutlineListView.dox +++ b/docs/user/interface/OutlineListView.dox @@ -580,7 +580,8 @@ bool oneLevelOnly) const \brief Returns the number of items under \a superItem. - \param superItem The base item. + \param superItem The base item, or NULL to use the virtual super item + of level 0 items. \param oneLevelOnly if \c true, only items located one level under superItem are considered. @@ -596,7 +597,8 @@ void* arg) \brief Calls \a eachFunc for each item under \a superItem. - \param superItem The base item. + \param superItem The base item, or NULL to use the virtual super item + of level 0 items. \param oneLevelOnly if \c true, only items located one level under superItem are considered. \param eachFunc The function to call on each item. @@ -611,7 +613,8 @@ bool oneLevelOnly, int32 index) const \brief Returns a pointer to the item at \a index under \a superItem. - \param superItem The base item. + \param superItem The base item, or NULL to use the virtual super item + of level 0 items. \param oneLevelOnly if \c true, only items located one level under superItem are considered. \param index The index of the item to get. diff --git a/headers/os/interface/OutlineListView.h b/headers/os/interface/OutlineListView.h index 2d519da82b..5fbc9a010a 100644 --- a/headers/os/interface/OutlineListView.h +++ b/headers/os/interface/OutlineListView.h @@ -139,6 +139,8 @@ private: int32 level, int32* _superIndex = NULL); int32 _FindPreviousVisibleIndex(int32 fullListIndex); + status_t _ItemsUnderSetup(BListItem* superItem, int32& startIndex, + uint32& baseLevel) const; private: BList fFullList; diff --git a/src/kits/interface/OutlineListView.cpp b/src/kits/interface/OutlineListView.cpp index f0affc8507..97b0bc3c7b 100644 --- a/src/kits/interface/OutlineListView.cpp +++ b/src/kits/interface/OutlineListView.cpp @@ -697,23 +697,21 @@ BOutlineListView::SortItemsUnder(BListItem* superItem, bool oneLevelOnly, int32 BOutlineListView::CountItemsUnder(BListItem* superItem, bool oneLevelOnly) const { - int32 i = FullListIndexOf(superItem); - if (i == -1) + int32 i = 0; + uint32 baseLevel = 0; + if (_ItemsUnderSetup(superItem, i, baseLevel) != B_OK) return 0; - ++i; int32 count = 0; - uint32 baseLevel = superItem->OutlineLevel(); - for (; i < FullListCountItems(); i++) { BListItem* item = FullListItemAt(i); // If we jump out of the subtree, return count - if (item->fLevel <= baseLevel) + if (item->fLevel < baseLevel) return count; // If the level matches, increase count - if (!oneLevelOnly || item->fLevel == baseLevel + 1) + if (!oneLevelOnly || item->fLevel == baseLevel) count++; } @@ -725,20 +723,20 @@ BListItem* BOutlineListView::EachItemUnder(BListItem* superItem, bool oneLevelOnly, BListItem* (*eachFunc)(BListItem* item, void* arg), void* arg) { - int32 i = FullListIndexOf(superItem); - if (i == -1) + int32 i = 0; + uint32 baseLevel = 0; + if (_ItemsUnderSetup(superItem, i, baseLevel) != B_OK) return NULL; - i++; // skip the superitem while (i < FullListCountItems()) { BListItem* item = FullListItemAt(i); // If we jump out of the subtree, return NULL - if (item->fLevel <= superItem->OutlineLevel()) + if (item->fLevel < baseLevel) return NULL; // If the level matches, check the index - if (!oneLevelOnly || item->fLevel == superItem->OutlineLevel() + 1) { + if (!oneLevelOnly || item->fLevel == baseLevel) { item = eachFunc(item, arg); if (item != NULL) return item; @@ -755,19 +753,20 @@ BListItem* BOutlineListView::ItemUnderAt(BListItem* superItem, bool oneLevelOnly, int32 index) const { - int32 i = FullListIndexOf(superItem); - if (i == -1) + int32 i = 0; + uint32 baseLevel = 0; + if (_ItemsUnderSetup(superItem, i, baseLevel) != B_OK) return NULL; while (i < FullListCountItems()) { BListItem* item = FullListItemAt(i); // If we jump out of the subtree, return NULL - if (item->fLevel < superItem->OutlineLevel()) + if (item->fLevel < baseLevel) return NULL; // If the level matches, check the index - if (!oneLevelOnly || item->fLevel == superItem->OutlineLevel() + 1) { + if (!oneLevelOnly || item->fLevel == baseLevel) { if (index == 0) return item; @@ -1209,3 +1208,19 @@ BOutlineListView::_FindPreviousVisibleIndex(int32 fullListIndex) return -1; } + + +status_t +BOutlineListView::_ItemsUnderSetup(BListItem* superItem, int32& startIndex, uint32& baseLevel) const +{ + if (superItem != NULL) { + startIndex = FullListIndexOf(superItem) + 1; + if (startIndex == 0) + return B_ENTRY_NOT_FOUND; + baseLevel = superItem->OutlineLevel() + 1; + } else { + startIndex = 0; + baseLevel = 0; + } + return B_OK; +} diff --git a/src/tests/kits/interface/OutlineListViewTest.cpp b/src/tests/kits/interface/OutlineListViewTest.cpp index 108b4c4c61..d6b92ac68b 100644 --- a/src/tests/kits/interface/OutlineListViewTest.cpp +++ b/src/tests/kits/interface/OutlineListViewTest.cpp @@ -19,17 +19,54 @@ int gIndex = 0; int gCount = 0; -BListItem* eachitemunder(BListItem* item, void* arg) { +template<> +struct CppUnit::assertion_traits +{ + static bool equal(const BListItem* x, const BListItem* y) { + return x == y; + } + + static string toString(const BListItem* x) { + if (x == NULL) + return "(null)"; + return ((BStringItem*)x)->Text(); + } +}; + + +BListItem* +CheckExpected(BListItem* item, void* arg) +{ BStringItem* str = (BStringItem*)item; - fprintf(stderr, "Item @%d: %s\n", gIndex, str->Text()); + fprintf(stderr, "Item @%d: %s\n", gIndex, str == NULL ? "(null)" : str->Text()); CHK(gIndex < gCount); - CPPUNIT_ASSERT_EQUAL(item, gExpected[gIndex]); + CPPUNIT_ASSERT_EQUAL(gExpected[gIndex], item); gIndex++; return NULL; } +BListItem* +FillExpected(BListItem* item, void* arg) +{ + gExpected[gCount] = item; + gCount++; + return NULL; +} + + +void +CheckItemsUnder(BOutlineListView* view, BListItem* superitem, bool oneLevelOnly) +{ + for (int i = 0; i < gCount; i++) + CPPUNIT_ASSERT_EQUAL(gExpected[i], view->ItemUnderAt(superitem, oneLevelOnly, i)); + + // Check that we don't get more items + CPPUNIT_ASSERT_EQUAL((BListItem*)NULL, view->ItemUnderAt(superitem, oneLevelOnly, gCount)); +} + + class OutlineListViewTest: public TestCase { public: @@ -37,18 +74,210 @@ class OutlineListViewTest: public TestCase OutlineListViewTest(std::string name) : TestCase(name) {} void EachItemUnder(); + void AddUnder(); + void ItemUnderAt(); static Test* Suite(); + + private: + static BOutlineListView* _SetupTest(const char* name); }; -void OutlineListViewTest::EachItemUnder() { - BApplication app( - "application/x-vnd.OutlineListView_EachItemUnder.test"); - BWindow* window = new BWindow(BRect(50,50,550,550), - "OutlineListView_EachItemUnder", B_TITLED_WINDOW, - B_QUIT_ON_WINDOW_CLOSE, 0); - BOutlineListView* view = new BOutlineListView(BRect(5,5,495,495), "View", +void +OutlineListViewTest::EachItemUnder() +{ + BOutlineListView* view = _SetupTest("OutlineListView_EachItemUnder"); + + // First test is easy + gExpected[0] = view->FullListItemAt(6); + gExpected[1] = view->FullListItemAt(8); + gExpected[2] = view->FullListItemAt(9); + gCount = 3; + gIndex = 0; + + fprintf(stderr, "Easy test\n"); + view->EachItemUnder(view->FullListItemAt(5), true, CheckExpected, NULL); + CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(view->FullListItemAt(5), true)); + + // Check that collapsing an item does not change the outcome + gIndex = 0; + view->Collapse(view->FullListItemAt(0)); + + fprintf(stderr, "One collapsed\n"); + view->EachItemUnder(view->FullListItemAt(5), true, CheckExpected, NULL); + + gIndex = 0; + view->Collapse(view->FullListItemAt(5)); + + fprintf(stderr, "Two collapsed\n"); + view->EachItemUnder(view->FullListItemAt(5), true, CheckExpected, NULL); + CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(view->FullListItemAt(5), true)); + + // Also check deeper levels + gExpected[1] = view->FullListItemAt(7); + gExpected[2] = view->FullListItemAt(8); + gExpected[3] = view->FullListItemAt(9); + gCount = 4; + gIndex = 0; + + fprintf(stderr, "All levels\n"); + view->EachItemUnder(view->FullListItemAt(5), false, CheckExpected, NULL); + CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(view->FullListItemAt(5), false)); + + view->Expand(view->FullListItemAt(5)); + view->Collapse(view->FullListItemAt(6)); + gIndex = 0; + + fprintf(stderr, "All levels with a collapsed sublevel\n"); + view->EachItemUnder(view->FullListItemAt(5), false, CheckExpected, NULL); + CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(view->FullListItemAt(5), false)); + + // NULL is the parent of level 0 items + gExpected[0] = view->FullListItemAt(0); + gExpected[1] = view->FullListItemAt(5); + gExpected[2] = view->FullListItemAt(10); + gCount = 3; + gIndex = 0; + + fprintf(stderr, "Level 0\n"); + view->EachItemUnder(NULL, true, CheckExpected, NULL); + CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(NULL, true)); + + // No visits when the item is not in the list + BListItem* notfound = new BStringItem("Not found"); + gCount = 0; + gIndex = 0; + + fprintf(stderr, "Item not in the list\n"); + view->EachItemUnder(notfound, true, CheckExpected, NULL); + CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(notfound, true)); + view->EachItemUnder(notfound, false, CheckExpected, NULL); + CPPUNIT_ASSERT_EQUAL(gCount, view->CountItemsUnder(notfound, false)); + + // Don't actually run anything + delete view->Window(); +} + + +void +OutlineListViewTest::AddUnder() +{ + BOutlineListView* view = _SetupTest("OutlineListView_AddUnder"); + + BListItem* one = view->FullListItemAt(0); + BListItem* oneA = view->FullListItemAt(1); + BListItem* oneA0 = new BStringItem("One-A-0"); + BListItem* oneA1 = view->FullListItemAt(2); + + int32 count = view->FullListCountItems(); + + BListItem* last = view->FullListItemAt(count - 1); + BListItem* newLast = new BStringItem("NewLast"); + + view->AddUnder(newLast, NULL); + view->AddUnder(oneA0, oneA); + + fprintf(stderr, "Count\n"); + CPPUNIT_ASSERT_EQUAL(count + 2, view->FullListCountItems()); + + fprintf(stderr, "Insertion order\n"); + CPPUNIT_ASSERT_EQUAL(one, view->FullListItemAt(0)); + CPPUNIT_ASSERT_EQUAL(oneA, view->FullListItemAt(1)); + CPPUNIT_ASSERT_EQUAL(oneA0, view->FullListItemAt(2)); + CPPUNIT_ASSERT_EQUAL(oneA1, view->FullListItemAt(3)); + CPPUNIT_ASSERT_EQUAL(last, view->FullListItemAt(count)); + CPPUNIT_ASSERT_EQUAL(newLast, view->FullListItemAt(count + 1)); + + fprintf(stderr, "Levels\n"); + CPPUNIT_ASSERT_EQUAL(0, one->OutlineLevel()); + CPPUNIT_ASSERT_EQUAL(1, oneA->OutlineLevel()); + CPPUNIT_ASSERT_EQUAL(2, oneA0->OutlineLevel()); + CPPUNIT_ASSERT_EQUAL(2, oneA1->OutlineLevel()); + CPPUNIT_ASSERT_EQUAL(0, newLast->OutlineLevel()); + + // Don't actually run anything + delete view->Window(); +} + + +void +OutlineListViewTest::ItemUnderAt() +{ + BOutlineListView* view = _SetupTest("OutlineListView_ItemUnderAt"); + + // EachItemUnder has already been checked, we can use it to know what to expect + gCount = 0; + view->EachItemUnder(view->FullListItemAt(5), true, FillExpected, NULL); + + fprintf(stderr, "Easy test\n"); + CheckItemsUnder(view, view->FullListItemAt(5), true); + + // Check that collapsing an item does not change the outcome + view->Collapse(view->FullListItemAt(0)); + + fprintf(stderr, "One collapsed\n"); + CheckItemsUnder(view, view->FullListItemAt(5), true); + + view->Collapse(view->FullListItemAt(5)); + + fprintf(stderr, "Two collapsed\n"); + CheckItemsUnder(view, view->FullListItemAt(5), true); + + // Also check deeper levels + gCount = 0; + view->EachItemUnder(view->FullListItemAt(5), false, FillExpected, NULL); + + fprintf(stderr, "All levels\n"); + CheckItemsUnder(view, view->FullListItemAt(5), false); + + view->Expand(view->FullListItemAt(5)); + view->Collapse(view->FullListItemAt(6)); + + fprintf(stderr, "All levels with a collapsed sublevel\n"); + CheckItemsUnder(view, view->FullListItemAt(5), false); + + // NULL is the parent of level 0 items + gCount = 0; + view->EachItemUnder(NULL, true, FillExpected, NULL); + + fprintf(stderr, "Level 0\n"); + CheckItemsUnder(view, NULL, true); + + // Get NULL when the item is not in the list + BListItem* notfound = new BStringItem("Not found"); + fprintf(stderr, "Item not in the list\n"); + CPPUNIT_ASSERT_EQUAL((BListItem*)NULL, view->ItemUnderAt(notfound, true, 0)); + CPPUNIT_ASSERT_EQUAL((BListItem*)NULL, view->ItemUnderAt(notfound, false, 0)); + + // Don't actually run anything + delete view->Window(); +} + + +Test* +OutlineListViewTest::Suite() +{ + TestSuite* SuiteOfTests = new TestSuite; + + ADD_TEST4(BOutlineListView, SuiteOfTests, OutlineListViewTest, EachItemUnder); + ADD_TEST4(BOutlineListView, SuiteOfTests, OutlineListViewTest, AddUnder); + ADD_TEST4(BOutlineListView, SuiteOfTests, OutlineListViewTest, ItemUnderAt); + + return SuiteOfTests; +} + + +BOutlineListView* +OutlineListViewTest::_SetupTest(const char* name) +{ + if (be_app == NULL) + new BApplication("application/x-vnd.OutlineListView.test"); + + BWindow* window = new BWindow(BRect(50, 50, 550, 550), name, + B_TITLED_WINDOW, B_QUIT_ON_WINDOW_CLOSE, 0); + + BOutlineListView* view = new BOutlineListView(BRect(5, 5, 495, 495), "View", B_MULTIPLE_SELECTION_LIST, B_FOLLOW_ALL); window->AddChild(view); @@ -70,42 +299,7 @@ void OutlineListViewTest::EachItemUnder() { view->AddItem(new BStringItem("Three-B", 1)); view->AddItem(new BStringItem("Three-C", 1)); - // First test is easy - gExpected[0] = view->FullListItemAt(6); - gExpected[1] = view->FullListItemAt(8); - gExpected[2] = view->FullListItemAt(9); - gCount = 3; - gIndex = 0; - - fprintf(stderr, "Easy test\n"); - view->EachItemUnder(view->FullListItemAt(5), true, eachitemunder, NULL); - - // Check that collapsing an item does not change the outcome - gIndex = 0; - view->Collapse(view->FullListItemAt(0)); - - fprintf(stderr, "One collapsed\n"); - view->EachItemUnder(view->FullListItemAt(5), true, eachitemunder, NULL); - - gIndex = 0; - view->Collapse(view->FullListItemAt(5)); - - fprintf(stderr, "Two collapsed\n"); - view->EachItemUnder(view->FullListItemAt(5), true, eachitemunder, NULL); - - // Don't actually run anything - delete window; -} - - -Test* OutlineListViewTest::Suite() -{ - TestSuite* SuiteOfTests = new TestSuite; - - ADD_TEST4(BOutlineListView, SuiteOfTests, OutlineListViewTest, - EachItemUnder); - - return SuiteOfTests; + return view; }