Merge lp:~vthompson/music-app/music-uc1.3-now-playing-fix-1447428 into lp:music-app

Proposed by Victor Thompson
Status: Merged
Approved by: Victor Thompson
Approved revision: 915
Merged at revision: 939
Proposed branch: lp:~vthompson/music-app/music-uc1.3-now-playing-fix-1447428
Merge into: lp:music-app
Prerequisite: lp:~vthompson/music-app/music-uc1.3
Diff against target: 427 lines (+87/-70)
5 files modified
app/music-app.qml (+3/-9)
app/ui/NowPlaying.qml (+25/-10)
debian/changelog (+1/-0)
po/com.ubuntu.music.pot (+51/-47)
tests/autopilot/music_app/__init__.py (+7/-4)
To merge this branch: bzr merge lp:~vthompson/music-app/music-uc1.3-now-playing-fix-1447428
Reviewer Review Type Date Requested Status
Jenkins Bot continuous-integration Approve
Victor Thompson Approve
Andrew Hayzen Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Review via email: mp+275635@code.launchpad.net

Commit message

* Add initial page sections to Now Playing view

Description of the change

* Add initial page sections to Now Playing view

This adds a Now Playing toggle to the Now Playing view, using the newer Page Head Sections API. This has been approved by Design.

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

PASSED: Continuous integration, rev:908
http://91.189.93.70:8080/job/music-app-ci/1399/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/251

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

review: Approve (continuous-integration)
909. By Victor Thompson

Merge trunk and resolve conflicts.

910. By Victor Thompson

Add FIXME.

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

PASSED: Continuous integration, rev:910
http://91.189.93.70:8080/job/music-app-ci/1411/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/263

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

review: Approve (continuous-integration)
911. By Victor Thompson

Merge trunk and resolve conflicts.

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

PASSED: Continuous integration, rev:911
http://91.189.93.70:8080/job/music-app-ci/1414/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/266

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

review: Approve (continuous-integration)
912. By Victor Thompson

Fix AP.

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

Tests running...

Ran 19 tests in 259.503s
OK

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

PASSED: Continuous integration, rev:912
http://91.189.93.70:8080/job/music-app-ci/1415/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/267

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

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

One inline comment

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

Also would be really really nice if we could centre align the head sections, wonder if that is something we can request?

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

When you switch from the Queue -> Full View I think it should set the state back to normal, so that it disables any multiselects that have happened.

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

As for centering the sections, we should talk to the SDK and Design teams to see how they envision apps making this visible. I assumed having them left justified was preferred, but for something like this (a view toggle) centering the section items seems like a decent idea.

913. By Victor Thompson

Remove onVisibleChanged and add code to close multiselection.

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

I'm not sure why I added the onVisibleChanged handler. I've removed it to fix the playlist add issue. I've also made it so multiselect will be close when toggling.

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

PASSED: Continuous integration, rev:913
http://91.189.93.70:8080/job/music-app-ci/1416/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/268

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

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

#blocked on lp:1511839 landing in ota8

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

One inline comment, if pushNowPlaying() is called, it needs to be ensured that the Full View is selected as this could be called via uri-handler but in the queue view.

review: Needs Fixing
914. By Victor Thompson

Resolve comment.

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

LGTM, nice to make use of the new uc1.3 components and get rid of the confusing/ambiguous header action :-)

review: Approve
915. By Victor Thompson

Merge trunk and resolve conflict

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

Tests running...
/usr/lib/python3/dist-packages/gi/overrides/Gtk.py:1554: Warning: g_object_ref: assertion 'G_IS_OBJECT (object)' failed
  initialized, argv = Gtk.init_check(sys.argv)

Ran 19 tests in 242.214s
OK

