Merge lp:~ahayzen/music-app/remix-store-tab-index into lp:music-app/remix

Proposed by Andrew Hayzen
Status: Merged
Approved by: Victor Thompson
Approved revision: 744
Merged at revision: 741
Proposed branch: lp:~ahayzen/music-app/remix-store-tab-index
Merge into: lp:music-app/remix
Diff against target: 41 lines (+12/-3)
1 file modified
music-app.qml (+12/-3)
To merge this branch: bzr merge lp:~ahayzen/music-app/remix-store-tab-index
Reviewer Review Type Date Requested Status
Victor Thompson Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Review via email: mp+241877@code.launchpad.net

Commit message

* Restore the tab index the user was on when starting the app

Description of the change

* Restore the tab index the user was on when starting the app

TESTING:
* Test when you have no settings and no recent - Expect Albums
* Test when you have no setting but do have recent - Expect Recent
* Test when you have settings (already stored tabIndex) - Expect stored result from last application run

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

I tested this to force the stored tabIndex to be larger then the number of possible tabs and the initial view it lands on (Recent) is empty until a different tab is selected. I think we might want to ensure that the index we set "tabs.selectedTabIndex" to is within the number of possible tabs.

review: Needs Fixing
742. By Andrew Hayzen

* Protect against a too high saved index

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

Shouldn't the condition being added be an OR rather than and AND?

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

Oh damn I was still thinking the wrong way around as I was originally going todo !== -1 :P

743. By Andrew Hayzen

* Switch to using OR

744. By Andrew Hayzen

* Actually fix it this time :P

Revision history for this message
Victor Thompson (vthompson) wrote :

LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'music-app.qml'
2--- music-app.qml 2014-11-12 01:45:00 +0000
3+++ music-app.qml 2014-11-15 21:56:24 +0000
4@@ -47,6 +47,7 @@
5 category: "StartupSettings"
6
7 property int queueIndex: 0
8+ property int tabIndex: -1
9 }
10
11 // Global keyboard shortcuts
12@@ -564,11 +565,13 @@
13 // push the page to view
14 mainPageStack.push(tabs)
15
16+ // if a tab index exists restore it, otherwise goto Recent if there are items otherwise go to Albums
17+ tabs.selectedTabIndex = startupSettings.tabIndex === -1 || startupSettings.tabIndex > tabs.count - 1
18+ ? (Library.isRecentEmpty() ? albumsTab.index : startTab.index)
19+ : startupSettings.tabIndex
20+
21 loadedUI = true;
22
23- // goto Recent if there are items otherwise go to Albums
24- tabs.selectedTabIndex = Library.isRecentEmpty() ? albumsTab.index : startTab.index
25-
26 // Run post load
27 tabs.ensurePopulated(tabs.selectedTab);
28
29@@ -893,6 +896,12 @@
30 fill: parent
31 }
32
33+ onSelectedTabIndexChanged: {
34+ if (loadedUI) { // store the tab index if changed by the user
35+ startupSettings.tabIndex = selectedTabIndex
36+ }
37+ }
38+
39 // First tab is all music
40 Tab {
41 property bool populated: false

Subscribers

People subscribed via source and target branches