Ubuntu Music App

Merge lp:~ahayzen/music-app/remix-add-card-view into lp:music-app/remix

  1. remix-add-card-view
  2. Merge into remix
Proposed by Andrew Hayzen
Status: Merged
Approved by: Victor Thompson
Approved revision: 653
Merged at revision: 653
Proposed branch: lp:~ahayzen/music-app/remix-add-card-view
Merge into: lp:music-app/remix
Prerequisite: lp:~ahayzen/music-app/remix-remove-blurred-background
Diff against target: 541 lines (+378/-123)
4 files modified
MusicAlbums.qml (+21/-123)
common/Card.qml (+157/-0)
common/CardView.qml (+41/-0)
common/ColumnFlow.qml (+159/-0)
To merge this branch: bzr merge lp:~ahayzen/music-app/remix-add-card-view
Reviewer Review Type Date Requested Status
Victor Thompson Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Andrew Hayzen Abstain
Review via email: mp+236257@code.launchpad.net

Commit message

* Add CardView and Card
* Implement in Albums tab

Description of the change

* Add CardView and Card
* Implement in Albums tab

A few rough edges and hard coded things but wanted an initial review.

To post a comment you must log in.
lp:~ahayzen/music-app/remix-add-card-view updated
642. By Andrew Hayzen

* Cleanup old code

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

I ran autopilot on a utopic VM against #642 and it passed.

$ autopilot3 run music_app
Loading tests from: /home/andy/Workspace/sdk/remix-add-card-view/tests/autopilot

Tests running...
<class 'Xlib.protocol.request.QueryExtension'>

Ran 18 tests in 289.682s
OK

lp:~ahayzen/music-app/remix-add-card-view updated
643. By Andrew Hayzen

* Remove unused imports

644. By Andrew Hayzen

* Further tidying

645. By Andrew Hayzen

* Tweak overlay opacity

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

Per the design spec, please 1) add more spacing before the album name and after the artist name, 2) verify that design agrees with the font sizes (look a little big), and 3) we need to verify if the card view is to be used for an Albums view.

review: Needs Fixing
lp:~ahayzen/music-app/remix-add-card-view updated
646. By Andrew Hayzen

* Change spacing for labels
* Change font sizes

647. By Andrew Hayzen

* Merge of /remix

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

OK so I have fixed the label issues, verified the font size with design and also discussed the pages that will use CardView.

<ahayzen> jounih, for the Albums tab are we expecting to use the cardview?
<jounih> ahayzen: I think we could use the cardview for everything for now. Maybe some of the views would work better as a listview but we can start off with cards for everything
<jounih> what do you think?
<ahayzen> jounih, my understanding was the 'start' page would be cardview... the albums would be cards.... the artists and songs tabs make sense to be a list i think...and the playlists is up for debate
<jounih> ahayzen: playlists definitely cards - albums and playlists are very similar. OK to use listview for artists and songs - i’ll need to provide a design for that, i’ll do it now
<ahayzen> jounih, yep agreed thanks :)

After discussions with nik he recommended to use the Flow {} component to do the staggered grid, so I am going to investigate that and therefore blocking myself for this mp until a decision is made there.

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

PASSED: Continuous integration, rev:647
http://91.189.93.70:8080/job/music-app-remix-ci/4/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/616
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/616/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/4

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

review: Approve (continuous-integration)
lp:~ahayzen/music-app/remix-add-card-view updated
648. By Andrew Hayzen

* Move to using Flow {}

649. By Andrew Hayzen

* Move to ColumnFlow {}

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

PASSED: Continuous integration, rev:649
http://91.189.93.70:8080/job/music-app-remix-ci/28/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/702
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/702/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/28

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

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

Unblocking myself as we have now moved to ColumnFlow {}, please rereview.

review: Abstain
lp:~ahayzen/music-app/remix-add-card-view updated
650. By Andrew Hayzen

* Merge of trunk

651. By Andrew Hayzen

* Add upstream location

652. By Andrew Hayzen

* Merge of trunk

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

PASSED: Continuous integration, rev:650
http://91.189.93.70:8080/job/music-app-remix-ci/30/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/708
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/708/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/30

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

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

