ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrew Hayzen <ahayzen@xxxxxxxxx>]

Andrew Hayzen has proposed merging lp:~ahayzen/music-app/fix-child-page-freezes into lp:music-app.

Commit message:
* Fix child page freezes when stacked onto recent or playlists

Requested reviews:
  Music App Developers (music-app-dev)

For more details, see:
https://code.launchpad.net/~ahayzen/music-app/fix-child-page-freezes/+merge/263188

* Fix child page freezes when stacked onto recent or playlists

This fixes issues when something changes on/childof recent/playlists and you are currently viewing a page that was spawned from them. If you refresh the models you can make the views freeze or not act correctly, this uses the current tab to detect if you are parent of them.

Note we had a solution for this before that wasn't as 'tidy' and simple but was also partially broken/reverted, I wonder if we had a bad merge conflict resolution in the /refactor switch.
-- 
Your team Music App Developers is requested to review the proposed merge of lp:~ahayzen/music-app/fix-child-page-freezes into lp:music-app.

=== modified file 'app/components/Queue.qml'
--- app/components/Queue.qml	2015-06-06 21:11:10 +0000
+++ app/components/Queue.qml	2015-06-27 18:31:22 +0000
@@ -72,7 +72,6 @@
         reorderable: true
         rightSideActions: [
             AddToPlaylist{
-
             }
         ]
 

=== modified file 'app/music-app.qml'
--- app/music-app.qml	2015-06-02 05:01:05 +0000
+++ app/music-app.qml	2015-06-27 18:31:22 +0000
@@ -903,7 +903,6 @@
                 }
             }
 
-
             // fifth tab is the playlists
             Tab {
                 property bool populated: false

=== modified file 'app/ui/AddToPlaylist.qml'
--- app/ui/AddToPlaylist.qml	2015-02-16 20:27:40 +0000
+++ app/ui/AddToPlaylist.qml	2015-06-27 18:31:22 +0000
@@ -60,7 +60,6 @@
     ]
 
     property var chosenElements: []
-    property var page
 
     onVisibleChanged: {
         // Load the playlistmodel if it hasn't loaded or is empty
@@ -96,27 +95,26 @@
             onClicked: {
                 Playlists.addToPlaylistList(name, chosenElements)
 
-                // Check that the parent parent page is not being refiltered
-                if (page !== undefined && page.page !== undefined && page.page.title === i18n.tr("Playlists")) {
-                    page.page.changed = true
+                if (tabs.selectedTab.title === i18n.tr("Playlists")) {
+                    // If we are on a page above playlists then set changed
+                    tabs.selectedTab.page.changed = true;
+                    tabs.selectedTab.page.childrenChanged = true;
                 } else {
+                    // Otherwise just reload the playlists
                     playlistModel.filterPlaylists();
                 }
 
                 if (Library.recentContainsPlaylist(name)) {
-                    // Check that the parent parent page is not being refiltered
-                    if (page !== undefined && page.page !== undefined && page.page.title === i18n.tr("Recent")) {
-                        page.page.changed = true
+                    if (tabs.selectedTab.title === i18n.tr("Recent")) {
+                        // If we are on a page above recent then set changed
+                        tabs.selectedTab.page.changed = true;
+                        tabs.selectedTab.page.childrenChanged = true;
                     } else {
-                        recentModel.filterRecent()
+                        // Otherwise just reload recent
+                        recentModel.filterRecent();
                     }
                 }
 
-                if (page !== undefined && name === page.line2 && page.playlistChanged !== undefined) {
-                    page.playlistChanged = true
-                    page.covers = Playlists.getPlaylistCovers(name)
-                }
-
                 mainPageStack.goBack();  // go back to the previous page
             }
         }

=== modified file 'app/ui/Playlists.qml'
--- app/ui/Playlists.qml	2015-04-28 17:37:33 +0000
+++ app/ui/Playlists.qml	2015-06-27 18:31:22 +0000
@@ -52,6 +52,7 @@
     ]
 
     property bool changed: false
+    property bool childrenChanged: false
 
     onVisibleChanged: {
         if (changed) {

=== modified file 'app/ui/Recent.qml'
--- app/ui/Recent.qml	2015-02-16 20:27:40 +0000
+++ app/ui/Recent.qml	2015-06-27 18:31:22 +0000
@@ -37,6 +37,7 @@
     title: i18n.tr("Recent")
 
     property bool changed: false
+    property bool childrenChanged: false
 
     onVisibleChanged: {
         if (changed) {

=== modified file 'app/ui/Songs.qml'
--- app/ui/Songs.qml	2015-02-16 20:27:40 +0000
+++ app/ui/Songs.qml	2015-06-27 18:31:22 +0000
@@ -117,7 +117,6 @@
                 AddToQueue {
                 },
                 AddToPlaylist {
-
                 }
             ]
 

=== modified file 'app/ui/SongsView.qml'
--- app/ui/SongsView.qml	2015-04-28 17:37:33 +0000
+++ app/ui/SongsView.qml	2015-06-27 18:31:22 +0000
@@ -53,11 +53,9 @@
 
     property bool loaded: false  // used to detect difference between first and further loads
 
-    property bool playlistChanged: false
-
     onVisibleChanged: {
-        if (playlistChanged) {
-            playlistChanged = false
+        if (page.childrenChanged) {
+            page.childrenChanged = false
             refreshWaitTimer.start()
         }
     }
@@ -69,8 +67,9 @@
             if (songStackPage.line1 === i18n.tr("Playlist")) {
                 if (songStackPage.visible) {
                     albumTracksModel.filterPlaylistTracks(line2)
+                    covers = Playlists.getPlaylistCovers(line2)
                 } else {
-                    songStackPage.playlistChanged = true
+                    songStackPage.page.childrenChanged = true;
                 }
             }
         }
@@ -82,6 +81,18 @@
         onTriggered: albumTracksModel.filterPlaylistTracks(line2)
     }
 
+    function recentChangedHelper()
+    {
+        // if parent Recent then set changed otherwise refilter
+        if (songStackPage.page.title === i18n.tr("Recent")) {
+            if (songStackPage.page !== undefined) {
+                songStackPage.page.changed = true
+            }
+        } else {
+            recentModel.filterRecent()
+        }
+    }
+
     function playlistChangedHelper(force)
     {
         force = force === undefined ? false : force  // default force to false
@@ -96,14 +107,7 @@
         }
 
         if (Library.recentContainsPlaylist(songStackPage.line2) || force) {
-            // if parent Recent then set changed otherwise refilter
-            if (songStackPage.page.title === i18n.tr("Recent")) {
-                if (songStackPage.page !== undefined) {
-                    songStackPage.page.changed = true
-                }
-            } else {
-                recentModel.filterRecent()
-            }
+            recentChangedHelper();
         }
     }
 
@@ -197,7 +201,7 @@
                             console.debug("Unknown type to add to recent")
                         }
 
-                        recentModel.filterRecent()
+                        recentChangedHelper();
                     }
                 }
                 QueueAllButton {
@@ -216,7 +220,7 @@
                             console.debug("Unknown type to add to recent")
                         }
 
-                        recentModel.filterRecent()
+                        recentChangedHelper();
                     }
                 }
             }
@@ -316,7 +320,6 @@
 
                 },
                 AddToPlaylist {
-
                 }
             ]
 
@@ -331,7 +334,7 @@
                     console.debug("Unknown type to add to recent")
                 }
 
-                recentModel.filterRecent()
+                recentChangedHelper();
             }
             onReorder: {
                 console.debug("Move: ", from, to);