Review Request: Another approach to fix bug 291068, be more permissive

Bart Cerneels bart.cerneels at kde.org
Tue Feb 7 08:57:06 UTC 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103856/#review10387
-----------------------------------------------------------

Ship it!


I just tried it and it works perfectly. Just commit it and we'll fix any problems that come up.

One thing though: from my experience you should be very careful with hackish things in Qt ItemModels. They interact with the view in complex, non-transparent ways that are even subject to change in the future. Those regressions are the worst to debug.

- Bart Cerneels


On Feb. 3, 2012, 11:58 a.m., Matěj Laitl wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103856/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2012, 11:58 a.m.)
> 
> 
> Review request for Amarok and Bart Cerneels.
> 
> 
> Description
> -------
> 
> Another approach to fix bug 291068, be more permissive
> 
> Bart originally solved the bug by enabling track dropping only
> precisely on root collection rows. This is IMO too much restrictive
> as it prevents you to drag tracks between 2 large expanded collections
> without excessive scrolling. (something I would miss) Additionally,
> there was no visual indication that the drop will not be performed.
> 
> This is my try to rework it in a way that:
>  * keeps drops onto artists/albums (whatever you first level
>    entity in collection browser is) allowed. There is a drop indicator
>    that clearly shows that the drop will go _between_ the entities, not
>    to them.
>  * disables drops to read-only collections
>  * disabled drops are indicated visually using the not-allowed mouse
>    cursor (the tricky part, but commented well in code)
> 
> BUG: 291068
> 
> 
> This addresses bug 291068.
>     https://bugs.kde.org/show_bug.cgi?id=291068
> 
> 
> Diffs
> -----
> 
>   ChangeLog ed4e955deeebf6a506e87ca8973645b9eeadc67b 
>   src/browsers/CollectionTreeItemModel.cpp d2bab082f44b37fa8945c827006439e7deb0017e 
>   src/browsers/CollectionTreeItemModelBase.h c6cca46fff956f9123299cb29c81e6a792abbc3b 
>   src/browsers/CollectionTreeItemModelBase.cpp e7f8e620a0ae2f7546c879dfd5d67a13fcb0f34a 
>   src/browsers/CollectionTreeView.h b70f99d811d1f7f271ec79298c3b0140fbd0ede4 
>   src/browsers/CollectionTreeView.cpp c9069c770fd28fe16e76b1af132f3c7dff4c82d3 
> 
> Diff: http://git.reviewboard.kde.org/r/103856/diff/diff
> 
> 
> Testing
> -------
> 
> Works as described here.
> 
> 
> Thanks,
> 
> Matěj Laitl
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120207/5c7fa53c/attachment.html>


More information about the Amarok-devel mailing list