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
=== modified file 'app/components/ColumnFlow.qml'
--- app/components/ColumnFlow.qml 2015-01-11 17:40:13 +0000
+++ app/components/ColumnFlow.qml 2015-05-02 16:02:00 +0000
@@ -40,6 +40,7 @@
40 property bool removing: false40 property bool removing: false
41 property bool restoring: false // is the view restoring?41 property bool restoring: false // is the view restoring?
42 property var restoreItems: ({}) // when rebuilding items are stored here temporarily42 property var restoreItems: ({}) // when rebuilding items are stored here temporarily
43 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)
4344
44 onColumnWidthChanged: {45 onColumnWidthChanged: {
45 if (restoring) {46 if (restoring) {
@@ -62,6 +63,10 @@
62 }63 }
6364
64 onVisibleChanged: {65 onVisibleChanged: {
66 if (visible) {
67 preloading = false;
68 }
69
65 if (visible && delayRebuildIndex !== -1) { // restore from count change70 if (visible && delayRebuildIndex !== -1) { // restore from count change
66 if (delayRebuildIndex === 0) {71 if (delayRebuildIndex === 0) {
67 reset()72 reset()
@@ -86,7 +91,7 @@
86 Connections {91 Connections {
87 target: model === undefined ? fakeModel : model92 target: model === undefined ? fakeModel : model
88 onModelReset: {93 onModelReset: {
89 if (!visible) {94 if (!visible && lastIndex > 0 && !preloading) {
90 delayRebuildIndex = 095 delayRebuildIndex = 0
91 } else {96 } else {
92 reset()97 reset()
@@ -94,7 +99,7 @@
94 }99 }
95 }100 }
96 onRowsInserted: {101 onRowsInserted: {
97 if (!visible) {102 if (!visible && lastIndex > 0 && !preloading) {
98 setDelayRebuildIndex(first)103 setDelayRebuildIndex(first)
99 } else {104 } else {
100 if (first <= lastIndex) {105 if (first <= lastIndex) {
@@ -161,7 +166,8 @@
161 // and166 // and
162 // allow if the y position is within the viewport167 // allow if the y position is within the viewport
163 // or if loadBefore is true then allow if the y position is before the viewport168 // or if loadBefore is true then allow if the y position is before the viewport
164 if (((count > 0 && lastIndex < count && insertMax === undefined) || (insertMax !== undefined && lastIndex <= insertMax)) && (inViewport(y, 0) || (loadBefore === true && beforeViewport(y)))) {169 if (((count > 0 && lastIndex < count && insertMax === undefined) || (insertMax !== undefined && lastIndex <= insertMax))
170 && (inViewport(y, 0) || (loadBefore === true && beforeViewport(y)))) {
165 incubateObject(lastIndex++, columnsByHeight[i], getMaxInColumn(columnsByHeight[i]), append);171 incubateObject(lastIndex++, columnsByHeight[i], getMaxInColumn(columnsByHeight[i]), append);
166 workDone = true172 workDone = true
167 } else {173 } else {
@@ -489,4 +495,15 @@
489 delayRebuildIndex = index495 delayRebuildIndex = index
490 }496 }
491 }497 }
498
499 Component.onCompleted: {
500 // Ensure that initial column vars are set
501 for (var j=0; j < columns; j++) {
502 columnHeights.push({})
503 }
504
505 cacheColumnHeights()
506
507 append(true)
508 }
492}509}
493510
=== modified file 'app/components/Flickables/CardView.qml'
--- app/components/Flickables/CardView.qml 2015-02-08 04:06:28 +0000
+++ app/components/Flickables/CardView.qml 2015-05-02 16:02:00 +0000
@@ -35,17 +35,11 @@
35 property alias delegate: flow.delegate35 property alias delegate: flow.delegate
36 property var getter36 property var getter
37 property alias header: headerLoader.sourceComponent37 property alias header: headerLoader.sourceComponent
38 property var model: flow.model38 property alias model: flow.model
39 property real itemWidth: units.gu(15)39 property real itemWidth: units.gu(15)
4040
41 onGetterChanged: flow.getter = getter // cannot use alias to set a function (must be var)41 onGetterChanged: flow.getter = getter // cannot use alias to set a function (must be var)
4242
43 onVisibleChanged: {
44 if (visible) { // only load model once CardView is visible
45 flow.model = model
46 }
47 }
48
49 Loader {43 Loader {
50 id: headerLoader44 id: headerLoader
51 visible: sourceComponent !== undefined45 visible: sourceComponent !== undefined
5246
=== modified file 'app/components/MusicToolbar.qml'
--- app/components/MusicToolbar.qml 2015-02-04 16:43:17 +0000
+++ app/components/MusicToolbar.qml 2015-05-02 16:02:00 +0000
@@ -20,6 +20,7 @@
20import QtQuick 2.320import QtQuick 2.3
21import QtMultimedia 5.021import QtMultimedia 5.0
22import Ubuntu.Components 1.122import Ubuntu.Components 1.1
23import Ubuntu.Thumbnailer 0.1
2324
24Rectangle {25Rectangle {
25 anchors {26 anchors {
2627
=== modified file 'app/music-app.qml'
--- app/music-app.qml 2015-04-29 00:55:02 +0000
+++ app/music-app.qml 2015-05-02 16:02:00 +0000
@@ -458,10 +458,12 @@
458458
459 // TODO: improve in refactoring to be able detect when a track is removed459 // TODO: improve in refactoring to be able detect when a track is removed
460 // Update playlists page460 // Update playlists page
461 if (playlistsPage.visible) {461 if (playlistsPageLoader.status === Loader.Ready) {
462 playlistModel.filterPlaylists()462 if (playlistsPageLoader.item.visible) {
463 } else {463 playlistModel.filterPlaylists()
464 playlistsPage.changed = true464 } else {
465 playlistsPageLoader.item.changed = true
466 }
465 }467 }
466 }468 }
467 }469 }
@@ -497,10 +499,12 @@
497 console.debug("Removed recent:", JSON.stringify(removed))499 console.debug("Removed recent:", JSON.stringify(removed))
498 Library.recentRemoveAlbums(removed)500 Library.recentRemoveAlbums(removed)
499501
500 if (recentPage.visible) {502 if (recentPageLoader.status === Loader.Ready) {
501 recentModel.filterRecent()503 if (recentPageLoader.item.visible) {
502 } else {504 recentModel.filterRecent()
503 recentPage.changed = true505 } else {
506 recentPageLoader.item.changed = true
507 }
504 }508 }
505 }509 }
506 }510 }
@@ -756,6 +760,7 @@
756 }760 }
757761
758 property Tab lastTab: selectedTab762 property Tab lastTab: selectedTab
763 property bool firstTabAyncLoadComplete: false
759764
760 onSelectedTabChanged: {765 onSelectedTabChanged: {
761 // pause loading of the models in the old tab766 // pause loading of the models in the old tab
@@ -797,11 +802,23 @@
797 id: recentTab802 id: recentTab
798 objectName: "recentTab"803 objectName: "recentTab"
799 anchors.fill: parent804 anchors.fill: parent
800 title: page.title805 title: i18n.tr("Recent")
801806
802 // Tab content begins here807 // Tab content begins here
803 page: Recent {808 page: Loader {
804 id: recentPage809 id: recentPageLoader
810 anchors {
811 fill: parent
812 }
813 active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
814 asynchronous: true
815 source: "ui/Recent.qml"
816
817 onStatusChanged: {
818 if (status == Loader.Ready) {
819 tabs.firstTabAyncLoadComplete = true
820 }
821 }
805 }822 }
806 }823 }
807 }824 }
@@ -844,11 +861,23 @@
844 id: artistsTab861 id: artistsTab
845 objectName: "artistsTab"862 objectName: "artistsTab"
846 anchors.fill: parent863 anchors.fill: parent
847 title: page.title864 title: i18n.tr("Artists")
848865
849 // tab content866 // tab content
850 page: Artists {867 page: Loader {
851 id: artistsPage868 id: artistsPageLoader
869 anchors {
870 fill: parent
871 }
872 active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
873 asynchronous: true
874 source: "ui/Artists.qml"
875
876 onStatusChanged: {
877 if (status == Loader.Ready) {
878 tabs.firstTabAyncLoadComplete = true
879 }
880 }
852 }881 }
853 }882 }
854883
@@ -861,11 +890,23 @@
861 id: albumsTab890 id: albumsTab
862 objectName: "albumsTab"891 objectName: "albumsTab"
863 anchors.fill: parent892 anchors.fill: parent
864 title: page.title893 title: i18n.tr("Albums")
865894
866 // Tab content begins here895 // Tab content begins here
867 page: Albums {896 page: Loader {
868 id: albumsPage897 id: albumsPageLoader
898 anchors {
899 fill: parent
900 }
901 active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
902 asynchronous: true
903 source: "ui/Albums.qml"
904
905 onStatusChanged: {
906 if (status == Loader.Ready) {
907 tabs.firstTabAyncLoadComplete = true
908 }
909 }
869 }910 }
870 }911 }
871912
@@ -878,11 +919,23 @@
878 id: genresTab919 id: genresTab
879 objectName: "genresTab"920 objectName: "genresTab"
880 anchors.fill: parent921 anchors.fill: parent
881 title: page.title922 title: i18n.tr("Genres")
882923
883 // Tab content begins here924 // Tab content begins here
884 page: Genres {925 page: Loader {
885 id: genresPage926 id: genresPageLoader
927 anchors {
928 fill: parent
929 }
930 active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
931 asynchronous: true
932 source: "ui/Genres.qml"
933
934 onStatusChanged: {
935 if (status == Loader.Ready) {
936 tabs.firstTabAyncLoadComplete = true
937 }
938 }
886 }939 }
887 }940 }
888941
@@ -895,11 +948,23 @@
895 id: songsTab948 id: songsTab
896 objectName: "songsTab"949 objectName: "songsTab"
897 anchors.fill: parent950 anchors.fill: parent
898 title: page.title951 title: i18n.tr("Songs")
899952
900 // Tab content begins here953 // Tab content begins here
901 page: Songs {954 page: Loader {
902 id: tracksPage955 id: songsPageLoader
956 anchors {
957 fill: parent
958 }
959 active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
960 asynchronous: true
961 source: "ui/Songs.qml"
962
963 onStatusChanged: {
964 if (status == Loader.Ready) {
965 tabs.firstTabAyncLoadComplete = true
966 }
967 }
903 }968 }
904 }969 }
905970
@@ -913,17 +978,33 @@
913 id: playlistsTab978 id: playlistsTab
914 objectName: "playlistsTab"979 objectName: "playlistsTab"
915 anchors.fill: parent980 anchors.fill: parent
916 title: page.title981 title: i18n.tr("Playlists")
917982
918 // Tab content begins here983 // Tab content begins here
919 page: Playlists {984 page: Loader {
920 id: playlistsPage985 id: playlistsPageLoader
986 anchors {
987 fill: parent
988 }
989 active: tabs.firstTabAyncLoadComplete && musicToolbar.status == Loader.Ready
990 asynchronous: true
991 source: "ui/Playlists.qml"
992
993 onStatusChanged: {
994 if (status == Loader.Ready) {
995 tabs.firstTabAyncLoadComplete = true
996 }
997 }
921 }998 }
922 }999 }
9231000
924 // Set the models in the tab to allow/disallow loading1001 // Set the models in the tab to allow/disallow loading
925 function allowLoading(tabToLoad, state)1002 function allowLoading(tabToLoad, state)
926 {1003 {
1004 if (loadedUI) {
1005 selectedTab.page.active = true
1006 }
1007
927 if (tabToLoad && tabToLoad.model !== undefined)1008 if (tabToLoad && tabToLoad.model !== undefined)
928 {1009 {
929 for (var i=0; i < tabToLoad.model.length; i++)1010 for (var i=0; i < tabToLoad.model.length; i++)
9301011
=== modified file 'app/ui/Albums.qml'
--- app/ui/Albums.qml 2015-02-16 20:27:40 +0000
+++ app/ui/Albums.qml 2015-05-02 16:02:00 +0000
@@ -20,6 +20,7 @@
20import QtQuick 2.320import QtQuick 2.3
21import Ubuntu.Components 1.121import Ubuntu.Components 1.1
22import Ubuntu.MediaScanner 0.122import Ubuntu.MediaScanner 0.1
23import Ubuntu.Thumbnailer 0.1
23import "../components"24import "../components"
24import "../components/Delegates"25import "../components/Delegates"
25import "../components/Flickables"26import "../components/Flickables"
2627
=== modified file 'app/ui/Genres.qml'
--- app/ui/Genres.qml 2015-02-16 20:27:40 +0000
+++ app/ui/Genres.qml 2015-05-02 16:02:00 +0000
@@ -20,6 +20,7 @@
20import QtQuick 2.320import QtQuick 2.3
21import Ubuntu.Components 1.121import Ubuntu.Components 1.1
22import Ubuntu.MediaScanner 0.122import Ubuntu.MediaScanner 0.1
23import Ubuntu.Thumbnailer 0.1
23import "../components"24import "../components"
24import "../components/Delegates"25import "../components/Delegates"
25import "../components/Flickables"26import "../components/Flickables"
2627
=== modified file 'app/ui/Playlists.qml'
--- app/ui/Playlists.qml 2015-04-28 17:37:33 +0000
+++ app/ui/Playlists.qml 2015-05-02 16:02:00 +0000
@@ -21,6 +21,7 @@
21import QtQuick 2.321import QtQuick 2.3
22import Ubuntu.Components 1.122import Ubuntu.Components 1.1
23import Ubuntu.Components.Popups 1.023import Ubuntu.Components.Popups 1.0
24import Ubuntu.Thumbnailer 0.1
24import QtMultimedia 5.025import QtMultimedia 5.0
25import QtQuick.LocalStorage 2.026import QtQuick.LocalStorage 2.0
26import "../logic/playlists.js" as Playlists27import "../logic/playlists.js" as Playlists

Subscribers

People subscribed via source and target branches