Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review60666 --- Looks good to me, just one suggestion mediaelements/mediacontroller/MediaController.qml https://git.reviewboard.kde.org/r/116874/#comment42305 runtimeData.playing || runtimeData.paused should work better, thats usually the case in other media players - Shantanu Tushar On June 19, 2014, 9:04 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 19, 2014, 9:04 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
On June 21, 2014, 1:59 p.m., Shantanu Tushar wrote: mediaelements/mediacontroller/MediaController.qml, line 100 https://git.reviewboard.kde.org/r/116874/diff/9/?file=282642#file282642line100 runtimeData.playing || runtimeData.paused should work better, thats usually the case in other media players Yes, that was necessary. Thanks ! Updating the diff right away :) - R.Harish --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review60666 --- On June 19, 2014, 9:04 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 19, 2014, 9:04 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 21, 2014, 3:55 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- added the runtimeData.paused condition also. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs (updated) - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review60669 --- Ship it! Ship It! - Shantanu Tushar On June 21, 2014, 3:55 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 21, 2014, 3:55 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review60670 --- This review has been submitted with commit fec9938bcef6dd884720008d9577aea18c82cf99 by Shantanu Tushar on behalf of R. Harish Navnit to branch master. - Commit Hook On June 21, 2014, 3:55 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 21, 2014, 3:55 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 21, 2014, 3:58 p.m.) Status -- This change has been marked as submitted. Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review60674 --- This review has been submitted with commit 97e717aae08e03fa4fa7ada024b0f6dfa019c6d4 by Shantanu Tushar on behalf of R. Harish Navnit to branch vsrao-seriesbackend. - Commit Hook On June 21, 2014, 3:58 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 21, 2014, 3:58 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated June 19, 2014, 9:04 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- After the latest commit the issue 330990 seems to have been solved. This patch just enables/disables the buttons when required. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs (updated) - mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated April 19, 2014, 12:44 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- With the changes in the patch, the next and previous buttons get disabled once a playlist is changed and remain disabled even after going back to the playlist playing the current song. How can this be resolved ? P.S: I've also disabled the youtube subfolder build since the build was failing on my system with it. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs (updated) - mediaelements/playlist/Playlist.qml 5dde297 mediaelements/mediacontroller/MediaController.qml 2fce0a0 browsingbackends/onlineservices/CMakeLists.txt 6473437 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review56054 --- browsingbackends/onlineservices/CMakeLists.txt https://git.reviewboard.kde.org/r/116874/#comment39122 This should not be in diff. Refer to some git tutorial on how to get only some changes in diff - Bhushan Shah On April 19, 2014, 6:14 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated April 19, 2014, 6:14 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/playlist/Playlist.qml 5dde297 mediaelements/mediacontroller/MediaController.qml 2fce0a0 browsingbackends/onlineservices/CMakeLists.txt 6473437 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated April 19, 2014, 2:02 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- Removed the comment in the previous patch. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs (updated) - mediaelements/playlist/Playlist.qml 5dde297 mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated April 19, 2014, 3:37 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- Removed the comment. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs (updated) - mediaelements/playlist/Playlist.qml 5dde297 mediaelements/mediacontroller/MediaController.qml 2fce0a0 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
On April 14, 2014, 12:58 p.m., Sinny Kumari wrote: checking only url of runtimeData will work only when PMC is launched and nothing has been played. You need to handle many other cases like when media get stopped, next/prev shouldn't be enabled until and unless current media is getting played from playlist etc I am checking the playlistModel.currentIndex != -1 condition for the next and previous buttons. For the rest, I think checking runtimeData.url works fine. But with this, next and prev get disabled when a new playlist is selected, and do not get enabled unless a song from that playlist is selected. Expected behavior ? - R.Harish --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review55718 --- On April 12, 2014, 3:23 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated April 12, 2014, 3:23 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 mediaelements/playlist/Playlist.qml 5dde297 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review55718 --- checking only url of runtimeData will work only when PMC is launched and nothing has been played. You need to handle many other cases like when media get stopped, next/prev shouldn't be enabled until and unless current media is getting played from playlist etc mediaelements/mediacontroller/MediaController.qml https://git.reviewboard.kde.org/r/116874/#comment38784 you can directly write enabled: runtimeData.url - Sinny Kumari On April 12, 2014, 3:23 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated April 12, 2014, 3:23 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/mediacontroller/MediaController.qml 2fce0a0 mediaelements/playlist/Playlist.qml 5dde297 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
On March 27, 2014, 1:05 p.m., Sebastian Kügler wrote: If a button doesn't do anything, it should be indicated by disabling it, not just making it a no-op. Hey, Should I disable the other buttons like the play/pause, mediascroller, currentmediatime etc ? I mean when there's no media playing, these should be inactive too. I already have a patch that does the above mentioned, I'll upload it for review soon :) - R.Harish --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review54293 --- On March 28, 2014, 12:39 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 28, 2014, 12:39 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/playlist/Playlist.qml 5dde297 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 28, 2014, 12:39 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- Updated the diff, instead of adding the patch as a new file. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs (updated) - mediaelements/playlist/Playlist.qml 5dde297 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 28, 2014, 12:37 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- Removed the new diff file. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/playlist/Playlist.qml fd83c21 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review54431 --- See inline comment about line breaks. Also, the disabling of the buttons isn't in this patch. For reviewers, it's more convenient if you fix all the issues pointed out, and then upload a new patch. mediaelements/playlist/Playlist.qml https://git.reviewboard.kde.org/r/116874/#comment38086 brackets go on the same line as the if statement - Sebastian Kügler On March 28, 2014, 12:39 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 28, 2014, 12:39 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/playlist/Playlist.qml 5dde297 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 27, 2014, 11:22 a.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- Since I was having troubles building from the latest source, I started off again from a fresh clone and made changes which I believe fix the issue. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - mediaelements/playlist/Playlist.qml fd83c21 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. File Attachments (updated) pulled the latest source and made changes https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116874/#review54293
---
If a button doesn't do anything, it should be indicated by disabling it, not
just making it a no-op.
mediaelements/playlist/Playlist.qml
https://git.reviewboard.kde.org/r/116874/#comment37984
Add { } to the if statement, it makes the code easier to read, and prevents
errors when people insert a line without adjusting {}
It's also Plasma's recommended coding style.
mediaelements/playlist/Playlist.qml
https://git.reviewboard.kde.org/r/116874/#comment37985
indenting is wrong now
- Sebastian Kügler
On March 27, 2014, 11:22 a.m., R.Harish Navnit wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116874/
---
(Updated March 27, 2014, 11:22 a.m.)
Review request for Plasma, Shantanu Tushar and Sinny Kumari.
Bugs: 330990 and Bug
http://bugs.kde.org/show_bug.cgi?id=330990
http://bugs.kde.org/show_bug.cgi?id=Bug
Repository: plasma-mediacenter
Description
---
Now the next and previous buttons do not work unless a media is playing.
Diffs
-
mediaelements/playlist/Playlist.qml fd83c21
Diff: https://git.reviewboard.kde.org/r/116874/diff/
Testing
---
I've done the testing for this patch.
These are the steps that I followed.
1. Added a new playlist and added a few songs to the playlist.
2. Re-opened Plasma Media Center
3. Clicked the Next and Previous buttons.
4. Nothing was played.
5. Selected a song from the list and then tested the next and previous
buttons.
6. The buttons worked seamlessly.
File Attachments
pulled the latest source and made changes
https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch
Thanks,
R.Harish Navnit
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
On March 27, 2014, 1:05 p.m., Sebastian Kügler wrote:
mediaelements/playlist/Playlist.qml, line 159
https://git.reviewboard.kde.org/r/116874/diff/3/?file=257374#file257374line159
indenting is wrong now
I have added a new diff file after I pulled the latest source and built
successfully. Please do check that
patch(https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch),
I've added {} to the if-statement in that one. It however, isn't disabled when
the playlist isn't active.
- R.Harish
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116874/#review54293
---
On March 27, 2014, 11:22 a.m., R.Harish Navnit wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116874/
---
(Updated March 27, 2014, 11:22 a.m.)
Review request for Plasma, Shantanu Tushar and Sinny Kumari.
Bugs: 330990 and Bug
http://bugs.kde.org/show_bug.cgi?id=330990
http://bugs.kde.org/show_bug.cgi?id=Bug
Repository: plasma-mediacenter
Description
---
Now the next and previous buttons do not work unless a media is playing.
Diffs
-
mediaelements/playlist/Playlist.qml fd83c21
Diff: https://git.reviewboard.kde.org/r/116874/diff/
Testing
---
I've done the testing for this patch.
These are the steps that I followed.
1. Added a new playlist and added a few songs to the playlist.
2. Re-opened Plasma Media Center
3. Clicked the Next and Previous buttons.
4. Nothing was played.
5. Selected a song from the list and then tested the next and previous
buttons.
6. The buttons worked seamlessly.
File Attachments
pulled the latest source and made changes
https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch
Thanks,
R.Harish Navnit
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
On March 27, 2014, 1:05 p.m., Sebastian Kügler wrote:
mediaelements/playlist/Playlist.qml, line 159
https://git.reviewboard.kde.org/r/116874/diff/3/?file=257374#file257374line159
indenting is wrong now
R.Harish Navnit wrote:
I have added a new diff file after I pulled the latest source and built
successfully. Please do check that
patch(https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch),
I've added {} to the if-statement in that one. It however, isn't disabled
when the playlist isn't active.
Can you update the diff in the review instead of attaching it? Otherwise its
not possible to leave comments on code.
- Shantanu
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116874/#review54293
---
On March 27, 2014, 11:22 a.m., R.Harish Navnit wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116874/
---
(Updated March 27, 2014, 11:22 a.m.)
Review request for Plasma, Shantanu Tushar and Sinny Kumari.
Bugs: 330990 and Bug
http://bugs.kde.org/show_bug.cgi?id=330990
http://bugs.kde.org/show_bug.cgi?id=Bug
Repository: plasma-mediacenter
Description
---
Now the next and previous buttons do not work unless a media is playing.
Diffs
-
mediaelements/playlist/Playlist.qml fd83c21
Diff: https://git.reviewboard.kde.org/r/116874/diff/
Testing
---
I've done the testing for this patch.
These are the steps that I followed.
1. Added a new playlist and added a few songs to the playlist.
2. Re-opened Plasma Media Center
3. Clicked the Next and Previous buttons.
4. Nothing was played.
5. Selected a song from the list and then tested the next and previous
buttons.
6. The buttons worked seamlessly.
File Attachments
pulled the latest source and made changes
https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch
Thanks,
R.Harish Navnit
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 26, 2014, 4 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- Checked with the playlistModel.currentIndex. Now, Songs do not play when the next button is pressed, which solves the issue, but the currentIndex gets incremented(expected behaviour ?) Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs (updated) - mediaelements/playlist/Playlist.qml fd83c21 Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
On March 19, 2014, 6:05 p.m., Sinny Kumari wrote: shells/newshell/package/contents/ui/mediacenter.qml, line 90 https://git.reviewboard.kde.org/r/116874/diff/2/?file=255217#file255217line90 Instead of checking runtimeData.url check if playlist current Index is -1. I've checked this condition and it doesn't seem to work for me. I'm pasting the output log http://pastebin.ubuntu.com/7135890/, check the output of the debug statements that I'd added. I'm not uploading the patch since, it doesn't fix the issue, I'll upload it if needed(for reference) - R.Harish --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review53435 --- On March 18, 2014, 6 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 18, 2014, 6 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - shells/newshell/package/contents/ui/mediacenter.qml b6cb87c Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review53435 --- With current patch, suppose you play anything from media browser and then press next/prev in mediacontroller, then Playlist will start playing next/prev song. shells/newshell/package/contents/ui/mediacenter.qml https://git.reviewboard.kde.org/r/116874/#comment37582 Instead of checking runtimeData.url check if playlist current Index is -1. - Sinny Kumari On March 18, 2014, 6 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 18, 2014, 6 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - shells/newshell/package/contents/ui/mediacenter.qml b6cb87c Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - shells/newshell/package/contents/ui/mediacenter.qml b6cb87c Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review53324 --- shells/newshell/package/contents/ui/mediacenter.qml https://git.reviewboard.kde.org/r/116874/#comment37513 What if a user has paused and then clicked the next button. runtimeData.playing will be false but the next song should play. - Ashish Madeti On March 18, 2014, 7:59 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 18, 2014, 7:59 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - shells/newshell/package/contents/ui/mediacenter.qml b6cb87c Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 18, 2014, 6 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Changes --- used runtimeData.url instead of runtimeData.playing. Seems to solve the issue reported by Ashish Madeti. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs (updated) - shells/newshell/package/contents/ui/mediacenter.qml b6cb87c Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/#review53331 --- shells/newshell/package/contents/ui/mediacenter.qml https://git.reviewboard.kde.org/r/116874/#comment37515 yes, that's an issue that I'll have to look into. Thanks for reporting. - R.Harish Navnit On March 18, 2014, 2:29 p.m., R.Harish Navnit wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116874/ --- (Updated March 18, 2014, 2:29 p.m.) Review request for Plasma, Shantanu Tushar and Sinny Kumari. Bugs: 330990 and Bug http://bugs.kde.org/show_bug.cgi?id=330990 http://bugs.kde.org/show_bug.cgi?id=Bug Repository: plasma-mediacenter Description --- Now the next and previous buttons do not work unless a media is playing. Diffs - shells/newshell/package/contents/ui/mediacenter.qml b6cb87c Diff: https://git.reviewboard.kde.org/r/116874/diff/ Testing --- I've done the testing for this patch. These are the steps that I followed. 1. Added a new playlist and added a few songs to the playlist. 2. Re-opened Plasma Media Center 3. Clicked the Next and Previous buttons. 4. Nothing was played. 5. Selected a song from the list and then tested the next and previous buttons. 6. The buttons worked seamlessly. Thanks, R.Harish Navnit ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