review: Approve
Revision history for this message
Jenkins Bot (ubuntu-core-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 'app/music-app.qml'
2--- app/music-app.qml 2015-10-28 01:05:33 +0000
3+++ app/music-app.qml 2015-11-16 02:01:16 +0000
4@@ -369,7 +369,7 @@
5
6 if (!clear) {
7 // If same track and on Now playing page then toggle
8- if ((mainPageStack.currentPage.title === i18n.tr("Now playing") || mainPageStack.currentPage.title === i18n.tr("Queue"))
9+ if (mainPageStack.currentPage.title === i18n.tr("Now playing")
10 && trackQueue.model.get(player.currentIndex) !== undefined
11 && Qt.resolvedUrl(trackQueue.model.get(player.currentIndex).filename) === file) {
12 player.toggle()
13@@ -399,11 +399,6 @@
14 else {
15 player.playSong(trackQueue.model.get(index).filename, index);
16 }
17-
18- // Show the Now playing page and make sure the track is visible
19- if (mainPageStack.currentPage.title !== i18n.tr("Queue")) {
20- tabs.pushNowPlaying();
21- }
22 }
23
24 function playRandomSong(shuffle)
25@@ -984,12 +979,11 @@
26 function pushNowPlaying()
27 {
28 // only push if on a different page
29- if (mainPageStack.currentPage.title !== i18n.tr("Now playing")
30- && mainPageStack.currentPage.title !== i18n.tr("Queue")) {
31+ if (mainPageStack.currentPage.title !== i18n.tr("Now playing")) {
32 mainPageStack.push(Qt.resolvedUrl("ui/NowPlaying.qml"), {})
33 }
34
35- if (mainPageStack.currentPage.title === i18n.tr("Queue")) {
36+ if (mainPageStack.currentPage.isListView === true) {
37 mainPageStack.currentPage.isListView = false; // ensure full view
38 }
39 }
40
41=== modified file 'app/ui/NowPlaying.qml'
42--- app/ui/NowPlaying.qml 2015-10-28 01:05:33 +0000
43+++ app/ui/NowPlaying.qml 2015-11-16 02:01:16 +0000
44@@ -30,13 +30,15 @@
45 flickable: isListView ? queueListLoader.item : null // Ensures that the header is shown in fullview
46 objectName: "nowPlayingPage"
47 showToolbar: false
48- title: isListView ? queueTitle : nowPlayingTitle
49+ title: nowPlayingTitle
50 visible: false
51
52 property bool isListView: false
53 // TRANSLATORS: this appears in the header with limited space (around 20 characters)
54 property string nowPlayingTitle: i18n.tr("Now playing")
55 // TRANSLATORS: this appears in the header with limited space (around 20 characters)
56+ property string fullViewTitle: i18n.tr("Full view")
57+ // TRANSLATORS: this appears in the header with limited space (around 20 characters)
58 property string queueTitle: i18n.tr("Queue")
59
60 onIsListViewChanged: {
61@@ -51,6 +53,9 @@
62 }
63 })
64 }
65+ } else {
66+ // Close multiselection mode.
67+ queueListLoader.item.closeSelection()
68 }
69 }
70
71@@ -72,6 +77,20 @@
72 queueListLoader.item.positionViewAtIndex(index, ListView.Center);
73 }
74
75+ PageHeadSections {
76+ id: defaultStateSections
77+ model: [fullViewTitle, queueTitle]
78+ selectedIndex: isListView
79+ }
80+
81+ head {
82+ sections {
83+ model: defaultStateSections.model
84+ selectedIndex: defaultStateSections.selectedIndex
85+ onSelectedIndexChanged: isListView = !isListView
86+ }
87+ }
88+
89 state: isListView && queueListLoader.item.state === "multiselectable" ? "selection" : "default"
90 states: [
91 PageHeadState {
92@@ -80,13 +99,6 @@
93 name: "default"
94 actions: [
95 Action {
96- objectName: "toggleView"
97- iconName: isListView ? "stock_image" : "view-list-symbolic"
98- onTriggered: {
99- isListView = !isListView
100- }
101- },
102- Action {
103 enabled: trackQueue.model.count > 0
104 iconName: "add-to-playlist"
105 // TRANSLATORS: this action appears in the overflow drawer with limited space (around 18 characters)
106@@ -138,9 +150,12 @@
107 left: parent.left
108 right: parent.right
109 top: parent.top
110- topMargin: units.gu(6.125) // FIXME: 6.125 is the header.height
111+ topMargin: headerHeight
112 }
113- height: parent.height - units.gu(6.125) - units.gu(9.5) // FIXME: 6.125 is the header.height
114+
115+ property real headerHeight: units.gu(10.125) // FIXME: 10.125 is the header.height with the page sections
116+
117+ height: parent.height - headerHeight - units.gu(9.5)
118 source: "../components/NowPlayingFullView.qml"
119 visible: !isListView
120 }
121
122=== modified file 'debian/changelog'
123--- debian/changelog 2015-11-03 02:44:19 +0000
124+++ debian/changelog 2015-11-16 02:01:16 +0000
125@@ -9,6 +9,7 @@
126 * Update canned mediastore.db schema version from 8 to 9 (LP: #1511585).
127 * Update canned mediastore.db schema version from 9 to 10.
128 * Add directions for updating and debugging schema version changes.
129+ * Add initial page sections to Now Playing view (LP: #1447428)
130
131 [ Andrew Hayzen ]
132 * Switch to using the new uc1.3 listitems within the SDK
133
134=== modified file 'po/com.ubuntu.music.pot'
135--- po/com.ubuntu.music.pot 2015-08-21 19:41:03 +0000
136+++ po/com.ubuntu.music.pot 2015-11-16 02:01:16 +0000
137@@ -8,7 +8,7 @@
138 msgstr ""
139 "Project-Id-Version: music-app\n"
140 "Report-Msgid-Bugs-To: \n"
141-"POT-Creation-Date: 2015-08-21 20:40+0100\n"
142+"POT-Creation-Date: 2015-10-24 14:06-0500\n"
143 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
144 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
145 "Language-Team: LANGUAGE <LL@li.org>\n"
146@@ -37,7 +37,7 @@
147 msgid "Cancel"
148 msgstr ""
149
150-#: ../app/components/Dialog/ContentHubWaitDialog.qml:34
151+#: ../app/components/Dialog/ContentHubWaitDialog.qml:33
152 msgid "Waiting for file(s)..."
153 msgstr ""
154
155@@ -83,7 +83,7 @@
156 msgstr ""
157
158 #: ../app/components/Dialog/RemovePlaylistDialog.qml:37
159-#: ../app/components/ListItemActions/Remove.qml:28
160+#: ../app/components/ListItemActions/Remove.qml:27
161 msgid "Remove"
162 msgstr ""
163
164@@ -94,7 +94,7 @@
165 #. TRANSLATORS: this action appears in the overflow drawer with limited space (around 18 characters)
166 #: ../app/components/HeadState/MultiSelectHeadState.qml:39
167 #: ../app/components/ListItemActions/AddToPlaylist.qml:26
168-#: ../app/ui/NowPlaying.qml:93
169+#: ../app/ui/NowPlaying.qml:108
170 msgid "Add to playlist"
171 msgstr ""
172
173@@ -180,11 +180,11 @@
174 msgid "Shuffle"
175 msgstr ""
176
177-#: ../app/components/Walkthrough/Slide1.qml:58
178+#: ../app/components/Walkthrough/Slide1.qml:59
179 msgid "Welcome to Music"
180 msgstr ""
181
182-#: ../app/components/Walkthrough/Slide1.qml:72
183+#: ../app/components/Walkthrough/Slide1.qml:73
184 msgid ""
185 "Enjoy your favorite music with Ubuntu's Music App. Take a short tour on how "
186 "to get started or press skip to start listening now."
187@@ -195,7 +195,7 @@
188 msgstr ""
189
190 #: ../app/components/Walkthrough/Slide2.qml:67
191-#: ../app/ui/LibraryEmptyState.qml:117
192+#: ../app/ui/LibraryEmptyState.qml:123
193 msgid ""
194 "Connect your device to any computer and simply drag files to the Music "
195 "folder or insert removable media with music."
196@@ -213,84 +213,78 @@
197 msgid "Start"
198 msgstr ""
199
200-#: ../app/components/Walkthrough/Walkthrough.qml:84
201+#: ../app/components/Walkthrough/Walkthrough.qml:120
202 msgid "Skip"
203 msgstr ""
204
205-#: ../app/music-app.qml:155
206+#: ../app/music-app.qml:156
207 msgid "Next"
208 msgstr ""
209
210-#: ../app/music-app.qml:156
211+#: ../app/music-app.qml:157
212 msgid "Next Track"
213 msgstr ""
214
215-#: ../app/music-app.qml:162
216+#: ../app/music-app.qml:163
217 msgid "Pause"
218 msgstr ""
219
220-#: ../app/music-app.qml:162
221+#: ../app/music-app.qml:163
222 msgid "Play"
223 msgstr ""
224
225-#: ../app/music-app.qml:164
226+#: ../app/music-app.qml:165
227 msgid "Pause Playback"
228 msgstr ""
229
230-#: ../app/music-app.qml:164
231+#: ../app/music-app.qml:165
232 msgid "Continue or start playback"
233 msgstr ""
234
235-#: ../app/music-app.qml:169
236+#: ../app/music-app.qml:170
237 msgid "Back"
238 msgstr ""
239
240-#: ../app/music-app.qml:170
241+#: ../app/music-app.qml:171
242 msgid "Go back to last page"
243 msgstr ""
244
245-#: ../app/music-app.qml:178
246+#: ../app/music-app.qml:179
247 msgid "Previous"
248 msgstr ""
249
250-#: ../app/music-app.qml:179
251+#: ../app/music-app.qml:180
252 msgid "Previous Track"
253 msgstr ""
254
255-#: ../app/music-app.qml:184
256+#: ../app/music-app.qml:185
257 msgid "Stop"
258 msgstr ""
259
260-#: ../app/music-app.qml:185
261+#: ../app/music-app.qml:186
262 msgid "Stop Playback"
263 msgstr ""
264
265-#: ../app/music-app.qml:276 com.ubuntu.music_music.desktop.in.in.h:1
266+#: ../app/music-app.qml:277 com.ubuntu.music_music.desktop.in.in.h:1
267 msgid "Music"
268 msgstr ""
269
270-#: ../app/music-app.qml:306
271+#: ../app/music-app.qml:307
272 msgid "Debug: "
273 msgstr ""
274
275 #. TRANSLATORS: this appears in the header with limited space (around 20 characters)
276-#: ../app/music-app.qml:371 ../app/music-app.qml:986
277+#: ../app/music-app.qml:372 ../app/music-app.qml:982
278 #: ../app/ui/NowPlaying.qml:38
279 msgid "Now playing"
280 msgstr ""
281
282 #. TRANSLATORS: this appears in the header with limited space (around 20 characters)
283-#: ../app/music-app.qml:371 ../app/music-app.qml:403 ../app/music-app.qml:987
284-#: ../app/music-app.qml:991 ../app/ui/NowPlaying.qml:40
285-msgid "Queue"
286-msgstr ""
287-
288-#. TRANSLATORS: this appears in the header with limited space (around 20 characters)
289-#: ../app/ui/AddToPlaylist.qml:46
290+#: ../app/ui/AddToPlaylist.qml:45
291 msgid "Select playlist"
292 msgstr ""
293
294-#: ../app/ui/AddToPlaylist.qml:94 ../app/ui/Playlists.qml:87
295+#: ../app/ui/AddToPlaylist.qml:93 ../app/ui/Playlists.qml:86
296 #: ../app/ui/SongsView.qml:292 ../app/ui/SongsView.qml:293
297 #, qt-format
298 msgid "%1 song"
299@@ -300,12 +294,12 @@
300
301 #. TRANSLATORS: this is the name of the playlists page shown in the tab header.
302 #. Remember to keep the translation short to fit the screen width
303-#: ../app/ui/AddToPlaylist.qml:99 ../app/ui/Playlists.qml:38
304+#: ../app/ui/AddToPlaylist.qml:98 ../app/ui/Playlists.qml:37
305 #: ../app/ui/SongsView.qml:103
306 msgid "Playlists"
307 msgstr ""
308
309-#: ../app/ui/AddToPlaylist.qml:109 ../app/ui/Recent.qml:37
310+#: ../app/ui/AddToPlaylist.qml:108 ../app/ui/Recent.qml:36
311 #: ../app/ui/SongsView.qml:91
312 msgid "Recent"
313 msgstr ""
314@@ -314,35 +308,35 @@
315 msgid "Albums"
316 msgstr ""
317
318-#: ../app/ui/Albums.qml:72 ../app/ui/ArtistView.qml:129
319-#: ../app/ui/ArtistView.qml:142 ../app/ui/Recent.qml:82
320+#: ../app/ui/Albums.qml:72 ../app/ui/ArtistView.qml:128
321+#: ../app/ui/ArtistView.qml:141 ../app/ui/Recent.qml:81
322 #: ../app/ui/SongsView.qml:249
323 msgid "Unknown Album"
324 msgstr ""
325
326-#: ../app/ui/Albums.qml:73 ../app/ui/ArtistView.qml:85
327-#: ../app/ui/ArtistView.qml:141 ../app/ui/Artists.qml:80
328-#: ../app/ui/Recent.qml:83 ../app/ui/SongsView.qml:270
329+#: ../app/ui/Albums.qml:73 ../app/ui/ArtistView.qml:84
330+#: ../app/ui/ArtistView.qml:140 ../app/ui/Artists.qml:79
331+#: ../app/ui/Recent.qml:82 ../app/ui/SongsView.qml:270
332 msgid "Unknown Artist"
333 msgstr ""
334
335-#: ../app/ui/Albums.qml:83 ../app/ui/ArtistView.qml:140
336-#: ../app/ui/Recent.qml:98
337+#: ../app/ui/Albums.qml:83 ../app/ui/ArtistView.qml:139
338+#: ../app/ui/Recent.qml:97
339 msgid "Album"
340 msgstr ""
341
342-#: ../app/ui/ArtistView.qml:104
343+#: ../app/ui/ArtistView.qml:103
344 #, qt-format
345 msgid "%1 album"
346 msgid_plural "%1 albums"
347 msgstr[0] ""
348 msgstr[1] ""
349
350-#: ../app/ui/Artists.qml:38
351+#: ../app/ui/Artists.qml:37
352 msgid "Artists"
353 msgstr ""
354
355-#: ../app/ui/Artists.qml:108
356+#: ../app/ui/Artists.qml:107
357 msgid "Artist"
358 msgstr ""
359
360@@ -361,17 +355,27 @@
361 msgid "Genre"
362 msgstr ""
363
364-#: ../app/ui/LibraryEmptyState.qml:106
365+#: ../app/ui/LibraryEmptyState.qml:112
366 msgid "No music found"
367 msgstr ""
368
369+#. TRANSLATORS: this appears in the header with limited space (around 20 characters)
370+#: ../app/ui/NowPlaying.qml:40
371+msgid "Full view"
372+msgstr ""
373+
374+#. TRANSLATORS: this appears in the header with limited space (around 20 characters)
375+#: ../app/ui/NowPlaying.qml:42
376+msgid "Queue"
377+msgstr ""
378+
379 #. TRANSLATORS: this action appears in the overflow drawer with limited space (around 18 characters)
380-#: ../app/ui/NowPlaying.qml:108
381+#: ../app/ui/NowPlaying.qml:123
382 msgid "Clear queue"
383 msgstr ""
384
385-#: ../app/ui/Playlists.qml:99 ../app/ui/Playlists.qml:100
386-#: ../app/ui/Recent.qml:83 ../app/ui/Recent.qml:98 ../app/ui/SongsView.qml:67
387+#: ../app/ui/Playlists.qml:98 ../app/ui/Playlists.qml:99
388+#: ../app/ui/Recent.qml:82 ../app/ui/Recent.qml:97 ../app/ui/SongsView.qml:67
389 #: ../app/ui/SongsView.qml:82 ../app/ui/SongsView.qml:114
390 #: ../app/ui/SongsView.qml:154 ../app/ui/SongsView.qml:188
391 #: ../app/ui/SongsView.qml:203 ../app/ui/SongsView.qml:222
392
393=== modified file 'tests/autopilot/music_app/__init__.py'
394--- tests/autopilot/music_app/__init__.py 2015-11-02 05:04:08 +0000
395+++ tests/autopilot/music_app/__init__.py 2015-11-16 02:01:16 +0000
396@@ -27,7 +27,7 @@
397 """Wrapper which ensures the now playing is full before clicking"""
398 def func_wrapper(self, *args, **kwargs):
399 if self.isListView:
400- self.click_toggle_view()
401+ self.click_full_view()
402
403 return func(self, *args, **kwargs)
404
405@@ -38,7 +38,7 @@
406 """Wrapper which ensures the now playing is list before clicking"""
407 def func_wrapper(self, *args, **kwargs):
408 if not self.isListView:
409- self.click_toggle_view()
410+ self.click_queue_view()
411
412 return func(self, *args, **kwargs)
413
414@@ -289,8 +289,11 @@
415 def click_shuffle_button(self):
416 return self.wait_select_single("*", objectName="shuffleShape")
417
418- def click_toggle_view(self):
419- self.main_view.get_header().click_action_button("toggleView")
420+ def click_full_view(self):
421+ self.main_view.get_header().switch_to_section_by_index(0)
422+
423+ def click_queue_view(self):
424+ self.main_view.get_header().switch_to_section_by_index(1)
425
426 @ensure_now_playing_list
427 def get_track(self, i):

Subscribers

People subscribed via source and target branches