Merge lp:~vthompson/music-app/remix-now-playing-main-view into lp:music-app/remix

Proposed by Victor Thompson
Status: Merged
Approved by: Victor Thompson
Approved revision: 666
Merged at revision: 648
Proposed branch: lp:~vthompson/music-app/remix-now-playing-main-view
Merge into: lp:music-app/remix
Prerequisite: lp:~vthompson/music-app/remix-songs-page
Diff against target: 414 lines (+315/-17)
4 files modified
MusicNowPlaying.qml (+306/-12)
MusicToolbar.qml (+7/-4)
common/BlurredBackground.qml (+1/-1)
music-app.qml (+1/-0)
To merge this branch: bzr merge lp:~vthompson/music-app/remix-now-playing-main-view
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Andrew Hayzen Approve
Kill Animals (community) Needs Fixing
Review via email: mp+236998@code.launchpad.net

This proposal supersedes a proposal from 2014-10-03.

Commit message

Initial Now Playing toggle

Description of the change

This branch is dependent upon "remix-songs-page" as it uses the same blurred background. This is a fairly minor change and adds the toggle to switch between the views. The List View still needs to be defined by design before the code can be further simplified. A branch exists [2] that places the controls inside the view. This should only be places inside the "fullview".

[1] https://code.launchpad.net/~vthompson/music-app/remix-songs-page
[2] https://code.launchpad.net/~akiva/music-app/remix-added-controls-to-now-playing

To post a comment you must log in.
654. By Victor Thompson

Clean slate

655. By Victor Thompson

Initial Now Playing toggle

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

FAILED: Continuous integration, rev:655
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~vthompson/music-app/remix-now-playing-main-view/+merge/236998/+edit-commit-message

http://91.189.93.70:8080/job/music-app-remix-ci/14/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/673
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/673/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/14

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Kill Animals (kill-animals) wrote :

Comment on Line 44

Also, we will need to set the header to not hide when isListView === true, unless we are going to set the flickable: property. I don't know how it happened when I was testing it, but I somehow managed to make the header hide, and was not able to make it reappear, thus stranding me on that page.

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

As for the toggle when hitting back. Do you think hitting back needs to reset it back to "isListView = true"? My assumption on leaving it as last toggled was to default to the view the user last selected. However, I can see that other apps always default to the larger view. Maybe we should default to the "full view" as well once the controls are in the view. Then when the user hits back we will reset to "isListView = false".

I wasn't able to trace down where the header was getting hidden, but I get similar results occasionally. To get it to show I just tap the top of the app a few times. I agree we need to figure this out, perhaps it can wait until we make the "full view" the default as you suggest on line 44.

656. By Akiva Avraham <akiva@akiva-ThinkPad-X230>

* Add play controls to the full view of the Now Playing page

657. By Victor Thompson

Fit items into view

658. By Victor Thompson

Clean slate

659. By Victor Thompson

Initial Now Playing toggle

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

Default to the listview view for now

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
Andrew Hayzen (ahayzen) wrote :

Some inline comments (~14) most of them about margins/icons, however upon reflection i'm starting to wonder if the design's height is larger than the N4 display? See if you can do some of them to get it closer to the designs.

review: Needs Fixing
Revision history for this message
Kill Animals (kill-animals) wrote :

I will post a branch that fixes many of these things soon, so you can merge.

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

See inline replies.

661. By Victor Thompson

Update per comments

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

Merge of remix

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

Small tweaks and a toolbar hack

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
664. By Victor Thompson

update

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
665. By Victor Thompson

fix update

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 recommend we fix the coloring of the slider in a different review. I have a fix for it [1], but I want to make sure it's logical.

[1] https://code.launchpad.net/~vthompson/music-app/remix-now-playing-main-view-blue-slider/+merge/237170

666. By Victor Thompson

Only jump to track in queue view to prevent header from hiding in full view

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
Kill Animals (kill-animals) wrote :

Victor; can you please make sure to merge my branch into this one? Perhaps you did not see, but I had it ready to merge, having solved all conflicts, until you did all this work today, which... I now have to go back and resolve a bunch of conflicts.

