Skip to content
This repository has been archived by the owner on Sep 26, 2022. It is now read-only.

WIP: implement cross category drag and drop #424

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

0x-r4bbit
Copy link
Member

I think UI wise, this seems to be pretty much done.

To allow drag and drop into and out of categories we need to switch to a different approach:

  1. No more use of DelegateModel. The reason DelegateModel was used in the first place, was so that the chat list can be easily visually updated as the user is dragging an item over the list (using its .move() API). Because dragged items could origin from a different category (/chat list), there's no benefit to using DelegateModel anymore
  2. Removing DelegateModel also eliminates the warnings reported in [StatusQ] bug: expanding a category produces warning status-desktop#7572 which was significantly slowing down the rendering performance.
  3. Instead of having items slide up and down, we now just add a "drop indicator" between items. This makes the whole implementation easier and more predictable.
  4. There's another DropArea added to StatusChatList which is activated when a chat list is empty. This is needed so users can drag chat items into empty categories or even just out of categories into "uncategorized" lists
  5. Dragged elements now keep track of not only their new position but also their original position (previous orignalOrder was just assigned the new position. We now have newOrder for that. Should probably rename both to originalPosition and newPosition respectively`).
  6. chatItemReordered signal no longer just emits the same categoryId in which the dragged item resides in, but rather its newCategoryId, in case it was dragged into a different category. Status-go is smart enough to recognize that and moves the chat item into the new category accordingly

What's missing:

Proper visual state update after the drop has happened but before the signal was sent to the backend.

width: 288

property string uuid: Utils.uuid()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can get rid off this one as it's no longer used

@@ -29,199 +31,262 @@ Column {

signal chatItemSelected(string id)
signal chatItemUnmuted(string id)
signal chatItemReordered(string id, int from, int to)
signal chatItemReordered(string categoryId, string id, int from, int to)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change and needs to be noted.

Column {
id: statusChatListItemsWrapper
width: parent.width
spacing: 4
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This column is needed to get a sense of how many items live in this chatlist.
At the moment, we always pass every chat item to every chat list and hide them if they don't belong to the corresponding category. This means, the model has never a count: 0 so we have to rely on the implicitHeight of the Repeater that renders them.

We can't use the root Column for that because we have other elements living inside of it.

originalOrder: model.position
chatId: model.chatId || model.id
categoryId: model.categoryId || ""
/* name: (!!statusChatList.chatNameFn ? statusChatList.chatNameFn(model) : model.name) + " POSITION: " + statusChatListItem.newOrder */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be removed

onDropped: statusChatList.chatItemReordered(statusChatListItem.chatId, drag.source.originalOrder, statusChatListItem.DelegateModel.itemsIndex)
Loader {
id: emptyListDropAreaLoader
active: statusChatListItemsWrapper.implicitHeight == 0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be enough. When the wrapper gets an implicitHeight at runtime, the loader is still active. Hence, we're updating active explicityly in onImplicitHeightChanged

StatusChatListDropIndicator {
anchors.bottom: parent.bottom
anchors.bottomMargin: -2
visible: dropArea.containsDrag
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misses opacity: 0.5

console.log("FROM CATEGORY: ", model.categoryId)
console.log("TO: ", categoryId)
console.log("FROM POSITION: ", from)
console.log("TO POSITION: ", to)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously these need to be removed. Leaving them for now for debugging purpose. Will go away before merge

@@ -8,6 +9,7 @@ Column {

spacing: 0
opacity: dragged ? 0.5 : 1
property string uuid: Utils.uuid()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be removed as it's no longer used. Left over from building/testing

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant