Review Request: bug fix : add DropMimeData in KDescendantsProxyModel

[Stephen Kelly]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6035/#review9115
-----------------------------------------------------------


It wasn't so much forgotten as omitted because of complexity and weird UI.

Drops can be released directly on an item, or between two items.

If a drop on a kdescendantsproxymodel is released on an item it is reasonably clear that the dropped data should be made a child of the item it is dropped on.

In that case, dropMimeData is called with row == -1, column == -1 and parent == the drop target.

If a drop on a kdescendantsproxymodel is released between two items it is not clear where it should be invoked in the source model.

Consider this source tree:

- A
- - B
- - C
- D
- E

If I take E and drag it onto A, it becomes a sibling of C. Fair enough. If I instead drop it between A and B I'll get a visual indicator of a horizontal line indicating whether it is to be dropped below A (result is it goes above D as a sibling of A), or above B (result is it goes above B as a first child of A).

In general, if I drop between two adjacent items the result will be that the new (or moved) item will appear between them.

Consider what would happen in the proxy model case:

- A
- B
- C
- D
- E

Dropping E between A and B could result in E being between A and B (and change the structure of the source model such that E is a child of A (Weird - not what the user did), or it would appear between C and D as a child of the parent of A (Weird - not where the user dropped it, and it also changes the structure of the source model, which is not what the user is looking at).

Even dropping on the target itself would be weird. If you drop E on B, it becomes a child of B in the source model, but it appears as the next sibling of B in the proxy. Therefore it teaches the user that if they want E to appear below a certain item, they should drop it on that item (the user doesn't see the changes to the source model, except maybe somewhere else in the application).

KDescendantsProxyModel is a bit of a special case in this regard because it completely changes the structural representation of the source model. dropMimeData also changes the structure of the model, so this method is incompatible with this class.

Finally, I couldn't think of any way to use the class where you would want to do this, so I just left it out.

So I'm -1 on this patch.

Thanks all the same. It's good to know other people are watching the code I'm writing to try to make it better.

Steve.

PS:
I've just read the dropMimeData doc entry, and it's wrong and misleading. Don't rely on it too much. It is not the responsibility of the view to decide where things go in the model for one thing.

- Stephen


On 2010-12-03 16:37:47, Mario Bensi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6035/
> -----------------------------------------------------------
> 
> (Updated 2010-12-03 16:37:47)
> 
> 
> Review request for kdelibs and Stephen Kelly.
> 
> 
> Summary
> -------
> 
> bug fix : add a forgotten method in KDescendantsProxyModel to forward the DropMimeData to the sourceModel
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdeui/itemviews/kdescendantsproxymodel.h 1203163 
>   trunk/KDE/kdelibs/kdeui/itemviews/kdescendantsproxymodel.cpp 1203318 
> 
> Diff: http://svn.reviewboard.kde.org/r/6035/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mario
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20101203/d45fe2c5/attachment.htm>