Ubuntu Music App

Merge lp:~ahayzen/music-app/refactor-async-loader-pages into lp:music-app

  1. refactor-async-loader-pages
  2. Merge into trunk
Proposed by Andrew Hayzen
Status: Work in progress
Proposed branch: lp:~ahayzen/music-app/refactor-async-loader-pages
Merge into: lp:music-app
Diff against target: 355 lines (+132/-36)
7 files modified
app/components/ColumnFlow.qml (+20/-3)
app/components/Flickables/CardView.qml (+1/-7)
app/components/MusicToolbar.qml (+1/-0)
app/music-app.qml (+107/-26)
app/ui/Albums.qml (+1/-0)
app/ui/Genres.qml (+1/-0)
app/ui/Playlists.qml (+1/-0)
To merge this branch: bzr merge lp:~ahayzen/music-app/refactor-async-loader-pages
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Victor Thompson Needs Information
Review via email: mp+248477@code.launchpad.net

Commit message

* Load pages in async, the current page first then the other pages

Description of the change

* Load pages in async, the current page first then the other pages

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

PASSED: Continuous integration, rev:834
http://91.189.93.70:8080/job/music-app-refactor-ci/20/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/980
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/980/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-refactor-vivid-amd64-ci/20

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

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

The reason this makes a difference is because we are now waiting to load the Songs tab until it becomes visible. I'm not sure the others really need to be loaded asynchronously--as they currently behave this way internally anyway.

I'd also like us to revisit what it means to load things in this fashion. There's a loading time hit for every CardView filled tab (and now ListView) we currently have in the app. So with this MP it's every tab. This makes it a bit annoying (slow and distracting, as well) to browse from tab to tab. I think a better idea might be to load things asynchronously, but do so upon app startup. Maybe wait until the current view has finished, then load the other views. This would mean the below Loaders would be made "active: true" and the CardView would be made to load when not visible. But we should discuss this.

Since this isn't very important for the goal of refactoring, let's think this through a bit more before making a decision.

review: Needs Information
Reply
lp:~ahayzen/music-app/refactor-async-loader-pages updated
835. By Andrew Hayzen

* Load the other pages/tabs once the first is complete

836. By Andrew Hayzen

* Merge of trunk

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

PASSED: Continuous integration, rev:836
http://91.189.93.70:8080/job/music-app-refactor-ci/27/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/1009
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/1009/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-refactor-vivid-amd64-ci/27

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

review: Approve (continuous-integration)
Reply
lp:~ahayzen/music-app/refactor-async-loader-pages updated
837. By Andrew Hayzen

* Merge of trunk

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

We are going to make a version for trunk first, therefore marking this as WIP for now.

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

PASSED: Continuous integration, rev:837
http://91.189.93.70:8080/job/music-app-refactor-ci/31/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/1016
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/1016/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-refactor-vivid-amd64-ci/31

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

review: Approve (continuous-integration)
Reply
lp:~ahayzen/music-app/refactor-async-loader-pages updated
838. By Andrew Hayzen

* Allow empty ColumnFlow's to load when not visible

839. By Andrew Hayzen

* Pull of /refactor

840. By Andrew Hayzen

* Pull of changes to ColumnFlow.qml in async-loader-pages that teach it about preloading

841. By Andrew Hayzen

* Wait until the toolbar has shown before loading further pages

Unmerged revisions

841. By Andrew Hayzen

* Wait until the toolbar has shown before loading further pages

840. By Andrew Hayzen

* Pull of changes to ColumnFlow.qml in async-loader-pages that teach it about preloading

839. By Andrew Hayzen

* Pull of /refactor

838. By Andrew Hayzen

* Allow empty ColumnFlow's to load when not visible

837. By Andrew Hayzen

* Merge of trunk

836. By Andrew Hayzen

* Merge of trunk

835. By Andrew Hayzen

* Load the other pages/tabs once the first is complete

834. By Andrew Hayzen