PASSED: Continuous integration, rev:652
http://91.189.93.70:8080/job/music-app-remix-ci/31/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/710
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/710/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/31

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

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

See 1 inline comment. Also, I had tried to implement search with this feature and It seems that sometimes the first column would not start at the very top when it reEval's the layout (http://i.imgur.com/f79dNCL.png). I don't think this is critical to fix, but it might be worth looking into later.

review: Needs Information
lp:~ahayzen/music-app/remix-add-card-view updated
653. By Andrew Hayzen

* Fix so that when items are removed from the model there are no gaps

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

PASSED: Continuous integration, rev:653
http://91.189.93.70:8080/job/music-app-remix-ci/42/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/732
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/732/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/42

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

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

LGTM! This is a very welcomed improvement!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'MusicAlbums.qml'
--- MusicAlbums.qml 2014-09-20 15:41:33 +0000
+++ MusicAlbums.qml 2014-10-06 19:09:48 +0000
@@ -19,37 +19,17 @@
1919
20import QtQuick 2.320import QtQuick 2.3
21import Ubuntu.Components 1.121import Ubuntu.Components 1.1
22import Ubuntu.Components.Popups 1.0
23import Ubuntu.MediaScanner 0.122import Ubuntu.MediaScanner 0.1
24import Ubuntu.Thumbnailer 0.1
25import QtMultimedia 5.0
26import QtQuick.LocalStorage 2.0
27import QtGraphicalEffects 1.0
28import "settings.js" as Settings
29import "playlists.js" as Playlists
30import "common"23import "common"
3124
25
32MusicPage {26MusicPage {
33 id: mainpage27 id: albumsPage
34 objectName: "albumsPage"28 objectName: "albumsPage"
35 title: i18n.tr("Albums")29 title: i18n.tr("Albums")
3630
37 // TODO: This ListView is empty and causes the header to get painted with the desired background color because the31 CardView {
38 // page is now vertically flickable.32 id: albumCardView
39 ListView {
40 anchors.fill: parent
41 anchors.bottomMargin: musicToolbar.mouseAreaOffset + musicToolbar.minimizedHeight
42 }
43
44 GridView {
45 id: albumlist
46 anchors.fill: parent
47 anchors.leftMargin: units.gu(1)
48 anchors.top: parent.top
49 anchors.topMargin: mainView.header.height + units.gu(1)
50 anchors.bottomMargin: units.gu(1)
51 cellHeight: height/3
52 cellWidth: height/3
53 model: SortFilterModel {33 model: SortFilterModel {
54 id: albumsModelFilter34 id: albumsModelFilter
55 property alias rowCount: albumsModel.rowCount35 property alias rowCount: albumsModel.rowCount
@@ -60,106 +40,24 @@
60 sort.property: "title"40 sort.property: "title"
61 sort.order: Qt.AscendingOrder41 sort.order: Qt.AscendingOrder
62 }42 }
6343 delegate: Card {
64 delegate: albumDelegate44 id: albumCard
65 flow: GridView.TopToBottom45 imageSource: model.art
6646 objectName: "albumsPageGridItem" + index
67 Component {47 primaryText: model.title
68 id: albumDelegate48 secondaryText: model.artist
69 Item {49
70 property string artist: model.artist50 onClicked: {
71 property string album: model.title51 songsPage.album = model.title;
72 property var covers: [{art: model.art}]52 songsPage.covers = [{art: model.art}]
7353 songsPage.genre = undefined
74 id: albumItem54 songsPage.isAlbum = true
75 height: albumlist.cellHeight - units.gu(1)55 songsPage.line1 = model.artist
76 objectName: "albumsPageGridItem" + index56 songsPage.line2 = model.title
77 width: albumlist.cellHeight - units.gu(1)57 songsPage.title = i18n.tr("Album")
78 anchors.margins: units.gu(1)58
7959 mainPageStack.push(songsPage)
80 CoverRow {
81 id: albumShape
82 anchors {
83 top: parent.top
84 left: parent.left
85 verticalCenter: parent.verticalCenter
86 }
87 count: albumItem.covers.length
88 size: albumItem.width
89 covers: albumItem.covers
90 spacing: units.gu(2)
91 }
92 Item { // Background so can see text in current state
93 id: albumBg
94 anchors {
95 bottom: parent.bottom
96 left: parent.left
97 right: parent.right
98 }
99 height: units.gu(6)
100 clip: true
101 UbuntuShape{
102 anchors {
103 bottom: parent.bottom
104 left: parent.left
105 right: parent.right
106 }
107 height: albumShape.height
108 radius: "medium"
109 color: styleMusic.common.black
110 opacity: 0.6
111 }
112 }
113 Label {
114 id: albumArtist
115 objectName: "albums-albumartist"
116 anchors.bottom: parent.bottom
117 anchors.bottomMargin: units.gu(1)
118 anchors.left: parent.left
119 anchors.leftMargin: units.gu(1)
120 anchors.right: parent.right
121 anchors.rightMargin: units.gu(1)
122 color: styleMusic.common.white
123 elide: Text.ElideRight
124 text: model.artist
125 fontSize: "x-small"
126 }
127 Label {
128 id: albumLabel
129 anchors.bottom: parent.bottom
130 anchors.bottomMargin: units.gu(3)
131 anchors.left: parent.left
132 anchors.leftMargin: units.gu(1)
133 anchors.right: parent.right
134 anchors.rightMargin: units.gu(1)
135 color: styleMusic.common.white
136 elide: Text.ElideRight
137 text: model.title
138 fontSize: "small"
139 font.weight: Font.DemiBold
140 }
141
142
143 MouseArea {
144 anchors.fill: parent
145 onClicked: {
146 songsPage.album = model.title;
147 songsPage.covers = [{art: model.art}]
148 songsPage.genre = undefined
149 songsPage.isAlbum = true
150 songsPage.line1 = model.artist
151 songsPage.line2 = model.title
152 songsPage.title = i18n.tr("Album")
153
154 mainPageStack.push(songsPage)
155 }
156
157 // TODO: If http://pad.lv/1354753 is fixed to expose whether the Shape should appear pressed, update this as well.
158 onPressedChanged: albumShape.pressed = pressed
159 }
160 }60 }
161 }61 }
162 }62 }
163}63}
164
165
16664
=== added file 'common/Card.qml'
--- common/Card.qml 1970-01-01 00:00:00 +0000
+++ common/Card.qml 2014-10-06 19:09:48 +0000
@@ -0,0 +1,157 @@
1/*
2 * Copyright (C) 2014
3 * Andrew Hayzen <ahayzen@gmail.com>
4 *
5 * This program is free software; you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License as published by
7 * the Free Software Foundation; version 3.
8 *
9 * This program is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 * GNU General Public License for more details.
13 *
14 * You should have received a copy of the GNU General Public License
15 * along with this program. If not, see <http://www.gnu.org/licenses/>.
16 */
17
18import QtQuick 2.3
19import Ubuntu.Components 1.1
20
21
22Rectangle {
23 id: card
24 color: "transparent"
25 height: cardColumn.childrenRect.height + 2 * bg.anchors.margins
26
27 property alias imageSource: image.source
28 property alias primaryText: primaryLabel.text
29 property alias secondaryText: secondaryLabel.text
30
31 signal clicked(var mouse)
32 signal pressAndHold(var mouse)
33
34 /* Animations */
35 Behavior on height {
36 UbuntuNumberAnimation {
37
38 }
39 }
40
41 Behavior on width {
42 UbuntuNumberAnimation {
43
44 }
45 }
46
47 Behavior on x {
48 UbuntuNumberAnimation {
49
50 }
51 }
52
53 Behavior on y {
54 UbuntuNumberAnimation {
55
56 }
57 }
58
59 /* Background for card */
60 Rectangle {
61 id: bg
62 anchors {
63 fill: parent
64 margins: units.gu(1)
65 }
66 color: "#2c2c34"
67 }
68
69 /* Column containing image and labels */
70 Column {
71 id: cardColumn
72 anchors {
73 fill: bg
74 }
75 spacing: units.gu(0.5)
76
77 Image {
78 id: image
79 height: parent.width
80 width: parent.width
81
82 onStatusChanged: {
83 if (status === Image.Error) {
84 source = Qt.resolvedUrl("../images/music-app-cover@30.png")
85 }
86 }
87 }
88
89 Rectangle {
90 color: "transparent"
91 height: units.gu(1)
92 width: units.gu(1)
93 }
94
95 Label {
96 id: primaryLabel
97 anchors {
98 left: parent.left
99 leftMargin: units.gu(1)
100 right: parent.right
101 rightMargin: units.gu(1)
102 }
103 color: "#FFF"
104 elide: Text.ElideRight
105 fontSize: "small"
106 opacity: 1.0
107 wrapMode: Text.WordWrap
108 }
109
110 Label {
111 id: secondaryLabel
112 anchors {
113 left: parent.left
114 leftMargin: units.gu(1)
115 right: parent.right
116 rightMargin: units.gu(1)
117 }
118 color: "#FFF"
119 elide: Text.ElideRight
120 fontSize: "small"
121 opacity: 0.4
122 wrapMode: Text.WordWrap
123 }
124
125 Rectangle {
126 color: "transparent"
127 height: units.gu(1.5)
128 width: units.gu(1)
129 }
130 }
131
132 /* Overlay for when card is pressed */
133 Rectangle {
134 id: overlay
135 anchors {
136 fill: bg
137 }
138 color: "#000"
139 opacity: 0
140
141 Behavior on opacity {
142 UbuntuNumberAnimation {
143
144 }
145 }
146 }
147
148 /* Capture mouse events */
149 MouseArea {
150 anchors {
151 fill: parent
152 }
153 onClicked: card.clicked(mouse)
154 onPressAndHold: card.pressAndHold(mouse)
155 onPressedChanged: overlay.opacity = pressed ? 0.3 : 0
156 }
157}
0158
=== added file 'common/CardView.qml'
--- common/CardView.qml 1970-01-01 00:00:00 +0000
+++ common/CardView.qml 2014-10-06 19:09:48 +0000
@@ -0,0 +1,41 @@
1/*
2 * Copyright (C) 2014
3 * Andrew Hayzen <ahayzen@gmail.com>
4 *
5 * This program is free software; you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License as published by
7 * the Free Software Foundation; version 3.
8 *
9 * This program is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 * GNU General Public License for more details.
13 *
14 * You should have received a copy of the GNU General Public License
15 * along with this program. If not, see <http://www.gnu.org/licenses/>.
16 */
17
18import QtQuick 2.3
19import Ubuntu.Components 1.1
20
21
22Flickable {
23 anchors {
24 fill: parent
25 margins: units.gu(1)
26 }
27
28 // dont use flow.contentHeight as it is inaccurate due to height of labels
29 // changing as they load
30 contentHeight: flow.childrenRect.height
31 contentWidth: width
32
33 property alias delegate: flow.delegate
34 property alias model: flow.model
35
36 ColumnFlow {
37 id: flow
38 columns: parseInt(width / units.gu(15))
39 width: parent.width
40 }
41}
042
=== added file 'common/ColumnFlow.qml'
--- common/ColumnFlow.qml 1970-01-01 00:00:00 +0000
+++ common/ColumnFlow.qml 2014-10-06 19:09:48 +0000
@@ -0,0 +1,159 @@
1/*
2 * Copyright (C) 2014
3 * Andrew Hayzen <ahayzen@gmail.com>
4 * Michael Spencer <sonrisesoftware@gmail.com>
5 *
6 * This program is free software; you can redistribute it and/or modify
7 * it under the terms of the GNU General Public License as published by
8 * the Free Software Foundation; version 3.
9 *
10 * This program is distributed in the hope that it will be useful,
11 * but WITHOUT ANY WARRANTY; without even the implied warranty of
12 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13 * GNU General Public License for more details.
14 *
15 * You should have received a copy of the GNU General Public License
16 * along with this program. If not, see <http://www.gnu.org/licenses/>.
17 *
18 * Upstream location:
19 * https://github.com/iBeliever/ubuntu-ui-extras/blob/master/ColumnFlow.qml
20 */
21
22import QtQuick 2.3
23
24
25Item {
26 id: columnFlow
27 property int columns: 1
28 property bool repeaterCompleted: false
29 property alias model: repeater.model
30 property alias delegate: repeater.delegate
31 property int contentHeight: 0
32
33 onColumnsChanged: reEvalColumns()
34 onModelChanged: reEvalColumns()
35 onWidthChanged: updateWidths()
36
37 function updateWidths() {
38 if (repeaterCompleted) {
39 var count = 0
40
41 //add the first <column> elements
42 for (var i = 0; count < columns && i < columnFlow.children.length; i++) {
43 //print(i, count)
44 if (!columnFlow.children[i] || String(columnFlow.children[i]).indexOf("QQuickRepeater") == 0)
45 //|| !columnFlow.children[i].visible) // CUSTOM - view is invisible at start
46 continue
47
48 columnFlow.children[i].width = width / columns
49
50 count++
51 }
52 }
53 }
54
55 function reEvalColumns() {
56 if (columnFlow.repeaterCompleted === false)
57 return
58
59 if (columns === 0) {
60 contentHeight = 0
61 return
62 }
63
64 var i, j
65 var columnHeights = new Array(columns);
66 var lastItem = new Array(columns)
67 var lastI = -1
68 var count = 0
69
70 //add the first <column> elements
71 for (i = 0; count < columns && i < columnFlow.children.length; i++) {
72 // CUSTOM - ignore if has just been removed
73 if (i === repeater.removeHintIndex && columnFlow.children[i] === repeater.removeHintItem) {
74 continue
75 }
76
77 if (!columnFlow.children[i] || String(columnFlow.children[i]).indexOf("QQuickRepeater") == 0)
78 //|| !columnFlow.children[i].visible) // CUSTOM - view is invisible at start
79 continue
80
81 lastItem[count] = i
82
83 columnHeights[count] = columnFlow.children[i].height
84 columnFlow.children[i].anchors.top = columnFlow.top
85 columnFlow.children[i].anchors.left = (lastI === -1 ? columnFlow.left : columnFlow.children[lastI].right)
86 columnFlow.children[i].anchors.right = undefined
87 columnFlow.children[i].width = columnFlow.width / columns
88
89 lastI = i
90 count++
91 }
92
93 //add the other elements
94 for (i = i; i < columnFlow.children.length; i++) {
95 var highestHeight = Number.MAX_VALUE
96 var newColumn = 0
97
98 // CUSTOM - ignore if has just been removed
99 if (i === repeater.removeHintIndex && columnFlow.children[i] === repeater.removeHintItem) {
100 continue
101 }
102
103 if (!columnFlow.children[i] || String(columnFlow.children[i]).indexOf("QQuickRepeater") == 0)
104 //|| !columnFlow.children[i].visible) // CUSTOM - view is invisible at start
105 continue
106
107 // find the shortest column
108 for (j = 0; j < columns; j++) {
109 if (columnHeights[j] !== null && columnHeights[j] < highestHeight) {
110 newColumn = j
111 highestHeight = columnHeights[j]
112 }
113 }
114
115 // add the element to the shortest column
116 columnFlow.children[i].anchors.top = columnFlow.children[lastItem[newColumn]].bottom
117 columnFlow.children[i].anchors.left = columnFlow.children[lastItem[newColumn]].left
118 columnFlow.children[i].anchors.right = columnFlow.children[lastItem[newColumn]].right
119
120 lastItem[newColumn] = i
121 columnHeights[newColumn] += columnFlow.children[i].height
122 }
123
124 var cHeight = 0
125
126 for (i = 0; i < columnHeights.length; i++) {
127 if (columnHeights[i])
128 cHeight = Math.max(cHeight, columnHeights[i])
129 }
130
131 contentHeight = cHeight
132 updateWidths()
133 }
134
135 Repeater {
136 id: repeater
137 model: columnFlow.model
138 Component.onCompleted: {
139 columnFlow.repeaterCompleted = true
140 columnFlow.reEvalColumns()
141 }
142
143 // Provide a hint of the removed item
144 property int removeHintIndex: -1 // CUSTOM
145 property var removeHintItem // CUSTOM
146
147 onItemAdded: columnFlow.reEvalColumns() // CUSTOM - ms2 models are live
148 onItemRemoved: {
149 removeHintIndex = index
150 removeHintItem = item
151
152 columnFlow.reEvalColumns() // CUSTOM - ms2 models are live
153
154 // Set back to null to allow freeing of memory
155 removeHintIndex = -1
156 removeHintItem = undefined
157 }
158 }
159}

Subscribers

People subscribed via source and target branches