Ubuntu Music App

Merge lp:~ahayzen/music-app/remix-async-queue-list into lp:music-app/remix

  1. remix-async-queue-list
  2. Merge into remix
Proposed by Andrew Hayzen
Status: Merged
Approved by: Victor Thompson
Approved revision: 709
Merged at revision: 704
Proposed branch: lp:~ahayzen/music-app/remix-async-queue-list
Merge into: lp:music-app/remix
Diff against target: 533 lines (+197/-169)
4 files modified
MusicNowPlaying.qml (+185/-155)
music-app.qml (+3/-1)
tests/autopilot/music_app/__init__.py (+0/-4)
tests/autopilot/music_app/tests/test_music.py (+9/-9)
To merge this branch: bzr merge lp:~ahayzen/music-app/remix-async-queue-list
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Victor Thompson Approve
Review via email: mp+239609@code.launchpad.net

Commit message

* Load queueList in async
* Fix to prevent jumping from queue->full view

Description of the change

* Load queueList in async
* Fix to prevent jumping from queue->full view

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

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

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

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

FAILED: Continuous integration, rev:706
http://91.189.93.70:8080/job/music-app-remix-ci/168/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1271/console
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/168

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

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

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

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

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

See one inline comment. I still want to do a bit more testing for regressions as well.

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

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

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

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

Changes look good!

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

FAILED: Autolanding.
More details in the following jenkins job:
http://91.189.93.70:8080/job/music-app-remix-autolanding/64/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1296
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1296/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-autolanding/64

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

FAILED: Autolanding.
More details in the following jenkins job:
http://91.189.93.70:8080/job/music-app-remix-autolanding/66/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1298/console
    FAILURE: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-autolanding/66/console

review: Needs Fixing (continuous-integration)
lp:~ahayzen/music-app/remix-async-queue-list updated
709. By Andrew Hayzen