* Load pages in async and only when visible

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/components/ColumnFlow.qml'
2--- app/components/ColumnFlow.qml 2015-01-11 17:40:13 +0000
3+++ app/components/ColumnFlow.qml 2015-05-02 16:02:00 +0000
4@@ -40,6 +40,7 @@
5 property bool removing: false
6 property bool restoring: false // is the view restoring?
7 property var restoreItems: ({}) // when rebuilding items are stored here temporarily
8+ property bool preloading: true // when visible has only been false allow loading (as no child objects [eg pages] can have been created on the fly)
9
10 onColumnWidthChanged: {
11 if (restoring) {
12@@ -62,6 +63,10 @@
13 }
14
15 onVisibleChanged: {
16+ if (visible) {
17+ preloading = false;
18+ }
19+
20 if (visible && delayRebuildIndex !== -1) { // restore from count change
21 if (delayRebuildIndex === 0) {
22 reset()
23@@ -86,7 +91,7 @@
24 Connections {
25 target: model === undefined ? fakeModel : model
26 onModelReset: {
27- if (!visible) {
28+ if (!visible && lastIndex > 0 && !preloading) {
29 delayRebuildIndex = 0
30 } else {
31 reset()
32@@ -94,7 +99,7 @@
33 }
34 }
35 onRowsInserted: {
36- if (!visible) {
37+ if (!visible && lastIndex > 0 && !preloading) {
38 setDelayRebuildIndex(first)
39 } else {
40 if (first <= lastIndex) {
41@@ -161,7 +166,8 @@
42 // and
43 // allow if the y position is within the viewport
44 // or if loadBefore is true then allow if the y position is before the viewport
45- if (((count > 0 && lastIndex < count && insertMax === undefined) || (insertMax !== undefined && lastIndex <= insertMax)) && (inViewport(y, 0) || (loadBefore === true && beforeViewport(y)))) {
46+ if (((count > 0 && lastIndex < count && insertMax === undefined) || (insertMax !== undefined && lastIndex <= insertMax))
47+ && (inViewport(y, 0) || (loadBefore === true && beforeViewport(y)))) {
48 incubateObject(lastIndex++, columnsByHeight[i], getMaxInColumn(columnsByHeight[i]), append);
49 workDone = true
50 } else {
51@@ -489,4 +495,15 @@
52 delayRebuildIndex = index
53 }
54 }
55+
56+ Component.onCompleted: {
57+ // Ensure that initial column vars are set
58+ for (var j=0; j < columns; j++) {
59+ columnHeights.push({})
60+ }
61+
62+ cacheColumnHeights()
63+
64+ append(true)
65+ }
66 }
67
68=== modified file 'app/components/Flickables/CardView.qml'
69--- app/components/Flickables/CardView.qml 2015-02-08 04:06:28 +0000
70+++ app/components/Flickables/CardView.qml 2015-05-02 16:02:00 +0000
71@@ -35,17 +35,11 @@
72 property alias delegate: flow.delegate
73 property var getter
74 property alias header: headerLoader.sourceComponent
75- property var model: flow.model
76+ property alias model: flow.model
77 property real itemWidth: units.gu(15)
78
79 onGetterChanged: flow.getter = getter // cannot use alias to set a function (must be var)
80
81- onVisibleChanged: {
82- if (visible) { // only load model once CardView is visible
83- flow.model = model
84- }
85- }
86-
87 Loader {
88 id: headerLoader
89 visible: sourceComponent !== undefined
90
91=== modified file 'app/components/MusicToolbar.qml'
92--- app/components/MusicToolbar.qml 2015-02-04 16:43:17 +0000
93+++ app/components/MusicToolbar.qml 2015-05-02 16:02:00 +0000
94@@ -20,6 +20,7 @@
95 import QtQuick 2.3
96 import QtMultimedia 5.0
97 import Ubuntu.Components 1.1
98+import Ubuntu.Thumbnailer 0.1
99
100 Rectangle {
101 anchors {
102
103=== modified file 'app/music-app.qml'
104--- app/music-app.qml 2015-04-29 00:55:02 +0000
105+++ app/music-app.qml 2015-05-02 16:02:00 +0000
106@@ -458,10 +458,12 @@
107
108 // TODO: improve in refactoring to be able detect when a track is removed
109 // Update playlists page
110- if (playlistsPage.visible) {
111- playlistModel.filterPlaylists()
112- } else {
113- playlistsPage.changed = true
114+ if (playlistsPageLoader.status === Loader.Ready) {
115+ if (playlistsPageLoader.item.visible) {
116+ playlistModel.filterPlaylists()
117+ } else {
118+ playlistsPageLoader.item.changed = true
119+ }
120 }
121 }
122 }
123@@ -497,10 +499,12 @@
124 console.debug("Removed recent:", JSON.stringify(removed))
125 Library.recentRemoveAlbums(removed)
126
127- if (recentPage.visible) {
128- recentModel.filterRecent()
129- } else {
130- recentPage.changed = true
131+ if (recentPageLoader.status === Loader.Ready) {
132+ if (recentPageLoader.item.visible) {
133+ recentModel.filterRecent()
134+ } else {
135+ recentPageLoader.item.changed = true
136+ }
137 }
138 }
139 }
140@@ -756,6 +760,7 @@
141 }
142
143 property Tab lastTab: selectedTab
144+ property bool firstTabAyncLoadComplete: false
145
146 onSelectedTabChanged: {
147 // pause loading of the models in the old tab
148@@ -797,11 +802,23 @@
149 id: recentTab
150 objectName: "recentTab"
151 anchors.fill: parent
152- title: page.title
153+ title: i18n.tr("Recent")
154
155 // Tab content begins here
156- page: Recent {
157- id: recentPage
158+ page: Loader {
159+ id: recentPageLoader
160+ anchors {
161+ fill: parent
162+ }
163+ active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
164+ asynchronous: true
165+ source: "ui/Recent.qml"
166+
167+ onStatusChanged: {
168+ if (status == Loader.Ready) {
169+ tabs.firstTabAyncLoadComplete = true
170+ }
171+ }
172 }
173 }
174 }
175@@ -844,11 +861,23 @@
176 id: artistsTab
177 objectName: "artistsTab"
178 anchors.fill: parent
179- title: page.title
180+ title: i18n.tr("Artists")
181
182 // tab content
183- page: Artists {
184- id: artistsPage
185+ page: Loader {
186+ id: artistsPageLoader
187+ anchors {
188+ fill: parent
189+ }
190+ active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
191+ asynchronous: true
192+ source: "ui/Artists.qml"
193+
194+ onStatusChanged: {
195+ if (status == Loader.Ready) {
196+ tabs.firstTabAyncLoadComplete = true
197+ }
198+ }
199 }
200 }
201
202@@ -861,11 +890,23 @@
203 id: albumsTab
204 objectName: "albumsTab"
205 anchors.fill: parent
206- title: page.title
207+ title: i18n.tr("Albums")
208
209 // Tab content begins here
210- page: Albums {
211- id: albumsPage
212+ page: Loader {
213+ id: albumsPageLoader
214+ anchors {
215+ fill: parent
216+ }
217+ active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
218+ asynchronous: true
219+ source: "ui/Albums.qml"
220+
221+ onStatusChanged: {
222+ if (status == Loader.Ready) {
223+ tabs.firstTabAyncLoadComplete = true
224+ }
225+ }
226 }
227 }
228
229@@ -878,11 +919,23 @@
230 id: genresTab
231 objectName: "genresTab"
232 anchors.fill: parent
233- title: page.title
234+ title: i18n.tr("Genres")
235
236 // Tab content begins here
237- page: Genres {
238- id: genresPage
239+ page: Loader {
240+ id: genresPageLoader
241+ anchors {
242+ fill: parent
243+ }
244+ active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
245+ asynchronous: true
246+ source: "ui/Genres.qml"
247+
248+ onStatusChanged: {
249+ if (status == Loader.Ready) {
250+ tabs.firstTabAyncLoadComplete = true
251+ }
252+ }
253 }
254 }
255
256@@ -895,11 +948,23 @@
257 id: songsTab
258 objectName: "songsTab"
259 anchors.fill: parent
260- title: page.title
261+ title: i18n.tr("Songs")
262
263 // Tab content begins here
264- page: Songs {
265- id: tracksPage
266+ page: Loader {
267+ id: songsPageLoader
268+ anchors {
269+ fill: parent
270+ }
271+ active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
272+ asynchronous: true
273+ source: "ui/Songs.qml"
274+
275+ onStatusChanged: {
276+ if (status == Loader.Ready) {
277+ tabs.firstTabAyncLoadComplete = true
278+ }
279+ }
280 }
281 }
282
283@@ -913,17 +978,33 @@
284 id: playlistsTab
285 objectName: "playlistsTab"
286 anchors.fill: parent
287- title: page.title
288+ title: i18n.tr("Playlists")
289
290 // Tab content begins here
291- page: Playlists {
292- id: playlistsPage
293+ page: Loader {
294+ id: playlistsPageLoader
295+ anchors {
296+ fill: parent
297+ }
298+ active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
299+ asynchronous: true
300+ source: "ui/Playlists.qml"
301+
302+ onStatusChanged: {
303+ if (status == Loader.Ready) {
304+ tabs.firstTabAyncLoadComplete = true
305+ }
306+ }
307 }
308 }
309
310 // Set the models in the tab to allow/disallow loading
311 function allowLoading(tabToLoad, state)
312 {
313+ if (loadedUI) {
314+ selectedTab.page.active = true
315+ }
316+
317 if (tabToLoad && tabToLoad.model !== undefined)
318 {
319 for (var i=0; i < tabToLoad.model.length; i++)
320
321=== modified file 'app/ui/Albums.qml'
322--- app/ui/Albums.qml 2015-02-16 20:27:40 +0000
323+++ app/ui/Albums.qml 2015-05-02 16:02:00 +0000
324@@ -20,6 +20,7 @@
325 import QtQuick 2.3
326 import Ubuntu.Components 1.1
327 import Ubuntu.MediaScanner 0.1
328+import Ubuntu.Thumbnailer 0.1
329 import "../components"
330 import "../components/Delegates"
331 import "../components/Flickables"
332
333=== modified file 'app/ui/Genres.qml'
334--- app/ui/Genres.qml 2015-02-16 20:27:40 +0000
335+++ app/ui/Genres.qml 2015-05-02 16:02:00 +0000
336@@ -20,6 +20,7 @@
337 import QtQuick 2.3
338 import Ubuntu.Components 1.1
339 import Ubuntu.MediaScanner 0.1
340+import Ubuntu.Thumbnailer 0.1
341 import "../components"
342 import "../components/Delegates"
343 import "../components/Flickables"
344
345=== modified file 'app/ui/Playlists.qml'
346--- app/ui/Playlists.qml 2015-04-28 17:37:33 +0000
347+++ app/ui/Playlists.qml 2015-05-02 16:02:00 +0000
348@@ -21,6 +21,7 @@
349 import QtQuick 2.3
350 import Ubuntu.Components 1.1
351 import Ubuntu.Components.Popups 1.0
352+import Ubuntu.Thumbnailer 0.1
353 import QtMultimedia 5.0
354 import QtQuick.LocalStorage 2.0
355 import "../logic/playlists.js" as Playlists

Subscribers

People subscribed via source and target branches