Ubuntu Music App

Merge lp:~ahayzen/music-app/remix-add-remove-playlists-recent-out-of-sync into lp:music-app/remix

  1. remix-add-remove-playlists-recent-out-of-sync
  2. Merge into remix
Proposed by Andrew Hayzen
Status: Merged
Approved by: Victor Thompson
Approved revision: 722
Merged at revision: 720
Proposed branch: lp:~ahayzen/music-app/remix-add-remove-playlists-recent-out-of-sync
Merge into: lp:music-app/remix
Diff against target: 222 lines (+79/-43)
3 files modified
MusicaddtoPlaylist.qml (+22/-2)
common/ListItemActions/AddToPlaylist.qml (+2/-1)
common/SongsPage.qml (+55/-40)
To merge this branch: bzr merge lp:~ahayzen/music-app/remix-add-remove-playlists-recent-out-of-sync
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Victor Thompson Approve
Review via email: mp+239931@code.launchpad.net

Commit message

* Fix for cover art getting out of sync on the SongsPage.qml
* Fix for addToPlaylists causing freeze to songsPage
* Ensure recent/playlists pages remain in sync as items are added/removed from the playlist

Description of the change

* Fix for cover art getting out of sync on the SongsPage.qml
* Fix for addToPlaylists causing freeze to songsPage
* Ensure recent/playlists pages remain in sync as items are added/removed from the playlist

To post a comment you must log in.
Revision history for this message
Victor Thompson (vthompson) wrote :

Can we consolidate some of this reused code to a helper function? I count at least 4 or 5 instances of the same logic.

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

FAILED: Continuous integration, rev:720
http://91.189.93.70:8080/job/music-app-remix-ci/212/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1372
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1372/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/212

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-remix-ci/212/rebuild

review: Needs Fixing (continuous-integration)
lp:~ahayzen/music-app/remix-add-remove-playlists-recent-out-of-sync updated
721. By Andrew Hayzen

* Make generic helper

722. By Andrew Hayzen

* Fix so that 1 level addToPlaylist works again

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:721
http://91.189.93.70:8080/job/music-app-remix-ci/213/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1373
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1373/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/213

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-remix-ci/213/rebuild