Because of this too, you may have also done a bunch of work that was already done by myself.

Revision history for this message
Kill Animals (kill-animals) wrote :

> Victor; can you please make sure to merge my branch into this one? Perhaps you
> did not see, but I had it ready to merge, having solved all conflicts, until
> you did all this work today, which... I now have to go back and resolve a
> bunch of conflicts.
>
> Because of this too, you may have also done a bunch of work that was already
> done by myself.

Errm okay it actually wasnt that big of a deal; for some reason I thought you had done a bunch of work there, but it was actually just the old code from before. Sorry for sounding like a stickler.

Anyways nice work on all the other components. I merged from lp:~vthompson/music-app/remix-now-playing-main-view, and so my merge proposal into yours should be up to date.

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

Akiva, sorry, when I tested your branch it did not appear correct, so I went ahead and implemented the necessary fixes. This is what it looks like on the device: http://i.imgur.com/r2dqbv9.png

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

LGTM, I vote land this so we can progress with the other mps.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'MusicNowPlaying.qml'
--- MusicNowPlaying.qml 2014-10-04 01:41:50 +0000
+++ MusicNowPlaying.qml 2014-10-04 17:24:39 +0000
@@ -17,7 +17,6 @@
17 * along with this program. If not, see <http://www.gnu.org/licenses/>.17 * along with this program. If not, see <http://www.gnu.org/licenses/>.
18 */18 */
1919
20
21import QtMultimedia 5.020import QtMultimedia 5.0
22import QtQuick 2.321import QtQuick 2.3
23import QtQuick.LocalStorage 2.022import QtQuick.LocalStorage 2.0
@@ -32,16 +31,27 @@
32 objectName: "nowPlayingPage"31 objectName: "nowPlayingPage"
33 title: i18n.tr("Now Playing")32 title: i18n.tr("Now Playing")
34 visible: false33 visible: false
34 onVisibleChanged: {
35 if (!visible) {
36 // Reset the isListView property
37 // TODO: In the future this will default to false
38 isListView = true
39 }
40 }
3541
36 property int ensureVisibleIndex: 0 // ensure first index is visible at startup42 property int ensureVisibleIndex: 0 // ensure first index is visible at startup
43 property bool isListView: true
3744
38 Rectangle {45 head {
39 anchors.fill: parent46 actions: [
40 color: styleMusic.nowPlaying.backgroundColor47 Action {
41 opacity: 0.75 // change later48 objectName: "toggleView"
42 MouseArea { // Block events to lower layers49 iconName: "media-playlist"
43 anchors.fill: parent50 onTriggered: {
44 }51 isListView = !isListView
52 }
53 }
54 ]
45 }55 }
4656
47 Connections {57 Connections {
@@ -55,9 +65,10 @@
5565
56 customdebug("MusicQueue update currentIndex: " + player.source);66 customdebug("MusicQueue update currentIndex: " + player.source);
5767
58 // Always jump to current track68 // TODO: Never jump to track? Or only jump to track in queue view?
59 nowPlaying.jumpToCurrent(musicToolbar.opened, nowPlaying, musicToolbar.currentTab)69 if (isListView) {
6070 nowPlaying.jumpToCurrent(musicToolbar.opened, nowPlaying, musicToolbar.currentTab)
71 }
61 }72 }
62 }73 }
6374
@@ -73,11 +84,294 @@
7384
74 function positionAt(index) {85 function positionAt(index) {
75 queuelist.positionViewAtIndex(index, ListView.Beginning);86 queuelist.positionViewAtIndex(index, ListView.Beginning);
76 queuelist.contentY -= header.height;87 }
88
89 Rectangle {
90 id: fullview
91 visible: !isListView
92 anchors.fill: parent
93 color: "transparent"
94 clip: true
95
96 BlurredBackground {
97 id: blurredBackground
98 anchors.top: parent.top
99 anchors.topMargin: mainView.header.height
100 height: units.gu(27)
101 art: albumImage.source
102
103 Image {
104 id: albumImage
105 anchors.centerIn: parent
106 width: units.gu(18)
107 height: width
108 smooth: true
109 source: player.currentMetaArt === "" ?
110 decodeURIComponent("image://albumart/artist=" +
111 player.currentMetaArtist +
112 "&album=" + player.currentMetaAlbum)
113 : player.currentMetaArt
114 }
115 }
116
117 /* Full toolbar */
118 Item {
119 id: musicToolbarFullContainer
120 anchors.top: blurredBackground.bottom
121 anchors.topMargin: units.gu(4)
122 width: blurredBackground.width
123
124 /* Column for labels in wideAspect */
125 Column {
126 id: nowPlayingWideAspectLabels
127 spacing: units.gu(1)
128 anchors {
129 left: parent.left
130 leftMargin: units.gu(2)
131 right: parent.right
132 rightMargin: units.gu(2)
133 }
134
135 /* Title of track */
136 Label {
137 id: nowPlayingWideAspectTitle
138 anchors {
139 left: parent.left
140 leftMargin: units.gu(1)
141 right: parent.right
142 rightMargin: units.gu(1)
143 }
144 color: styleMusic.playerControls.labelColor
145 elide: Text.ElideRight
146 fontSize: "x-large"
147 objectName: "playercontroltitle"
148 text: trackQueue.model.count === 0 ? "" : player.currentMetaTitle === "" ? player.currentMetaFile : player.currentMetaTitle
149 }
150
151 /* Artist of track */
152 Label {
153 id: nowPlayingWideAspectArtist
154 anchors {
155 left: parent.left
156 leftMargin: units.gu(1)
157 right: parent.right
158 rightMargin: units.gu(1)
159 }
160 color: styleMusic.nowPlaying.labelSecondaryColor
161 elide: Text.ElideRight
162 fontSize: "small"
163 text: trackQueue.model.count === 0 ? "" : player.currentMetaArtist
164 }
165 }
166
167 /* Progress bar component */
168 MouseArea {
169 id: musicToolbarFullProgressContainer
170 anchors.left: parent.left
171 anchors.leftMargin: units.gu(3)
172 anchors.right: parent.right
173 anchors.rightMargin: units.gu(3)
174 anchors.top: nowPlayingWideAspectLabels.bottom
175 anchors.topMargin: units.gu(3)
176 height: units.gu(3)
177 width: parent.width
178
179 /* Position label */
180 Label {
181 id: musicToolbarFullPositionLabel
182 anchors.top: progressSliderMusic.bottom
183 anchors.topMargin: units.gu(-2)
184 anchors.left: parent.left
185 color: styleMusic.nowPlaying.labelSecondaryColor
186 fontSize: "small"
187 height: parent.height
188 horizontalAlignment: Text.AlignHCenter
189 text: durationToString(player.position)
190 verticalAlignment: Text.AlignVCenter
191 width: units.gu(3)
192 }
193
194 Slider {
195 id: progressSliderMusic
196 anchors.left: parent.left
197 anchors.right: parent.right
198 function formatValue(v) { return durationToString(v) }
199
200 property bool seeking: false
201
202 onSeekingChanged: {
203 if (seeking === false) {
204 musicToolbarFullPositionLabel.text = durationToString(player.position)
205 }
206 }
207
208 onPressedChanged: {
209 seeking = pressed
210 if (!pressed) {
211 player.seek(value)
212 }
213 }
214
215 Connections {
216 target: player
217 onDurationChanged: {
218 musicToolbarFullDurationLabel.text = durationToString(player.duration)
219 progressSliderMusic.maximumValue = player.duration
220 }
221 onPositionChanged: {
222 if (progressSliderMusic.seeking === false) {
223 progressSliderMusic.value = player.position
224 musicToolbarFullPositionLabel.text = durationToString(player.position)
225 musicToolbarFullDurationLabel.text = durationToString(player.duration)
226 }
227 }
228 onStopped: {
229 musicToolbarFullPositionLabel.text = durationToString(0);
230 musicToolbarFullDurationLabel.text = durationToString(0);
231 }
232 }
233 }
234
235 /* Duration label */
236 Label {
237 id: musicToolbarFullDurationLabel
238 anchors.top: progressSliderMusic.bottom
239 anchors.topMargin: units.gu(-2)
240 anchors.right: parent.right
241 color: styleMusic.nowPlaying.labelSecondaryColor
242 fontSize: "small"
243 height: parent.height
244 horizontalAlignment: Text.AlignHCenter
245 text: durationToString(player.duration)
246 verticalAlignment: Text.AlignVCenter
247 width: units.gu(3)
248 }
249 }
250
251 /* Repeat button */
252 MouseArea {
253 id: nowPlayingRepeatButton
254 objectName: "repeatShape"
255 anchors.right: nowPlayingPreviousButton.left
256 anchors.rightMargin: units.gu(1)
257 anchors.verticalCenter: nowPlayingPlayButton.verticalCenter
258 height: units.gu(6)
259 opacity: player.repeat && !emptyPage.noMusic ? 1 : .4
260 width: height
261 onClicked: player.repeat = !player.repeat
262
263 Icon {
264 id: repeatIcon
265 height: units.gu(3)
266 width: height
267 anchors.verticalCenter: parent.verticalCenter
268 anchors.horizontalCenter: parent.horizontalCenter
269 color: "white"
270 name: "media-playlist-repeat"
271 opacity: player.repeat && !emptyPage.noMusic ? 1 : .4
272 }
273 }
274
275 /* Previous button */
276 MouseArea {
277 id: nowPlayingPreviousButton
278 anchors.right: nowPlayingPlayButton.left
279 anchors.rightMargin: units.gu(1)
280 anchors.verticalCenter: nowPlayingPlayButton.verticalCenter
281 height: units.gu(6)
282 objectName: "previousShape"
283 opacity: trackQueue.model.count === 0 ? .4 : 1
284 width: height
285 onClicked: player.previousSong()
286
287 Icon {
288 id: nowPlayingPreviousIndicator
289 height: units.gu(3)
290 width: height
291 anchors.verticalCenter: parent.verticalCenter
292 anchors.horizontalCenter: parent.horizontalCenter
293 color: "white"
294 name: "media-skip-backward"
295 opacity: 1
296 }
297 }
298
299 /* Play/Pause button */
300 MouseArea {
301 id: nowPlayingPlayButton
302 anchors.horizontalCenter: parent.horizontalCenter
303 anchors.top: musicToolbarFullProgressContainer.bottom
304 anchors.topMargin: units.gu(2)
305 height: units.gu(12)
306 objectName: "playShape"
307 width: height
308 onClicked: player.toggle()
309
310 Icon {
311 id: nowPlayingPlayIndicator
312 height: units.gu(6)
313 width: height
314 anchors.verticalCenter: parent.verticalCenter
315 anchors.horizontalCenter: parent.horizontalCenter
316 opacity: emptyPage.noMusic ? .4 : 1
317 color: "white"
318 name: player.playbackState === MediaPlayer.PlayingState ? "media-playback-pause" : "media-playback-start"
319 }
320 }
321
322 /* Next button */
323 MouseArea {
324 id: nowPlayingNextButton
325 anchors.left: nowPlayingPlayButton.right
326 anchors.leftMargin: units.gu(1)
327 anchors.verticalCenter: nowPlayingPlayButton.verticalCenter
328 height: units.gu(6)
329 objectName: "forwardShape"
330 opacity: trackQueue.model.count === 0 ? .4 : 1
331 width: height
332 onClicked: player.nextSong()
333
334 Icon {
335 id: nowPlayingNextIndicator
336 height: units.gu(3)
337 width: height
338 anchors.verticalCenter: parent.verticalCenter
339 anchors.horizontalCenter: parent.horizontalCenter
340 color: "white"
341 name: "media-skip-forward"
342 opacity: 1
343 }
344 }
345
346 /* Shuffle button */
347 MouseArea {
348 id: nowPlayingShuffleButton
349 objectName: "shuffleShape"
350 anchors.left: nowPlayingNextButton.right
351 anchors.leftMargin: units.gu(1)
352 anchors.verticalCenter: nowPlayingPlayButton.verticalCenter
353 height: units.gu(6)
354 opacity: player.shuffle && !emptyPage.noMusic ? 1 : .4
355 width: height
356 onClicked: player.shuffle = !player.shuffle
357
358 Icon {
359 id: shuffleIcon
360 height: units.gu(3)
361 width: height
362 anchors.verticalCenter: parent.verticalCenter
363 anchors.horizontalCenter: parent.horizontalCenter
364 color: "white"
365 name: "media-playlist-shuffle"
366 opacity: player.shuffle && !emptyPage.noMusic ? 1 : .4
367 }
368 }
369 }
77 }370 }
78371
79 ListView {372 ListView {
80 id: queuelist373 id: queuelist
374 visible: isListView
81 objectName: "nowPlayingQueueList"375 objectName: "nowPlayingQueueList"
82 anchors {376 anchors {
83 fill: parent377 fill: parent
84378
=== modified file 'MusicToolbar.qml'
--- MusicToolbar.qml 2014-10-01 20:51:17 +0000
+++ MusicToolbar.qml 2014-10-04 17:24:39 +0000
@@ -113,13 +113,16 @@
113 right: parent.right113 right: parent.right
114 bottom: parent.bottom114 bottom: parent.bottom
115 }115 }
116 height: currentMode === "full" ? fullHeight : expandedHeight116 // TODO: this will be removed when the toolbar is redone
117 height: currentMode === "hidden" ? 0 : (currentMode === "full" ? fullHeight : expandedHeight)
117 locked: true118 locked: true
118 opened: true119 opened: true
119120
121 // TODO: this will be removed when the toolbar is redone
120 // The current mode of the controls122 // The current mode of the controls
121 property string currentMode: wideAspect || (currentPage === nowPlaying)123 property string currentMode: !nowPlaying.isListView && currentPage === nowPlaying
122 ? "full" : "expanded"124 ? "hidden" : (wideAspect || currentPage === nowPlaying
125 ? "full" : "expanded")
123126
124 // Properties for the different heights127 // Properties for the different heights
125 property int expandedHeight: units.gu(7.25)128 property int expandedHeight: units.gu(7.25)
@@ -721,7 +724,7 @@
721 height: units.gu(2.5)724 height: units.gu(2.5)
722 name: player.playbackState === MediaPlayer.PlayingState ?725 name: player.playbackState === MediaPlayer.PlayingState ?
723 "media-playback-pause" : "media-playback-start"726 "media-playback-pause" : "media-playback-start"
724 objectName: "smallPlayShape"727 objectName: "disabledSmallPlayShape"
725 width: height728 width: height
726 }729 }
727730
728731
=== modified file 'common/BlurredBackground.qml'
--- common/BlurredBackground.qml 2014-10-03 01:13:43 +0000
+++ common/BlurredBackground.qml 2014-10-04 17:24:39 +0000
@@ -23,7 +23,7 @@
2323
24// Blurred background24// Blurred background
25Rectangle {25Rectangle {
26 anchors.fill: parent26 width: parent.width
27 property string art // : player.currentMetaFile === "" ? Qt.resolvedUrl("../images/music-app-cover@30.png") : player.currentMetaArt27 property string art // : player.currentMetaFile === "" ? Qt.resolvedUrl("../images/music-app-cover@30.png") : player.currentMetaArt
2828
29 // dark layer29 // dark layer
3030
=== modified file 'music-app.qml'
--- music-app.qml 2014-10-04 01:41:50 +0000
+++ music-app.qml 2014-10-04 17:24:39 +0000
@@ -962,6 +962,7 @@
962962
963 MusicToolbar {963 MusicToolbar {
964 id: musicToolbar964 id: musicToolbar
965 visible: nowPlaying.isListView || !nowPlaying.visible
965 objectName: "musicToolbarObject"966 objectName: "musicToolbarObject"
966 z: 200 // put on top of everything else967 z: 200 // put on top of everything else
967 }968 }

Subscribers

People subscribed via source and target branches