913903 - [Music] Music app should have a way to go to the first character of an album, song, or artist similar to the contacts

Status: RESOLVED DUPLICATE of bug 1209477

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

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Gaia   3628d9f5829f087782c708981c4863a7d885a96a
Gecko ab5f29823236
BuildID 20130906040204
Version 26.0a1

1. load a lot of music on the sdcard
2. launch music app
3. select songs

Expected: some way of jumping via the first character
Actual: no way to jump; have to scroll through the whole thing

Comment 1

[Dominic Kuo [:dkuo]]

Thanks for catching this, Naoki, I totally forget we didn't implement this for music yet. To fix this will depend on the patch of bug 901421, which I am addressing on, so I will also take this.

Comment 2

[Hema Koka [:hema]]

Renominating for koi

Comment 3

[Hema Koka [:hema]]

Lets consider this for music2 search.

Comment 4

[amitav.anand]

Started working on this feature implementation. I am able to get the UI as we have in contacts. 

Now working on the back end for scrolling using the navigation element. Scrolling for contact is handled in /gaia/apps/communications/contacts/js/views/list.js. Trying to implement the same in Music.

Comment 5

[Dominic Kuo [:dkuo]]

I am not working on this.

Comment 6

[amitav.anand]

Attachment: PR

Just for the record,created a PR. This PR contains only the UI part of this feature.

Comment 8

[Hubert Figuiere [:hub]]

Dominic, what's the status if this? (since you are r?).

It would be nice to have someone finish to implement this.

Comment 9

[Dominic Kuo [:dkuo]]