review: Approve (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

LGTM and I can not find any regressions.

review: Approve
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:722
http://91.189.93.70:8080/job/music-app-remix-ci/215/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1376
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1376/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/215

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-remix-ci/215/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'MusicaddtoPlaylist.qml'
2--- MusicaddtoPlaylist.qml 2014-10-24 18:18:07 +0000
3+++ MusicaddtoPlaylist.qml 2014-10-29 03:01:56 +0000
4@@ -23,6 +23,7 @@
5 import Ubuntu.Components.ListItems 1.0 as ListItem
6 import Ubuntu.Components.Popups 1.0
7 import QtQuick.LocalStorage 2.0
8+import "meta-database.js" as Library
9 import "playlists.js" as Playlists
10 import "common"
11
12@@ -42,6 +43,7 @@
13 visible: false
14
15 property var chosenElements: []
16+ property var page
17
18 head {
19 actions: [
20@@ -85,8 +87,26 @@
21 Playlists.addToPlaylist(name, chosenElements[i])
22 }
23
24- playlistModel.filterPlaylists();
25- albumTracksModel.filterPlaylistTracks(name)
26+ // Check that the parent parent page is not being refiltered
27+ if (page !== undefined && page.page !== undefined && page.page.title === i18n.tr("Playlists")) {
28+ page.page.changed = true
29+ } else {
30+ playlistModel.filterPlaylists();
31+ }
32+
33+ if (Library.recentContainsPlaylist(name)) {
34+ // Check that the parent parent page is not being refiltered
35+ if (page !== undefined && page.page !== undefined && page.page.title === i18n.tr("Recent")) {
36+ page.page.changed = true
37+ } else {
38+ recentModel.filterRecent()
39+ }
40+ }
41+
42+ if (page !== undefined && name === page.line2 && page.playlistChanged !== undefined) {
43+ page.playlistChanged = true
44+ page.covers = Playlists.getPlaylistCovers(name)
45+ }
46
47 musicToolbar.goBack(); // go back to the previous page
48 }
49
50=== modified file 'common/ListItemActions/AddToPlaylist.qml'
51--- common/ListItemActions/AddToPlaylist.qml 2014-10-26 16:18:12 +0000
52+++ common/ListItemActions/AddToPlaylist.qml 2014-10-29 03:01:56 +0000
53@@ -25,12 +25,13 @@
54 text: i18n.tr("Add to playlist")
55
56 property bool primed: false
57+ property var page
58
59 onTriggered: {
60 console.debug("Debug: Add track to playlist");
61
62 var comp = Qt.createComponent("../../MusicaddtoPlaylist.qml")
63- var addToPlaylist = comp.createObject(mainPageStack, {"chosenElements": [makeDict(model)]});
64+ var addToPlaylist = comp.createObject(mainPageStack, {"chosenElements": [makeDict(model)], "page": page});
65
66 if (addToPlaylist == null) { // Error Handling
67 console.log("Error creating object");
68
69=== modified file 'common/SongsPage.qml'
70--- common/SongsPage.qml 2014-10-29 00:49:40 +0000
71+++ common/SongsPage.qml 2014-10-29 03:01:56 +0000
72@@ -45,6 +45,44 @@
73 property alias album: songsModel.album
74 property alias genre: songsModel.genre
75
76+ property bool playlistChanged: false
77+
78+ onVisibleChanged: {
79+ if (playlistChanged) {
80+ playlistChanged = false
81+ refreshWaitTimer.start()
82+ }
83+ }
84+
85+ Timer { // FIXME: workaround for when the playlist is deleted and the delegate being deleting causes freezing
86+ id: refreshWaitTimer
87+ interval: 250
88+ onTriggered: albumTracksModel.filterPlaylistTracks(line2)
89+ }
90+
91+ function playlistChangedHelper()
92+ {
93+ // if parent Playlists then set changed otherwise refilter
94+ if (songStackPage.page.title === i18n.tr("Playlists")) {
95+ if (songStackPage.page !== undefined) {
96+ songStackPage.page.changed = true
97+ }
98+ } else {
99+ playlistModel.filterPlaylists()
100+ }
101+
102+ if (Library.recentContainsPlaylist(songStackPage.line2)) {
103+ // if parent Recent then set changed otherwise refilter
104+ if (songStackPage.page.title === i18n.tr("Recent")) {
105+ if (songStackPage.page !== undefined) {
106+ songStackPage.page.changed = true
107+ }
108+ } else {
109+ recentModel.filterRecent()
110+ }
111+ }
112+ }
113+
114 state: albumtrackslist.state === "multiselectable" ? "selection" : (songStackPage.line1 === i18n.tr("Playlist") ? "playlist" : "album")
115 states: [
116 PageHeadState {
117@@ -115,7 +153,7 @@
118 }
119
120 var comp = Qt.createComponent("../MusicaddtoPlaylist.qml")
121- var addToPlaylist = comp.createObject(mainPageStack, {"chosenElements": items});
122+ var addToPlaylist = comp.createObject(mainPageStack, {"chosenElements": items, "page": songStackPage});
123
124 if (addToPlaylist == null) { // Error Handling
125 console.log("Error creating object");
126@@ -157,8 +195,12 @@
127
128 albumtrackslist.closeSelection()
129
130- songStackPage.page.changed = true
131+ playlistChangedHelper() // update recent/playlist models
132+
133 albumTracksModel.filterPlaylistTracks(songStackPage.line2)
134+
135+ // refresh cover art
136+ songStackPage.covers = Playlists.getPlaylistCovers(songStackPage.line2)
137 }
138 }
139 ]
140@@ -368,7 +410,7 @@
141
142 },
143 AddToPlaylist {
144-
145+ page: songStackPage
146 }
147 ]
148
149@@ -399,8 +441,12 @@
150 onTriggered: {
151 Playlists.removeFromPlaylist(songStackPage.line2, model.i)
152
153- songStackPage.page.changed = true
154+ playlistChangedHelper() // update recent/playlist models
155+
156 albumTracksModel.filterPlaylistTracks(songStackPage.line2)
157+
158+ // refresh cover art
159+ songStackPage.covers = Playlists.getPlaylistCovers(songStackPage.line2)
160 }
161 }
162 }
163@@ -470,28 +516,13 @@
164 console.debug("Debug: User changed name from "+playlistName.placeholderText+" to "+playlistName.text)
165
166 if (Playlists.renamePlaylist(playlistName.placeholderText, playlistName.text) === true) {
167- // if parent Playlists then set changed otherwise refilter
168- if (songStackPage.page.title === i18n.tr("Playlists")) {
169- if (songStackPage.page !== undefined) {
170- songStackPage.page.changed = true
171- }
172- } else {
173- playlistModel.filterPlaylists()
174- }
175
176 if (Library.recentContainsPlaylist(playlistName.placeholderText)) {
177 Library.recentRenamePlaylist(playlistName.placeholderText, playlistName.text)
178-
179- // if parent Recent then set changed otherwise refilter
180- if (songStackPage.page.title === i18n.tr("Recent")) {
181- if (songStackPage.page !== undefined) {
182- songStackPage.page.changed = true
183- }
184- } else {
185- recentModel.filterRecent()
186- }
187 }
188
189+ playlistChangedHelper() // update recent/playlist models
190+
191 PopupUtils.close(dialogEditPlaylist)
192
193 line2 = playlistName.text
194@@ -533,25 +564,9 @@
195
196 if (Library.recentContainsPlaylist(dialogRemovePlaylist.oldPlaylistName)) {
197 Library.recentRemovePlaylist(dialogRemovePlaylist.oldPlaylistName)
198-
199- // if parent Recent then set changed otherwise refilter
200- if (songStackPage.page.title === i18n.tr("Recent")) {
201- if (songStackPage.page !== undefined) {
202- songStackPage.page.changed = true
203- }
204- } else {
205- recentModel.filterRecent()
206- }
207- }
208-
209- // if parent Playlists then set changed otherwise refilter
210- if (songStackPage.page.title === i18n.tr("Playlists")) {
211- if (songStackPage.page !== undefined) {
212- songStackPage.page.changed = true
213- }
214- } else {
215- playlistModel.filterPlaylists()
216- }
217+ }
218+
219+ playlistChangedHelper() // update recent/playlist models
220
221 songStackPage.page = undefined
222 PopupUtils.close(dialogRemovePlaylist)

Subscribers

People subscribed via source and target branches