1003960 - [Home2] Search Integration

Status: RESOLVED FIXED

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

Description

[Kevin Grandon :kgrandon]

We would like to ship the search app along with the vertical homescreen in 2.0.

The benefits to this is that we will have less code in the vertical homescreen, and we can get some early user testing in the search app.

The search app will largely remain the same, with the exception of the removal of local history results.

Comment 1

[Kevin Grandon :kgrandon]

Attachment: Just stashing my WIP here...

Comment 2

[Kevin Grandon :kgrandon]

Attachment: Github pull request

This is actually working surprisingly well. Going to run it by UX and do some more testing first, but feel free to check it out if you'd like.

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8415698 [details] [review]
Github pull request

Hey Ben, Vivien - this patch is shaping up quite nicely, and I think it's at a good point to open it up for review. Changes to rocketbar.js are minimal, and the effort to remove this when we have full rocketbar should be trivial. We also have unit tests and marionette tests in the homescreen case, so that should ease in the maintenance.

I like the fact that we can leverage most of the existing rocketbar.js code paths with this approach. I think this should mean for less duplication and less maintenance overhead. Let me know what you think!

Comment 4

[Kevin Grandon :kgrandon]

Also uploaded this on try just because it's new integration tests for a dev_app which I'm not sure we have yet. Hopefully everything works as expected: https://tbpl.mozilla.org/?tree=Try&rev=759c8decc301

Comment 5

[Ben Francis [:benfrancis]]

Comment on attachment 8415698 [details] [review]
Github pull request

I've left some comments in GitHub.

I'd be interested to get Vivien's take on this, particularly the use of inheritance to extend a 2.1 feature to build a 2.0 feature as I can't make up my mind whether this is a terrible idea or not. I do think it creates more risk of introducing regressions as we work on the full Rocketbar, but it is kind of cool ;)

Comment 6

[Kevin Grandon :kgrandon]

Comment on attachment 8415698 [details] [review]
Github pull request

Hey Ben - 

I think most of your concerns have been addressed, and this should be ready for another round of feedback. 

I think there will need to be some changes to the search app for things like local history and opening of browser windows - though I would prefer to do that in a follow-up so this commit doesn't become massively large.

Please take a look if you have time. Thanks!

Comment 7

[Ben Francis [:benfrancis]]

Comment on attachment 8415698 [details] [review]
Github pull request

Nice work.

Nit: the search bar placeholder text looks off-centre vertically for me on the Open C.

r+me with a green Travis.

Please file follow-up bugs for:
* Disable the places database (both DataStore and IndexedDB) with full Rocketbar turned off.
* Open a browser tab using a view URL web activity from the system app rather than opening a wrapper window from the search app when the user enters a URL.

Comment 8

[Kevin Grandon :kgrandon]

Comment on attachment 8415698 [details] [review]
Github pull request

Think we should be good here. The unit test failure was due to a recent upstream change (the statusbar fix). I've also done a few tweaks to the rocketbar text, and filed some bugs.

Thanks for the review. Once travis is green we'll merge.

Comment 9

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/1f0cd528128dc6047899e1c3f679de9065bad670

Comment 10

[Marcelino Veiga Tuimil [:sonmarce]]

Regarding customization, what is the default search engine and how can it be configured?

Comment 11

[Kevin Grandon :kgrandon]

(In reply to Marcelino Veiga Tuimil [:sonmarce] from comment #10)
> Regarding customization, what is the default search engine and how can it be
> configured?

The default search provider is currently everything.me surfaced by the search app. The search app is replaceable via a setting, but you would need to build an entire search application and tie into the system APIs. It's possible to do, but not an easy task.

The code for the search app is here: https://github.com/mozilla-b2g/gaia/tree/master/apps/search

We define the URL to the search app in settings here: https://github.com/mozilla-b2g/gaia/blob/master/build/settings.js#L150

Comment 12

[Marcelino Veiga Tuimil [:sonmarce]]

OK, thanks for the info

Comment 13

[Kevin Grandon :kgrandon]

Mass modify - set status-b2g-v2.0 fixed for fixed bugs under vertical homescreen dependency tree.