Ubuntu Music App

Merge lp:~ahayzen/music-app/now-playing-page-stack into lp:music-app/trusty

  1. now-playing-page-stack
  2. Merge into trusty
Proposed by Andrew Hayzen
Status: Merged
Approved by: Victor Thompson
Approved revision: 368
Merged at revision: 416
Proposed branch: lp:~ahayzen/music-app/now-playing-page-stack
Merge into: lp:music-app/trusty
Diff against target: 247 lines (+73/-84)
3 files modified
MusicNowPlaying.qml (+6/-30)
MusicToolbar.qml (+2/-3)
music-app.qml (+65/-51)
To merge this branch: bzr merge lp:~ahayzen/music-app/now-playing-page-stack
Reviewer Review Type Date Requested Status
Victor Thompson Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Andrew Hayzen Approve
Review via email: mp+209557@code.launchpad.net

Commit message

* Moved now playing page to be stacked on top of tabs
 * Don't hide back button as too confusing with header appearing/disappearing
 * Show old back button until lp:1256424 lands and show below header

Description of the change

 * Moved now playing page to be stacked ontop of tabs
 * Don't hide back button as too confusing with header appearing/disappearing
 * Show old back button until lp:1256424 lands and show below header

To post a comment you must log in.
Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Blocking until the new back button in the header lands in the SDK.

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

FAILED: Continuous integration, rev:362
http://91.189.93.70:8080/job/music-app-ci/615/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1588
    SUCCESS: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/615
    SUCCESS: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/617
    SUCCESS: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/336

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

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

The branch to land the back button in the header is currently a WIP, but we can track the MP [1]. There's also a blueprint [2] with other items that are header related as well, for those playing along at home.

[1] https://code.launchpad.net/~tpeeters/ubuntu-ui-toolkit/back-in-header/+merge/202471
[2] https://blueprints.launchpad.net/ubuntu-ui-toolkit/+spec/new-header

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

FAILED: Continuous integration, rev:363
http://91.189.93.70:8080/job/music-app-ci/616/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1589
    SUCCESS: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/616
    SUCCESS: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/618
    SUCCESS: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/337

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

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

Unblocking as bug 1256424 is going to be fixed after 14.04, therefore we are keeping our old back button to resolve bug 1239106 before 14.04 :)

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

PASSED: Continuous integration, rev:368
http://91.189.93.70:8080/job/music-app-ci/694/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2031
    SUCCESS: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/694
    SUCCESS: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/696
    SUCCESS: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/415

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

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

Currently the back button does not hide with the toolbar. This makes the behavior a bit odd. Was this done because it will eventually always be shown in the header? I think we should consider reverting to hiding it as we did before.

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

The reason I made the back button stay visible is

a) Because the back button will move to the header as you stated
b) Because the of the way the header is shown/hidden as you scroll up/down the list it becomes really confusing with the header/back button/toolbar all appearing and disappearing. Even I, who wrote it, became confused and wondered where the back button had gone. Therefore I thought the most sane solution for the interim period until the back button lands in the header would be to make it always visible.

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

Ok, that is reasonable.

Thanks! I'm sure we're all happy putting this bug down!

