838033 - [Music][User Story] Music search

Status: RESOLVED FIXED

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

Description

[Peter Dolanjski [:pdol]]

UCID: Music-031

User Story:
As a user I can search for music from any list view to make it easier to find music from a large collection.

Comment 1

[Rob MacDonald [:robmac]]

Here are some proposed flows... https://bugzilla.mozilla.org/attachment.cgi?id=710548

Comment 2

[Rob MacDonald [:robmac]]

UI specs updated and moved to dropbox:

https://www.dropbox.com/s/yepxm9hogne8apy/music-search.pdf

New version includes revisions to search field layout, transitions and additional information on what happens when the user taps on a search result.

Comment 3

[Jim Porter (:squib)]

I've been working on getting my branch to follow the spec, and it's turning out to be quite difficult. Here's how the layout works now: <http://i.imgur.com/LbMDnvO.png>. If I do the obvious, and just make a parent container for the searchbox and each of the views like so: <http://i.imgur.com/v9CLs5S.png>, then the scrollable height for each view is the height of the *tallest* view. It also doesn't let us have different scroll positions for each view.

I'd rather not have a separate search box for each view, since that's going to hurt startup (granted, probably not by a huge amount), and will make it more complicated to transition to the actual search page.

I could try to be tricky and go like the first way, but move the search box around in the DOM once we've finished transitioning to the new view, but that's 1) complicated, and 2) would probably make things behave strangely when we actually start searching. Basically, each view would have a placeholder for the search box at its top that we inject the search box into.

I could also put the search box in a separate section that lies between the header and the views, but then we can't just scroll up until we see the search box. We'd need to do something different, e.g. drag down on the *header* to show the search box. This isn't consistent with other apps, but to be honest, it's the UX that I think makes the most sense for this kind of widget.

Does anyone have any better ideas?

Comment 4

[Jim Porter (:squib)]

(In reply to Jim Porter (:squib) from comment #3)
> I could try to be tricky and go like the first way

This should be "the current way". Sorry for any confusion!

Comment 6

[Rob MacDonald [:robmac]]

I've moved the flows to Dropbox. The latest version is posted here:

https://www.dropbox.com/s/yepxm9hogne8apy/music-search.pdf

Comment 7

[Jim Porter (:squib)]

Rob, any ideas for comment 3?

Comment 8

[Gordon Brander :gordonb]

(In reply to Jim Porter (:squib) from comment #3)

Either of these approaches sounds reasonable to me.

> I'd rather not have a separate search box for each view, since that's going
> to hurt startup (granted, probably not by a huge amount), and will make it
> more complicated to transition to the actual search page.

If we go this route, that means 4 search boxes (for 4 list views). I wouldn't expect having few more DOM nodes to hurt perf, but lazy-loading the panels should remove any marginal cost.

> I could try to be tricky and go like the first way, but move the search box
> around in the DOM once we've finished transitioning to the new view, but
> that's 1) complicated, and 2) would probably make things behave strangely
> when we actually start searching. Basically, each view would have a
> placeholder for the search box at its top that we inject the search box into.

Moving the node between panels also should be effective. I expect it will be even easier if the container for the node has a fixed height (which should be ok for a search box).

Here's a 3rd idea:

* A single scrolling container that holds all panels
* Search box lives at the top of container
* Use JavaScript to set container height to the height of the active panel.
* Since container height is set dynamically, use position:absolute and transform:translateX(),translateY() to do panel animations.

http://d.pr/i/7Zky

Comment 10

[Jim Porter (:squib)]

Quick update on this: I'm cleaning things up, and will be talking to Rob and Gordon tomorrow to see if we can get closer to spec. I should have a review-ready patch by tomorrow. With some luck, I'll have enough finished that some preliminary review can start by this evening.

Comment 11

[Jim Porter (:squib)]

I had a very fruitful conversation with Rob and Gordon, and here's my initial plan (subject to change, of course):

* Add a search field to the top of the tiles and list views, but leave things the same
  otherwise.
* Put the search view *below* all the other views, rather than inline with them.
* When focusing a search field, transitionY both the current view and the search view,
  sliding it into place, but leaving inactive views where they are. This is important
  because it means that the search view has the same Y position as the sublist and player
  views, so transitions between them will work sanely. (This is also more-or-less a case of
  me getting lucky. If the flows were any different, this probably wouldn't work.)

We also talked about removing the transitions when you click on the buttons in the bottom tab bar, since the transitions are meant to show that you're going deeper into the hierarchy, which doesn't apply to those tab bar buttons.

Comment 12

[Dominic Kuo [:dkuo]]

Update:

Jim has a patch for this bug in
https://github.com/mozilla-b2g/gaia/pull/8310
and is already in review process.

We still need to fix some minor bugs and cleanup to complete that patch, I think we should be able to finish this before 3/22.

Comment 13

[Jason Smith [:jsmith]]

I believe Marcia is working on writing test cases for this, right?

Comment 14

[Marcia Knous [:marcia]]

Correct.

(In reply to Jason Smith [:jsmith] from comment #13)
> I believe Marcia is working on writing test cases for this, right?

Comment 15

[Dylan Oliver [:doliver]]

(In reply to Dominic Kuo [:dkuo] from comment #12)
> Update:
> We still need to fix some minor bugs and cleanup to complete that patch, I
> think we should be able to finish this before 3/22.

Hi Dominic, how is the progress here?

Comment 16

[Dominic Kuo [:dkuo]]

Dylan, The patch of music search is complete but buggy, however, I think it should be okay to land it first since it's leo+, and I have found some noticeable bugs on that patch and ask Jim if he can fix them before land it, or I will do the rest parts in followups as solving bugs, let's have music search first on this bug, thanks.

Comment 17

[Jim Porter (:squib)]

I should be landing this later today. I just had one question about Dominic's review comments, which he's answered, and so all I have left is a small fix and then landing.

Comment 18

[Jim Porter (:squib)]

Landed: https://github.com/mozilla-b2g/gaia/pull/8310

Dominic, feel free to CC me on any followups, and I'll help where I can.

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted commit 44abc870fd24f2c4d4de9fe9c2d162d29b16a143 as:
v1-train: 7efa5bad4424646a5759818ef4634d1d4e2160eb

Comment 20

[Michael Vines [:m1] [:evilmachines]]

Looks like we may have a bad v1-train uplift unfortunately.

When launching the music app:
E/GeckoConsole(  504): [JavaScript Error: "ReferenceError: callback is not defined" {file: "app://music.gaiamobile.org/j
s/music.js" line: 400}]

Ref: https://github.com/mozilla-b2g/gaia/commit/7efa5bad4424646a5759818ef4634d1d4e2160eb#L2R400

This also occurs when the play button is pressed, and playback does not start.

Comment 21

[Jim Porter (:squib)]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #20)
> Looks like we may have a bad v1-train uplift unfortunately.

This should be fixed in bug 856716, which is landed on master, blocking leo+, and just waiting for uplift.

Comment 22

[Jason Smith [:jsmith]]

Marcia confirmed we've got test cases in MozTrap for this. Putting + here.