EXP-1439 : Modify DaD on the Favorite toolbar so that reordering visible landmarks don't send any back to the end of the list

4 files changed, 86 insertions, 42 deletions

diff --git a/indra/newview/llfavoritesbar.cpp b/indra/newview/llfavoritesbar.cpp
index 98de418878..4e0c838454 100644
--- a/indra/newview/llfavoritesbar.cpp
+++ b/indra/newview/llfavoritesbar.cpp
@@ -504,20 +504,29 @@ BOOL LLFavoritesBarCtrl::handleDragAndDrop(S32 x, S32 y, MASK mask, BOOL drop,
 
 void LLFavoritesBarCtrl::handleExistingFavoriteDragAndDrop(S32 x, S32 y)
 {
+	// Identify the button hovered and the side to drop
 	LLFavoriteLandmarkButton* dest = dynamic_cast<LLFavoriteLandmarkButton*>(mLandingTab);
+	bool insert_before = true;	
+	if (!dest)
+	{
+		insert_before = false;
+		dest = dynamic_cast<LLFavoriteLandmarkButton*>(mLastTab);
+	}
 
-	// there is no need to handle if an item was dragged onto itself
+	// There is no need to handle if an item was dragged onto itself
 	if (dest && dest->getLandmarkId() == mDragItemId)
 	{
 		return;
 	}
 
+	// Insert the dragged item in the right place
 	if (dest)
 	{
-		LLInventoryModel::updateItemsOrder(mItems, mDragItemId, dest->getLandmarkId());
+		LLInventoryModel::updateItemsOrder(mItems, mDragItemId, dest->getLandmarkId(), insert_before);
 	}
 	else
 	{
+		// This can happen when the item list is empty
 		mItems.push_back(gInventory.getItem(mDragItemId));
 	}
 
@@ -534,22 +543,31 @@ void LLFavoritesBarCtrl::handleExistingFavoriteDragAndDrop(S32 x, S32 y)
 
 void LLFavoritesBarCtrl::handleNewFavoriteDragAndDrop(LLInventoryItem *item, const LLUUID& favorites_id, S32 x, S32 y)
 {
+	// Identify the button hovered and the side to drop
 	LLFavoriteLandmarkButton* dest = dynamic_cast<LLFavoriteLandmarkButton*>(mLandingTab);
-
-	// there is no need to handle if an item was dragged onto itself
+	bool insert_before = true;	
+	if (!dest)
+	{
+		insert_before = false;
+		dest = dynamic_cast<LLFavoriteLandmarkButton*>(mLastTab);
+	}
+	
+	// There is no need to handle if an item was dragged onto itself
 	if (dest && dest->getLandmarkId() == mDragItemId)
 	{
 		return;
 	}
-
+	
 	LLPointer<LLViewerInventoryItem> viewer_item = new LLViewerInventoryItem(item);
 
+	// Insert the dragged item in the right place
 	if (dest)
 	{
-		insertBeforeItem(mItems, dest->getLandmarkId(), viewer_item);
+		insertItem(mItems, dest->getLandmarkId(), viewer_item, insert_before);
 	}
 	else
 	{
+		// This can happen when the item list is empty
 		mItems.push_back(viewer_item);
 	}
 
@@ -1337,29 +1355,28 @@ BOOL LLFavoritesBarCtrl::needToSaveItemsOrder(const LLInventoryModel::item_array
 	return result;
 }
 
-LLInventoryModel::item_array_t::iterator LLFavoritesBarCtrl::findItemByUUID(LLInventoryModel::item_array_t& items, const LLUUID& id)
+void LLFavoritesBarCtrl::insertItem(LLInventoryModel::item_array_t& items, const LLUUID& dest_item_id, LLViewerInventoryItem* insertedItem, bool insert_before)
 {
-	LLInventoryModel::item_array_t::iterator result = items.end();
+	// Get the iterator to the destination item
+	LLInventoryModel::item_array_t::iterator it_dest = LLInventoryModel::findItemIterByUUID(items, dest_item_id);
+	if (it_dest == items.end())
+		return;
 
-	for (LLInventoryModel::item_array_t::iterator i = items.begin(); i != items.end(); ++i)
+	// Go to the next element if one wishes to insert after the dest element
+	if (!insert_before)
 	{
-		if ((*i)->getUUID() == id)
-		{
-			result = i;
-			break;
-		}
+		++it_dest;
 	}
-
-	return result;
-}
-
-void LLFavoritesBarCtrl::insertBeforeItem(LLInventoryModel::item_array_t& items, const LLUUID& beforeItemId, LLViewerInventoryItem* insertedItem)
-{
-	LLViewerInventoryItem* beforeItem = gInventory.getItem(beforeItemId);
-	llassert(beforeItem);
-	if (beforeItem)
+	
+	// Insert the source item in the right place
+	if (it_dest != items.end())
+	{
+		items.insert(it_dest, insertedItem);
+	}
+	else 
 	{
-		items.insert(findItemByUUID(items, beforeItem->getUUID()), insertedItem);
+		// Append to the list if it_dest reached the end
+		items.push_back(insertedItem);
 	}
 }
 
diff --git a/indra/newview/llfavoritesbar.h b/indra/newview/llfavoritesbar.h
index 1b11d6196e..2f75b3bb0e 100644
--- a/indra/newview/llfavoritesbar.h
+++ b/indra/newview/llfavoritesbar.h
@@ -130,7 +130,7 @@ private:
 	 * inserts an item identified by insertedItemId BEFORE an item identified by beforeItemId.
 	 * this function assumes that an item identified by insertedItemId doesn't exist in items array.
 	 */
-	void insertBeforeItem(LLInventoryModel::item_array_t& items, const LLUUID& beforeItemId, LLViewerInventoryItem* insertedItem);
+	void insertItem(LLInventoryModel::item_array_t& items, const LLUUID& dest_item_id, LLViewerInventoryItem* insertedItem, bool insert_before);
 
 	// finds an item by it's UUID in the items array
 	LLInventoryModel::item_array_t::iterator findItemByUUID(LLInventoryModel::item_array_t& items, const LLUUID& id);
diff --git a/indra/newview/llinventorymodel.cpp b/indra/newview/llinventorymodel.cpp
index e86c427ae2..fb02fe0ff7 100644
--- a/indra/newview/llinventorymodel.cpp
+++ b/indra/newview/llinventorymodel.cpp
@@ -2879,40 +2879,62 @@ BOOL LLInventoryModel::getIsFirstTimeInViewer2()
 	return sFirstTimeInViewer2;
 }
 
-static LLInventoryModel::item_array_t::iterator find_item_iter_by_uuid(LLInventoryModel::item_array_t& items, const LLUUID& id)
+LLInventoryModel::item_array_t::iterator LLInventoryModel::findItemIterByUUID(LLInventoryModel::item_array_t& items, const LLUUID& id)
 {
-	LLInventoryModel::item_array_t::iterator result = items.end();
+	LLInventoryModel::item_array_t::iterator curr_item = items.begin();
 
-	for (LLInventoryModel::item_array_t::iterator i = items.begin(); i != items.end(); ++i)
+	while (curr_item != items.end())
 	{
-		if ((*i)->getUUID() == id)
+		if ((*curr_item)->getUUID() == id)
 		{
-			result = i;
 			break;
 		}
+		++curr_item;
 	}
 
-	return result;
+	return curr_item;
 }
 
 // static
 // * @param[in, out] items - vector with items to be updated. It should be sorted in a right way
 // * before calling this method.
 // * @param src_item_id - LLUUID of inventory item to be moved in new position
-// * @param dest_item_id - LLUUID of inventory item before which source item should be placed.
-void LLInventoryModel::updateItemsOrder(LLInventoryModel::item_array_t& items, const LLUUID& src_item_id, const LLUUID& dest_item_id)
+// * @param dest_item_id - LLUUID of inventory item before (or after) which source item should 
+// * be placed.
+// * @param insert_before - bool indicating if src_item_id should be placed before or after 
+// * dest_item_id. Default is true.
+void LLInventoryModel::updateItemsOrder(LLInventoryModel::item_array_t& items, const LLUUID& src_item_id, const LLUUID& dest_item_id, bool insert_before)
 {
-	LLInventoryModel::item_array_t::iterator it_src = find_item_iter_by_uuid(items, src_item_id);
-	LLInventoryModel::item_array_t::iterator it_dest = find_item_iter_by_uuid(items, dest_item_id);
+	LLInventoryModel::item_array_t::iterator it_src = findItemIterByUUID(items, src_item_id);
+	LLInventoryModel::item_array_t::iterator it_dest = findItemIterByUUID(items, dest_item_id);
 
-	if (it_src == items.end() || it_dest == items.end()) return;
+	// If one of the passed UUID is not in the item list, bail out
+	if ((it_src == items.end()) || (it_dest == items.end())) 
+		return;
 
+	// Erase the source element from the list, keep a copy before erasing.
 	LLViewerInventoryItem* src_item = *it_src;
 	items.erase(it_src);
 	
-	// target iterator can not be valid because the container was changed, so update it.
-	it_dest = find_item_iter_by_uuid(items, dest_item_id);
-	items.insert(it_dest, src_item);
+	// Note: Target iterator is not valid anymore because the container was changed, so update it.
+	it_dest = findItemIterByUUID(items, dest_item_id);
+	
+	// Go to the next element if one wishes to insert after the dest element
+	if (!insert_before)
+	{
+		++it_dest;
+	}
+	
+	// Reinsert the source item in the right place
+	if (it_dest != items.end())
+	{
+		items.insert(it_dest, src_item);
+	}
+	else 
+	{
+		// Append to the list if it_dest reached the end
+		items.push_back(src_item);
+	}
 }
 
 //* @param[in] items vector of items in order to be saved.
diff --git a/indra/newview/llinventorymodel.h b/indra/newview/llinventorymodel.h
index e0e81f1006..a0fd455cf3 100644
--- a/indra/newview/llinventorymodel.h
+++ b/indra/newview/llinventorymodel.h
@@ -332,11 +332,16 @@ protected:
 	//--------------------------------------------------------------------
 public:
 	// Changes items order by insertion of the item identified by src_item_id
-	// before the item identified by dest_item_id. Both items must exist in items array.
-	// Sorting is stored after method is finished. Only src_item_id is moved before dest_item_id.
+	// before (or after) the item identified by dest_item_id. Both items must exist in items array.
+	// Sorting is stored after method is finished. Only src_item_id is moved before (or after) dest_item_id.
+	// The parameter "insert_before" controls on which side of dest_item_id src_item_id gets rensinserted.
 	static void updateItemsOrder(LLInventoryModel::item_array_t& items, 
 								 const LLUUID& src_item_id, 
-								 const LLUUID& dest_item_id);
+								 const LLUUID& dest_item_id,
+								 bool insert_before = true);
+	// Gets an iterator on an item vector knowing only the item UUID.
+	// Returns end() of the vector if not found.
+	static LLInventoryModel::item_array_t::iterator findItemIterByUUID(LLInventoryModel::item_array_t& items, const LLUUID& id);
 
 	// Saves current order of the passed items using inventory item sort field.
 	// Resets 'items' sort fields and saves them on server.