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

Subscribers

People subscribed via source and target branches