review: Approve

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-03-28 16:21:36 +0000
3+++ MusicNowPlaying.qml 2014-04-10 17:35:34 +0000
4@@ -30,23 +30,14 @@
5 Page {
6 id: nowPlaying
7 objectName: "nowplayingpage"
8- anchors.fill: parent
9- title: i18n.tr("Queue")
10+ title: i18n.tr("Now Playing")
11+ tools: null
12 visible: false
13
14 onVisibleChanged: {
15 if (visible === true)
16 {
17- header.hide();
18- header.opacity = 0;
19- header.enabled = false;
20- musicToolbar.setPage(nowPlaying, musicToolbar.currentPage);
21- }
22- else
23- {
24- header.enabled = true;
25- header.opacity = 1;
26- header.show();
27+ musicToolbar.setPage(nowPlaying, null, tabs.pageStack);
28 }
29 }
30
31@@ -808,32 +799,17 @@
32 }
33 }
34
35+ // TODO: Remove back button once lp:1256424 is fixed (button will be in header)
36 Rectangle {
37 id: nowPlayingBackButton
38 anchors {
39 left: parent.left
40 right: parent.right
41 }
42+
43 color: styleMusic.toolbar.fullBackgroundColor
44 height: units.gu(3.1)
45-
46- state: musicToolbar.opened ? "shown" : "hidden"
47- states: [
48- State {
49- name: "shown"
50- AnchorChanges {
51- target: nowPlayingBackButton
52- anchors.top: parent.top
53- }
54- },
55- State {
56- name: "hidden"
57- AnchorChanges {
58- target: nowPlayingBackButton
59- anchors.bottom: parent.top
60- }
61- }
62- ]
63+ y: header.y + header.height
64
65 Image {
66 id: expandItem
67
68=== modified file 'MusicToolbar.qml'
69--- MusicToolbar.qml 2014-04-02 00:12:58 +0000
70+++ MusicToolbar.qml 2014-04-10 17:35:34 +0000
71@@ -203,7 +203,7 @@
72 /* Clicking in the area shows the queue */
73 function trigger() {
74 if (trackQueue.model.count !== 0 && currentPage !== nowPlaying) {
75- nowPlaying.visible = true;
76+ tabs.setNowPlaying(true);
77 }
78 }
79
80@@ -651,7 +651,6 @@
81 anchors.fill: parent
82 color: styleMusic.playerControls.backgroundColor
83 state: trackQueue.model.count === 0 ? "disabled" : "enabled"
84-
85 states: [
86 State {
87 name: "disabled"
88@@ -978,7 +977,7 @@
89 anchors.fill: playerControlLabelContainer
90 color: "transparent"
91 function trigger() {
92- nowPlaying.visible = true;
93+ tabs.setNowPlaying(true);
94 }
95 }
96 }
97
98=== modified file 'music-app.qml'
99--- music-app.qml 2014-04-09 20:30:24 +0000
100+++ music-app.qml 2014-04-10 17:35:34 +0000
101@@ -449,7 +449,7 @@
102 console.log("Is current track: "+player.playbackState)
103
104 // Show the Now Playing page and make sure the track is visible
105- nowPlaying.visible = true;
106+ tabs.setNowPlaying(true);
107 nowPlaying.ensureVisibleIndex = index;
108
109 musicToolbar.showToolbar();
110@@ -478,7 +478,7 @@
111 player.playSong(file, index)
112
113 // Show the Now Playing page and make sure the track is visible
114- nowPlaying.visible = true;
115+ tabs.setNowPlaying(true);
116 nowPlaying.ensureVisibleIndex = index;
117
118 musicToolbar.showToolbar();
119@@ -975,58 +975,58 @@
120 z: 200 // put on top of everything else
121 }
122
123+ Page {
124+ id: emptyPage
125+ title: i18n.tr("Music")
126+ visible: false
127+
128+ property bool noMusic: griloModel.count === 0 && griloModel.loaded === true
129+
130+ onNoMusicChanged: {
131+ if (noMusic)
132+ pageStack.push(emptyPage)
133+ else if (pageStack.currentPage == emptyPage)
134+ pageStack.pop()
135+ }
136+
137+ tools: ToolbarItems {
138+ back: null
139+ locked: true
140+ opened: false
141+ }
142+
143+ // Overlay to show when no tracks detected on the device
144+ Rectangle {
145+ id: libraryEmpty
146+ anchors.fill: parent
147+ anchors.topMargin: -emptyPage.header.height
148+ color: styleMusic.libraryEmpty.backgroundColor
149+
150+ Column {
151+ anchors.centerIn: parent
152+
153+
154+ Label {
155+ anchors.horizontalCenter: parent.horizontalCenter
156+ color: styleMusic.libraryEmpty.labelColor
157+ fontSize: "large"
158+ font.bold: true
159+ text: "No music found"
160+ }
161+
162+ Label {
163+ anchors.horizontalCenter: parent.horizontalCenter
164+ color: styleMusic.libraryEmpty.labelColor
165+ fontSize: "medium"
166+ text: "Please import music and restart the app"
167+ }
168+ }
169+ }
170+ }
171+
172 PageStack {
173 id: pageStack
174
175- Page {
176- id: emptyPage
177- title: i18n.tr("Music")
178- visible: false
179-
180- property bool noMusic: griloModel.count === 0 && griloModel.loaded === true
181-
182- onNoMusicChanged: {
183- if (noMusic)
184- pageStack.push(emptyPage)
185- else if (pageStack.currentPage == emptyPage)
186- pageStack.pop()
187- }
188-
189- tools: ToolbarItems {
190- back: null
191- locked: true
192- opened: false
193- }
194-
195- // Overlay to show when no tracks detected on the device
196- Rectangle {
197- id: libraryEmpty
198- anchors.fill: parent
199- anchors.topMargin: -emptyPage.header.height
200- color: styleMusic.libraryEmpty.backgroundColor
201-
202- Column {
203- anchors.centerIn: parent
204-
205-
206- Label {
207- anchors.horizontalCenter: parent.horizontalCenter
208- color: styleMusic.libraryEmpty.labelColor
209- fontSize: "large"
210- font.bold: true
211- text: "No music found"
212- }
213-
214- Label {
215- anchors.horizontalCenter: parent.horizontalCenter
216- color: styleMusic.libraryEmpty.labelColor
217- fontSize: "medium"
218- text: "Please import music and restart the app"
219- }
220- }
221- }
222- }
223-
224 Tabs {
225 id: tabs
226 anchors {
227@@ -1151,6 +1151,20 @@
228 loading.visible = selectedTab.loading || !selectedTab.populated
229 }
230
231+ function setNowPlaying(visible)
232+ {
233+ if (visible) {
234+ pageStack.push(nowPlaying);
235+ }
236+ else {
237+ if (pageStack.currentPage === nowPlaying) {
238+ pageStack.pop()
239+ }
240+ }
241+ }
242+
243+ Component.onCompleted: musicToolbar.currentTab = selectedTab
244+
245 onSelectedTabChanged: {
246 // pause loading of the models in the old tab
247 if (musicToolbar.currentTab !== selectedTab &&

Subscribers

People subscribed via source and target branches

to status/vote changes: