Bug 325012 - Playlist reordering works only in one direction
Summary: Playlist reordering works only in one direction
Status: RESOLVED FIXED
Alias: None
Product: plasma-mediacenter
Classification: Unclassified
Component: Playlist (show other bugs)
Version: 1.1.0
Platform: unspecified Linux
: NOR major (vote)
Target Milestone: ---
Assignee: Sinny Kumari
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-17 17:38 UTC by Shantanu Tushar
Modified: 2014-03-06 14:20 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
this is a patch to the /plasma-media-center/libs/mediacenter/playlistmodel.cpp (947 bytes, patch)
2013-09-19 15:31 UTC, kaushik varanasi
Details
playlistmodel.cpp (7.60 KB, text/plain)
2013-09-19 15:37 UTC, kaushik varanasi
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Shantanu Tushar 2013-09-17 17:38:17 UTC
Playlist reordering works only if you press-n-hold, drag and drop a media item downwards. If you do so upwards, the order remains the same.

Reproducible: Always

Steps to Reproduce:
1. Add more than 2 songs to playlist

song1
song2
song3

2. Drag song1 below song2

Actual Results:  
song1 remains in its original position

Expected Results:  
song1 should go below song2 and the order should become

song2
song1
song3
Comment 1 kaushik varanasi 2013-09-19 15:31:10 UTC
Created attachment 82410 [details]
this is a patch to the /plasma-media-center/libs/mediacenter/playlistmodel.cpp

the drag and drop feature worked only in one direction. I fixed the bug.
now the drag and drop feature is complete.
Comment 2 Shantanu Tushar 2013-09-19 15:34:51 UTC
Thanks for the patch. However, in the following code-

 void PlaylistModel::moveItem(int firstIndex, int secondIndex)
 {
+	beginResetModel();
     d->musicList.move(firstIndex,secondIndex);
+    endResetModel();
     emit dataChanged(createIndex(firstIndex, 0), createIndex(secondIndex, 0));
 }

the call to d->musicList.move is there to avoid calling beginResetModel() and endResetModel(). This is because reset'ing a model is expensive, and instead we should only call move.

My guess is that during upwards, the move() method works fine, but dataChanged() is somehow not working. Try to figure out why (maybe look at the values passed to dataChanged and whats the difference between upwards and downwards).
Comment 3 kaushik varanasi 2013-09-19 15:37:32 UTC
Created attachment 82411 [details]
playlistmodel.cpp

in case my patch dosnt work, replace the /plasma-media-center/libs/mediacenter/playlistmodel.cpp file with this.


PS: as i said i am very new to open source so please excuse my amature behavior.
Comment 4 Shantanu Tushar 2013-09-23 15:28:26 UTC
Hey kaushik,
So as I mentioned, don't use reset and instead use dataChanged (as its already there). I think the problem is that lets say you pass firstIndex, secondIndex to dataChanged, it works only if firstIndex < secondIndex. I suspect that when reordering upwards, firstIndex becomes greater than secondIndex and hence the problem. Try putting a if condition when this happens and in that case reverse the order of firstIndex and secondIndex when calling dataChanged.

If it works, instead of adding a patch here, create a new review request at http://git.reviewboard.kde.org/ and upload your patch there. Add "plasma" in the group and publish your review. Then we can take a look at your patch :)
Comment 5 Shantanu Tushar 2014-03-06 14:20:36 UTC
Git commit 86f20ec9a746efb0add1de56e44bc5064de45a53 by Shantanu Tushar.
Committed on 06/03/2014 at 14:18.
Pushed by shantanu into branch 'master'.

Make playlist reordering work both the ways.

Patch by Ashish Madeti <ashishmadeti@gmail.com>
Welcome to KDE :)
REVIEW: 116626

M  +5    -1    libs/mediacenter/playlistmodel.cpp

http://commits.kde.org/plasma-mediacenter/86f20ec9a746efb0add1de56e44bc5064de45a53