936397 - [music2] [2] Implement the current views with tablet style

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia::Music, defect)

Description

[Dominic Kuo [:dkuo]]

After the first item(Bug 936385) of music2 is landed, we can start to implement the features that the current music has, if the schedule is allowed, probably we can implement the features with phone style as well.

This item should cover:
2-1. Mix.
2-2. Playlists.
2-3. Albums, Artists and All Songs.

Comment 1

[Dominic Kuo [:dkuo]]

The first part of bug 936385 is landed, this bug is not blocked so taking it and start to implement it.

Comment 2

[Francis Lee [:frlee]]

this is subset of Music2 re-layout for tablet, tag it 1.3+ with target milestones.

Comment 3

[Dominic Kuo [:dkuo]]

Attachment: Part 1 for 2-2 and 2-3

Jim,

This is the first patch for this bug, I have implemented the playlists, albums, artists and songs views but mix view because, the UX design of mix view is still under discussing. And basically I followed Evan's code structure and style so it should be not too hard to understand.
And there are two new js that I imported from shared/js, which are screen_layout.js and template.js that you probably not familiar with. Simply say, screen_layout.js is for judging the screen size, and template.js is for easier to render the DOM elements. Would you please review this one? thanks.

Evan,

I am not sure if you are busy for school so I set feedback to you but review so that I can get Jim's review and your feedback as well, and feel free to give us any ideas if you like, thanks.

Comment 4

[Jim Porter (:squib)]

Comment on attachment 8336242 [details] [review]
Part 1 for 2-2 and 2-3

I believe this was an issue prior to the patch, but on desktop, I'm seeing an issue where the mix page isn't deactivated when you click on another page, and so you get both the mix view and, say, the playlists view at once. If it's not too hard to fix, I think we should try to fix that here, since it'll make things work a lot better.

Otherwise, the code seems good, but I'm going to do another pass before I finish my review. I just wanted to note this issue now before I forget.

Comment 5

[Dominic Kuo [:dkuo]]

(In reply to Jim Porter (:squib) from comment #4)
> Comment on attachment 8336242 [details] [review]
> Part 1 for 2-2 and 2-3
> 
> I believe this was an issue prior to the patch, but on desktop, I'm seeing
> an issue where the mix page isn't deactivated when you click on another
> page, and so you get both the mix view and, say, the playlists view at once.
> If it's not too hard to fix, I think we should try to fix that here, since
> it'll make things work a lot better.

No problem, I will see where is the issue and you can keep on reviewing, thanks.

Comment 6

[Dominic Kuo [:dkuo]]

Jim, I have addressed the issue you mentioned in comment 4, that was an easy fix, it seems like a gecko issue but I am not sure. That is, if music2 is launched by its url with "#select-mix", then we select the other pages like you said, both of the :target css role will be applied to mix view and the view we select, and that's why the mix page was still activated. So I just added window.location.hash = '#select-mix' to tirgger the hashchange, it should be easy for other people to understand.

Comment 7

[Jim Porter (:squib)]

Comment on attachment 8336242 [details] [review]
Part 1 for 2-2 and 2-3

I've looked over this more and I think this is good to land. There might be things we want to clean up later, but this is a huge step towards making music2 usable, and I think we should land it as soon as possible so more people can try it out.

Comment 8

[eshapiro]

I haven't had a chance to run it, but I just looked through the code, lgtm! I agree about landing it sooner rather than later - having somewhat of a foundation in place seems like a good idea for moving forward.

Comment 9

[Dominic Kuo [:dkuo]]

Thanks guys! the travis finally gets green.

2-2 and 2-3 landed on master: bb5b11b420901fb2a6f0ffab5e6fb128e1d6160e

Comment 10

[Jim Porter (:squib)]

Closing all music2-specific bugs. The current plan is to gradually incorporate bits of music2 into the current music app, since it will take too long to achieve feature parity in music2.