(In reply to Hubert Figuiere [:hub] from comment #8)
> Dominic, what's the status if this? (since you are r?).
> 
> It would be nice to have someone finish to implement this.

[e-mail] has worked on this but the patch is not complete, he just attached here as a record.

Comment 10

[Hubert Figuiere [:hub]]

So we should pick that up sooner rather than later?

Comment 11

[Dominic Kuo [:dkuo]]

(In reply to Hubert Figuiere [:hub] from comment #10)
> So we should pick that up sooner rather than later?

The implementation of this feature will need the characters/indexes information, which the MediaDB is currently not provided, so we need to create them after the first time we enumerate all the records from the MediaDB, then save them in music app's own database, and it's the complex of this bug.

Hubert, since 2.2 music will have a redesign so probably we can use this bug to study how to get prepared for the feature on the new music ui, I guess the release management will not allow us to add this late feature in 2.1, also we can have time to test it because the performance might be an issue, the implementation should also consider how the scrolling search will be used by the user.

Comment 12

[Hubert Figuiere [:hub]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25465

Dominic, do you have feedback on this?

A couple of things I'd like to see is the UI part being move to the shared code so that contact can use it (lot of it is copy / paste).

Is it the right approach inside music? It works, maybe we a few glitches.

TODO: tests.

Thanks.

Comment 13

[Hubert Figuiere [:hub]]

Comment on attachment 8510463 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25465

Francisco, I'd like to make the alpha scroller from Contacts shared with other apps. Therefor in this PR I move it. 

Do you have any feedback? Any better way to do that?

Thank you kindly.

Comment 14

[Hubert Figuiere [:hub]]

Note that I have issues for things not in latin script. I wonder what would be a practical solution.

Comment 15

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8510463 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25465

Delegating this feedback to Jose, since he worked originally in the alpha scroll component.

I did a quick test and looks good to me, but will prefer if Jose takes a look.

Comment 16

[Hubert Figuiere [:hub]]

Jacqueline,

Do we have or need a UX spec for this. Right now I'm copying the behaviour and UI from contacts.

Thanks,

Comment 17

[Jose Manuel Cantera]

Comment on attachment 8510463 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25465

Thanks Hub,

The patch looks promising. I left a couple of comments on GH. Please ask for a review once you have a final version

cheers

Comment 18

[Hubert Figuiere [:hub]]

And we should see about moving this to a web component at a later stage.

Thank you kindly.

Comment 19

[Jacqueline Savory [:jsavory] UX]

Hi Hubert, following the UI and behaviour from contacts should work in this context. The only difference would be to remove the star or favourite icon from the list of letters (as music doesn't have favourites).

Comment 20

[Hubert Figuiere [:hub]]

I currently only have '#' and A to Z. Thanks.

Comment 21

[Dominic Kuo [:dkuo]]

(In reply to Hubert Figuiere [:hub] from comment #12)
> Created attachment 8510463 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25465
> 
> Dominic, do you have feedback on this?
> 
> A couple of things I'd like to see is the UI part being move to the shared
> code so that contact can use it (lot of it is copy / paste).
> 
> Is it the right approach inside music? It works, maybe we a few glitches.
> 
> TODO: tests.
> 
> Thanks.

I have just finished/landed the phase 1 refactoring of music, and I will look on this patch tomorrow, thanks for the waiting, Hub!

Comment 22

[Hubert Figuiere [:hub]]

I have update the PR, rebased on master after the refactor landed.

Comment 23

[Hubert Figuiere [:hub]]

Comment on attachment 8510463 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25465

Asking for review this time. We have test in marionette.

Jose I have addressed your comments. Otherwise nothing changed this time.

Dominic, I have added the navigation alpha scroller. It is like in Contacts. Let me know if you have issues.
I have a functional for music to make sure we actually scroll properly.
The widget itself is already tested by the original code that is just being moved around.

Thanks you kindly.

Comment 24

[Jose Manuel Cantera]

Comment on attachment 8510463 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25465

LGTM. Please take into account that bug 1076701 made some changes to the position and style of the scrolling bubble. If you haven't rebased your patch please do so

thanks

Comment 25

[Hubert Figuiere [:hub]]

Seems like I need to rebase again. Thanks a lot (also I meant r? not f?).

Thanks.

Comment 26

[Hubert Figuiere [:hub]]

(In reply to Jose Manuel Cantera from comment #24)

> LGTM. Please take into account that bug 1076701 made some changes to the
> position and style of the scrolling bubble. If you haven't rebased your
> patch please do so

There is no change on my side since we use the same CSS. I rebased anyway.

Comment 27

[Dominic Kuo [:dkuo]]

Comment on attachment 8510463 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25465

Hubert,

Thanks for patching this, after I tested with about 500 songs to my sd card, I think I am right about the alpha scrolling does not work when we have all the items in the ListView. Because of the lazy load strategy[1], the ListView only append the first 35 items(about 5 pages) in the dom tree so that switching between artist, album and songs can be responsive, and this means the headers(section-group-x) are not appended to the dom tree yet, the alpha scrolling won't seek to the sections that are not existed, this is a major problem and we have to fix it.

This is what I am worried about in comment 11, it seems that we need at least append the headers to the dom tree first, or the alpha scrolling won't be able to work, and after the alpha scrolling sought the some section, we need to display that section, which means to append the items around that area, so that the user can see the pages around that section if they try to scroll(not using the alpha scrolling) by themselves.

I think we can probably keep the first letters for ListView to use after we know the all the records, and save them(asyncStorage or cookie?) for next time we need to re-display the ListView, or more for next time when music app re-launches. Of course, we should update the saved first letters when the MediaDB has changes, and maybe hide the indexes before we have all the first letters(or all the records) ready, because we shouldn't display indexes when the alpha scrolling is not fully functional.

I am sure there are many details to get the ListView ready for the alpha scrolling, maybe we can split the bug into pieces? so that we can make sure the back-end things for ui I mentioned above are ready, then enable the alpha scrolling later, but that depends on you for sure :) make sense?

Please let me know if you have any questions!

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/music/js/ui/views/list_view.js#L435-L471

Comment 29

[Frederik Braun [:freddy]]

Hubert, are you still working on this?

Comment 30

[Hubert Figuiere [:hub]]

stalled because it doesn't work without changing the way we do the lists and load on demand. Not an easy issue. See comment 27.

Comment 31

[Hubert Figuiere [:hub]]

Bug 1209477 is about doing this for NGA.

Comment 32

[Jim Porter (:squib)]

(In reply to Hubert Figuiere [:hub] from comment #31)
> Bug 1209477 is about doing this for NGA.

Shall we close this one then?

Comment 33

[Hubert Figuiere [:hub]]

It is covered now. 

NGA = the new music app that is now in 2.5

OGA = the old one.