* Merge of lp:music-app/remix

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

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

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-remix-ci/179/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 'MusicNowPlaying.qml'
2--- MusicNowPlaying.qml 2014-10-26 16:18:12 +0000
3+++ MusicNowPlaying.qml 2014-10-26 21:40:41 +0000
4@@ -28,7 +28,7 @@
5
6 MusicPage {
7 id: nowPlaying
8- flickable: isListView ? queuelist : null // Ensures that the header is shown in fullview
9+ flickable: isListView ? queueListLoader.item : null // Ensures that the header is shown in fullview
10 objectName: "nowPlayingPage"
11 title: isListView ? i18n.tr("Queue") : i18n.tr("Now playing")
12 visible: false
13@@ -36,16 +36,39 @@
14 property bool isListView: false
15
16 onIsListViewChanged: {
17- if (isListView) {
18+ if (isListView) { // When changing to the queue positionAt the currentIndex
19+ // ensure the loader and listview is ready
20+ if (queueListLoader.status === Loader.Ready) {
21+ ensureListViewLoaded()
22+ } else {
23+ queueListLoader.onStatusChanged.connect(function() {
24+ if (queueListLoader.status === Loader.Ready) {
25+ ensureListViewLoaded()
26+ }
27+ })
28+ }
29+ }
30+ }
31+
32+ // Ensure that the listview has loaded before attempting to positionAt
33+ function ensureListViewLoaded() {
34+ if (queueListLoader.item.count === trackQueue.model.count) {
35 positionAt(player.currentIndex);
36+ } else {
37+ queueListLoader.item.onCountChanged.connect(function() {
38+ if (queueListLoader.item.count === trackQueue.model.count) {
39+ positionAt(player.currentIndex);
40+ }
41+ })
42 }
43 }
44
45+ // Position the view at the index
46 function positionAt(index) {
47- queuelist.positionViewAtIndex(index, ListView.Center);
48+ queueListLoader.item.positionViewAtIndex(index, ListView.Center);
49 }
50
51- state: isListView && queuelist.state === "multiselectable" ? "selection" : "default"
52+ state: isListView && queueListLoader.item.state === "multiselectable" ? "selection" : "default"
53 states: [
54 PageHeadState {
55 id: defaultState
56@@ -117,29 +140,29 @@
57 backAction: Action {
58 text: i18n.tr("Cancel selection")
59 iconName: "back"
60- onTriggered: queuelist.state = "normal"
61+ onTriggered: queueListLoader.item.state = "normal"
62 }
63 actions: [
64 Action {
65 text: i18n.tr("Select All")
66 iconName: "select"
67 onTriggered: {
68- if (queuelist.selectedItems.length === queuelist.model.count) {
69- queuelist.clearSelection()
70+ if (queueListLoader.item.selectedItems.length === trackQueue.model.count) {
71+ queueListLoader.item.clearSelection()
72 } else {
73- queuelist.selectAll()
74+ queueListLoader.item.selectAll()
75 }
76 }
77 },
78 Action {
79- enabled: queuelist.selectedItems.length > 0
80+ enabled: queueListLoader.item.selectedItems.length > 0
81 iconName: "add-to-playlist"
82 text: i18n.tr("Add to playlist")
83 onTriggered: {
84 var items = []
85
86- for (var i=0; i < queuelist.selectedItems.length; i++) {
87- items.push(makeDict(trackQueue.model.get(queuelist.selectedItems[i])));
88+ for (var i=0; i < queueListLoader.item.selectedItems.length; i++) {
89+ items.push(makeDict(trackQueue.model.get(queueListLoader.item.selectedItems[i])));
90 }
91
92 var comp = Qt.createComponent("MusicaddtoPlaylist.qml")
93@@ -151,19 +174,19 @@
94
95 mainPageStack.push(addToPlaylist)
96
97- queuelist.closeSelection()
98+ queueListLoader.item.closeSelection()
99 }
100 },
101 Action {
102- enabled: queuelist.selectedItems.length > 0
103+ enabled: queueListLoader.item.selectedItems.length > 0
104 iconName: "delete"
105 text: i18n.tr("Delete")
106 onTriggered: {
107- for (var i=0; i < queuelist.selectedItems.length; i++) {
108- removeQueue(queuelist.selectedItems[i])
109+ for (var i=0; i < queueListLoader.item.selectedItems.length; i++) {
110+ removeQueue(queueListLoader.item.selectedItems[i])
111 }
112
113- queuelist.closeSelection()
114+ queueListLoader.item.closeSelection()
115 }
116 }
117 ]
118@@ -476,14 +499,14 @@
119 {
120 var removedIndex = index
121
122- if (queuelist.count === 1) {
123+ if (trackQueue.model.count === 1) {
124 player.stop()
125 musicToolbar.goBack()
126 } else if (index === player.currentIndex) {
127 player.nextSong(player.isPlaying);
128 }
129
130- queuelist.model.remove(index);
131+ trackQueue.model.remove(index);
132
133 if (removedIndex < player.currentIndex) {
134 // update index as the old has been removed
135@@ -491,145 +514,152 @@
136 }
137 }
138
139- ListView {
140- id: queuelist
141+ Loader {
142+ id: queueListLoader
143 anchors {
144- bottomMargin: units.gu(2)
145 fill: parent
146- topMargin: units.gu(2)
147- }
148- delegate: queueDelegate
149- footer: Item {
150- height: mainView.height - (styleMusic.common.expandHeight + queuelist.currentHeight) + units.gu(8)
151- }
152- model: trackQueue.model
153- objectName: "nowPlayingQueueList"
154+ }
155+ asynchronous: true
156+ sourceComponent: ListView {
157+ id: queueList
158+ anchors {
159+ bottomMargin: units.gu(2)
160+ fill: parent
161+ topMargin: units.gu(2)
162+ }
163+ delegate: queueDelegate
164+ footer: Item {
165+ height: mainView.height - (styleMusic.common.expandHeight + queueList.currentHeight) + units.gu(8)
166+ }
167+ model: trackQueue.model
168+ objectName: "nowPlayingqueueList"
169+
170+ property int normalHeight: units.gu(6)
171+ property int transitionDuration: 250 // transition length of animations
172+
173+ onCountChanged: {
174+ customdebug("Queue: Now has: " + queueList.count + " tracks")
175+ }
176+
177+ // Requirements for ListItemWithActions
178+ property var selectedItems: []
179+
180+ signal clearSelection()
181+ signal closeSelection()
182+ signal selectAll()
183+
184+ onClearSelection: selectedItems = []
185+ onCloseSelection: {
186+ clearSelection()
187+ state = "normal"
188+ }
189+ onSelectAll: {
190+ var tmp = selectedItems
191+
192+ for (var i=0; i < model.count; i++) {
193+ if (tmp.indexOf(i) === -1) {
194+ tmp.push(i)
195+ }
196+ }
197+
198+ selectedItems = tmp
199+ }
200+ onVisibleChanged: {
201+ if (!visible) {
202+ clearSelection(true)
203+ }
204+ }
205+
206+ Component {
207+ id: queueDelegate
208+ ListItemWithActions {
209+ id: queueListItem
210+ color: player.currentIndex === index ? "#2c2c34" : "transparent"
211+ height: queueList.normalHeight
212+ objectName: "nowPlayingListItem" + index
213+ state: ""
214+
215+ leftSideAction: Remove {
216+ onTriggered: removeQueue(index)
217+ }
218+ multiselectable: true
219+ reorderable: true
220+ rightSideActions: [
221+ AddToPlaylist{
222+
223+ }
224+ ]
225+
226+ onItemClicked: {
227+ customdebug("File: " + model.filename) // debugger
228+ trackQueueClick(index); // toggle track state
229+ }
230+ onReorder: {
231+ console.debug("Move: ", from, to);
232+
233+ trackQueue.model.move(from, to, 1);
234+
235+
236+ // Maintain currentIndex with current song
237+ if (from === player.currentIndex) {
238+ player.currentIndex = to;
239+ }
240+ else if (from < player.currentIndex && to >= player.currentIndex) {
241+ player.currentIndex -= 1;
242+ }
243+ else if (from > player.currentIndex && to <= player.currentIndex) {
244+ player.currentIndex += 1;
245+ }
246+ }
247+
248+ Item {
249+ id: trackContainer;
250+ anchors {
251+ fill: parent
252+ }
253+
254+ NumberAnimation {
255+ id: trackContainerReorderAnimation
256+ target: trackContainer;
257+ property: "anchors.leftMargin";
258+ duration: queueList.transitionDuration;
259+ to: units.gu(2)
260+ }
261+
262+ NumberAnimation {
263+ id: trackContainerResetAnimation
264+ target: trackContainer;
265+ property: "anchors.leftMargin";
266+ duration: queueList.transitionDuration;
267+ to: units.gu(0.5)
268+ }
269+
270+ MusicRow {
271+ id: musicRow
272+ height: parent.height
273+ column: Column {
274+ Label {
275+ id: trackTitle
276+ color: player.currentIndex === index ? UbuntuColors.blue
277+ : styleMusic.common.music
278+ fontSize: "small"
279+ objectName: "titleLabel"
280+ text: model.title
281+ }
282+
283+ Label {
284+ id: trackArtist
285+ color: styleMusic.common.subtitle
286+ fontSize: "x-small"
287+ objectName: "artistLabel"
288+ text: model.author
289+ }
290+ }
291+ }
292+ }
293+ }
294+ }
295+ }
296 visible: isListView
297-
298- property int normalHeight: units.gu(6)
299- property int transitionDuration: 250 // transition length of animations
300-
301- onCountChanged: {
302- customdebug("Queue: Now has: " + queuelist.count + " tracks")
303- }
304-
305- // Requirements for ListItemWithActions
306- property var selectedItems: []
307-
308- signal clearSelection()
309- signal closeSelection()
310- signal selectAll()
311-
312- onClearSelection: selectedItems = []
313- onCloseSelection: {
314- clearSelection()
315- state = "normal"
316- }
317- onSelectAll: {
318- var tmp = selectedItems
319-
320- for (var i=0; i < model.count; i++) {
321- if (tmp.indexOf(i) === -1) {
322- tmp.push(i)
323- }
324- }
325-
326- selectedItems = tmp
327- }
328- onVisibleChanged: {
329- if (!visible) {
330- clearSelection(true)
331- }
332- }
333-
334- Component {
335- id: queueDelegate
336- ListItemWithActions {
337- id: queueListItem
338- color: player.currentIndex === index ? "#2c2c34" : "transparent"
339- height: queuelist.normalHeight
340- objectName: "nowPlayingListItem" + index
341- state: ""
342-
343- leftSideAction: Remove {
344- onTriggered: removeQueue(index)
345- }
346- multiselectable: true
347- reorderable: true
348- rightSideActions: [
349- AddToPlaylist{
350-
351- }
352- ]
353-
354- onItemClicked: {
355- customdebug("File: " + model.filename) // debugger
356- trackQueueClick(index); // toggle track state
357- }
358- onReorder: {
359- console.debug("Move: ", from, to);
360-
361- queuelist.model.move(from, to, 1);
362-
363-
364- // Maintain currentIndex with current song
365- if (from === player.currentIndex) {
366- player.currentIndex = to;
367- }
368- else if (from < player.currentIndex && to >= player.currentIndex) {
369- player.currentIndex -= 1;
370- }
371- else if (from > player.currentIndex && to <= player.currentIndex) {
372- player.currentIndex += 1;
373- }
374- }
375-
376- Item {
377- id: trackContainer;
378- anchors {
379- fill: parent
380- }
381-
382- NumberAnimation {
383- id: trackContainerReorderAnimation
384- target: trackContainer;
385- property: "anchors.leftMargin";
386- duration: queuelist.transitionDuration;
387- to: units.gu(2)
388- }
389-
390- NumberAnimation {
391- id: trackContainerResetAnimation
392- target: trackContainer;
393- property: "anchors.leftMargin";
394- duration: queuelist.transitionDuration;
395- to: units.gu(0.5)
396- }
397-
398- MusicRow {
399- id: musicRow
400- height: parent.height
401- column: Column {
402- Label {
403- id: trackTitle
404- color: player.currentIndex === index ? UbuntuColors.blue
405- : styleMusic.common.music
406- fontSize: "small"
407- objectName: "titleLabel"
408- text: model.title
409- }
410-
411- Label {
412- id: trackArtist
413- color: styleMusic.common.subtitle
414- fontSize: "x-small"
415- objectName: "artistLabel"
416- text: model.author
417- }
418- }
419- }
420- }
421- }
422- }
423 }
424 }
425
426=== modified file 'music-app.qml'
427--- music-app.qml 2014-10-26 20:34:59 +0000
428+++ music-app.qml 2014-10-26 21:40:41 +0000
429@@ -661,7 +661,9 @@
430 }
431
432 // Show the Now playing page and make sure the track is visible
433- tabs.pushNowPlaying();
434+ if (mainPageStack.currentPage.title !== i18n.tr("Queue")) {
435+ tabs.pushNowPlaying();
436+ }
437 }
438
439 function playRandomSong(shuffle)
440
441=== modified file 'tests/autopilot/music_app/__init__.py'
442--- tests/autopilot/music_app/__init__.py 2014-10-24 18:49:00 +0000
443+++ tests/autopilot/music_app/__init__.py 2014-10-26 21:40:41 +0000
444@@ -270,10 +270,6 @@
445 def click_toggle_view(self):
446 self.main_view.get_header().click_action_button("toggleView")
447
448- def get_count(self):
449- return self.select_single("QQuickListView",
450- objectName="nowPlayingQueueList").count
451-
452 def go_back(self):
453 """Use custom back button to go back"""
454 self.main_view.get_header().click_custom_back_button()
455
456=== modified file 'tests/autopilot/music_app/tests/test_music.py'
457--- tests/autopilot/music_app/tests/test_music.py 2014-10-24 12:55:02 +0000
458+++ tests/autopilot/music_app/tests/test_music.py 2014-10-26 21:40:41 +0000
459@@ -377,12 +377,12 @@
460 now_playing_page = self.app.get_now_playing_page()
461
462 # verify track queue has added all songs to initial value
463- self.assertThat(now_playing_page.get_count(),
464+ self.assertThat(self.app.get_queue_count(),
465 Equals(initial_tracks_count + 3))
466
467 # Assert that the song added to the list is playing
468 self.assertThat(self.player.currentIndex,
469- Eventually(NotEquals(now_playing_page.get_count())))
470+ Eventually(NotEquals(self.app.get_queue_count())))
471 self.assertThat(self.player.isPlaying, Eventually(Equals(True)))
472
473 # verify song's metadata matches the item added to the Now Playing view
474@@ -519,12 +519,12 @@
475 now_playing_page = self.app.get_now_playing_page()
476
477 # verify track queue has added all songs to initial value
478- self.assertThat(now_playing_page.get_count(),
479+ self.assertThat(self.app.get_queue_count(),
480 Equals(initial_tracks_count + 2))
481
482 # Assert that the song added to the list is playing
483 self.assertThat(self.player.currentIndex,
484- Eventually(NotEquals(now_playing_page.get_count())))
485+ Eventually(NotEquals(self.app.get_queue_count())))
486 self.assertThat(self.player.isPlaying, Eventually(Equals(True)))
487
488 # verify song's metadata matches the item added to the Now Playing view
489@@ -544,7 +544,7 @@
490 now_playing_page = self.app.get_now_playing_page()
491
492 # get initial queue count
493- initial_queue_count = now_playing_page.get_count()
494+ initial_queue_count = self.app.get_queue_count()
495
496 # get track row and swipe to reveal swipe to delete
497 track = now_playing_page.get_track(0)
498@@ -553,7 +553,7 @@
499 track.confirm_removal() # confirm delete
500
501 # verify song has been deleted
502- self.assertThat(now_playing_page.get_count(),
503+ self.assertThat(self.app.get_queue_count(),
504 Eventually(Equals(initial_queue_count - 1)))
505
506 def test_playback_stops_when_last_song_ends_and_repeat_off(self):
507@@ -567,7 +567,7 @@
508 now_playing_page.set_repeat(False)
509
510 # Skip through all songs in queue, stopping on last one.
511- for count in range(0, now_playing_page.get_count() - 1):
512+ for count in range(0, self.app.get_queue_count() - 1):
513 now_playing_page.click_forward_button()
514
515 # When the last song ends, playback should stop
516@@ -584,7 +584,7 @@
517 now_playing_page.set_repeat(True)
518
519 # Skip through all songs in queue, stopping on last one.
520- for count in range(0, now_playing_page.get_count() - 1):
521+ for count in range(0, self.app.get_queue_count() - 1):
522 now_playing_page.click_forward_button()
523
524 # Make sure we loop back to first song after last song ends
525@@ -603,7 +603,7 @@
526 now_playing_page.set_repeat(True)
527
528 # Skip through all songs in queue, INCLUDING last one.
529- for count in range(0, now_playing_page.get_count() - 1):
530+ for count in range(0, self.app.get_queue_count() - 1):
531 now_playing_page.click_forward_button()
532
533 # Make sure we loop back to first song after last song ends

Subscribers

People subscribed via source and target branches