Code review comment for lp:~osomon/unity-2d/dnd-reorder-apps

Revision history for this message
Ugo Riboni (uriboni) wrote :

I noticed that the DND code could be much better separated from everything else (this was something I missed in my original review, to be honest).

Essentially there's a MouseArea with id "dnd" that is defined in Launcher.qml and it is referenced in many places, including LauncherItem.qml and AutoScrollListView.qml
While this works ok since QML scoping rules are pretty loose, in my opinion it is abusing a little bit too much this looseness, and makes it harder for people to figure out where this 'dnd' component is defined and change things later.

I made a branch (based on the branch in this MR) where the DND code is pretty much entirely separated and where we need it in other components to workaround QT issues is more clearly explained by comments. You can find it at: lp:~uriboni/unity-2d/separate-dnd-code
I'm not sure if it's 100% working but after some testing I have not identified any regressions.
Can you please have a look and let me know if you think it makes sense to change your code in that way or a similar one ?

« Back to merge proposal