Skip to content

Commit

Permalink
Bug 1119515 - Implement initial search suggestions. r=kgrandon
Browse files Browse the repository at this point in the history
  • Loading branch information
daleharvey committed Jan 12, 2015
1 parent d15388e commit 1105180
Show file tree
Hide file tree
Showing 12 changed files with 235 additions and 39 deletions.
14 changes: 8 additions & 6 deletions apps/search/index.html
Expand Up @@ -51,6 +51,7 @@

<!-- Ordering of the following files will impact which queries we send first -->
<script defer src="js/providers/marketplace.js"></script>
<script defer src="js/providers/suggestions.js"></script>
<script defer src="js/providers/bookmarks.js"></script>
<script defer src="js/providers/places.js"></script>
<script defer src="js/providers/local_apps.js"></script>
Expand All @@ -71,12 +72,6 @@
<span data-l10n-id="offline-message"></span>
</div>

<div id="suggestions-wrapper">
<section id="suggestions"
class="suggestions" data-l10n-id="suggestions"
aria-label="Suggestions"></section>
</div>

<div id="suggestions-notice-wrapper" hidden>
<p data-l10n-id="suggestions-settings-notice"></p>
<button id="suggestions-notice-confirm" data-l10n-id="ok"></button>
Expand All @@ -91,6 +86,13 @@
<gaia-grid cols="4" text-rows="3" id="icons"></gaia-grid>
</section>
<div id="loading"><progress></progress></div>

<div id="suggestions-wrapper">
<h2><span id="suggestion-provider"></span></h2>
<section id="suggestions"
class="suggestions" data-l10n-id="suggestions"
aria-label="Suggestions"></section>
</div>
</div>

<menu id="contextmenu" hidden>
Expand Down
107 changes: 107 additions & 0 deletions apps/search/js/providers/suggestions.js
@@ -0,0 +1,107 @@
(function() {

'use strict';
/* globals Promise, Provider, Search */
/* globals SettingsListener */
/* globals LazyLoader */

var provider = null;
var config = [];

var suggestionTpl;
var searchTpl;

function providerUpdated(key) {
if (!provider) {
return;
}

config.forEach(item => {
if (item.urlTemplate === provider) {
searchTpl = item.urlTemplate;
suggestionTpl = item.suggestionsUrlTemplate;
var elem = document.getElementById('suggestion-provider');
navigator.mozL10n.setAttributes(elem, 'search-header', {
provider: item.title.toUpperCase()
});
}
});
}

var SEARCH_PROVIDERS_KEY = 'search.providers';
var SEARCH_TEMPLATE_KEY = 'search.urlTemplate';

var req = navigator.mozSettings.createLock().get(SEARCH_PROVIDERS_KEY);
req.onsuccess = function() {
if (SEARCH_PROVIDERS_KEY in req.result) {
config = req.result[SEARCH_PROVIDERS_KEY];
providerUpdated();
}
};

SettingsListener.observe(SEARCH_TEMPLATE_KEY, false, value => {
if (value) {
provider = value;
providerUpdated();
}
});

function encodeTerms(str, search) {
return str.replace('{searchTerms}', encodeURIComponent(search));
}

function Suggestions() {}

Suggestions.prototype = {

__proto__: Provider.prototype,

name: 'Suggestions',

remote: true,

init: function() {
Provider.prototype.init.apply(this, arguments);
},

click: function(e) {
var suggestion = e.target.dataset.suggestion;
var url = encodeTerms(searchTpl, suggestion);
Search.navigate(url);
},

search: function(input) {
return new Promise((resolve, reject) => {
var url = encodeTerms(suggestionTpl, input);
LazyLoader.getJSON(url, true).then(result => {
var results = result[1];
// We add an item to search the entered term as well
results.unshift(result[0]);
resolve(results);
});
});
},

render: function(items) {
var ul = document.createElement('ul');
ul.setAttribute('role', 'listbox');

items.forEach(function each(item) {
var li = document.createElement('li');
li.dataset.suggestion = li.textContent = item;
li.setAttribute('role', 'option');
ul.appendChild(li);
});

this.clear();

if (ul.childNodes.length) {
this.container.appendChild(ul);
}
}

};

Search.provider(new Suggestions());

}());
2 changes: 2 additions & 0 deletions apps/search/locales/search.en-US.properties
Expand Up @@ -37,3 +37,5 @@ month-8=SEPTEMBER
month-9=OCTOBER
month-10=NOVEMBER
month-11=DECEMBER

search-header={{ provider }} SEARCH
Binary file added apps/search/style/icons/search.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added apps/search/style/icons/search@1.5x.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added apps/search/style/icons/search@2.5x.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added apps/search/style/icons/search@2x.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
80 changes: 50 additions & 30 deletions apps/search/style/search.css
Expand Up @@ -43,38 +43,36 @@ body, a {
display: flex;
}

section.suggestions {
margin: 0 0.7rem;
font-size: 1.4rem;
font-style: italic;
overflow: hidden;
height: 3.5rem;
#suggestions-wrapper {
/* TODO: remove margin-top, fix for incorrectly sized grid */

This comment has been minimized.

Copy link
@KevinGrandon

KevinGrandon Jan 18, 2015

Can you comment about what exactly is incorrectly sized?

This comment has been minimized.

Copy link
@daleharvey

daleharvey Jan 19, 2015

Author Owner

The grid is incorrectly sized, need to follow up and fix it, I remember adding some code to make sure the grid gets a size but it looks to have regressed or have a broken edge case, do you want that added to the comment?

margin-top: 2rem;
}

#suggestions {
font-size: 1.6rem;
overflow-x: hidden;
margin-right: 1.6rem;
}

section.suggestions ul {
#suggestions ul {
list-style: none;
color: #00AACC;
white-space: nowrap;
overflow-x: scroll;
padding-left: 0;
height: 5rem;
}

section.suggestions ul li {
display: inline-block;
margin-left: 1rem;
padding: 0.3rem 0.7rem 0.9rem 0;
#suggestions li {
height: 6rem;
line-height: 6rem;
margin: 0;
border-bottom: 0.1rem solid rgba(255, 255, 255, 0.05);
background-image: url(/style/icons/search.png);
background-repeat: no-repeat;
background-position: 0.8rem center;
text-indent: 4.5rem;
}

section.suggestions ul li+li:before {
content: "\2022";
padding-right: 0.7rem;
color: #666;
}

section.suggestions ul li:first-child {
padding-left: 0.7rem;
#suggestions li:active {
background-color: rgba(0, 202, 242, 0.2);
}

section .result * {
Expand Down Expand Up @@ -131,14 +129,6 @@ gaia-grid {
display: none;
}

#suggestions-wrapper {
display: flex;
}

#suggestions {
flex: 1;
}

#suggestions-notice-wrapper {
background: rgba(45, 45, 45, 0.94);
z-index: 100;
Expand Down Expand Up @@ -194,3 +184,33 @@ gaia-grid {
#loading.loading {
display: block;
}

h2 {
text-align: center;
}

h2 span {
position: relative;
color: #00aac5;
font-size: 1.4rem;
font-weight: normal;
font-style: italic;
}

h2 span:before, h2 span:after {
border-top: 0.3rem solid #5f5f5f;
content:"";

This comment has been minimized.

Copy link
@KevinGrandon

KevinGrandon Jan 18, 2015

nit: probably a space after the colon.

position: absolute;
top: 0.9rem;
right: 9em;
bottom: 0;
width: 80%;
}

h2 span:after {
position: absolute;
top: 0.9rem;
left: 9em;
right:0;
bottom: 0;
}

This comment has been minimized.

Copy link
@KevinGrandon

KevinGrandon Jan 18, 2015

nit: keep newline.

1 change: 1 addition & 0 deletions apps/search/test/marionette/fixtures/suggestions_one.json
@@ -0,0 +1 @@
["foo", ["bar"]]
3 changes: 2 additions & 1 deletion apps/search/test/marionette/lib/search.js
Expand Up @@ -27,7 +27,8 @@ Search.Selectors = {
firstPlaceContainer: '#places',
firstRunConfirm: '#suggestions-notice-confirm',
topSites: '.top-site',
historyResults: '#history .result'
historyResults: '#history .result',
suggestions: '#suggestions li'
};

Search.prototype = {
Expand Down
57 changes: 57 additions & 0 deletions apps/search/test/marionette/suggestions_test.js
@@ -0,0 +1,57 @@
'use strict';

var Rocketbar = require('../../../system/test/marionette/lib/rocketbar.js');
var Server = require('../../../../shared/test/integration/server');

marionette('Search - Suggestions Test', function() {

var client = marionette.client(require(__dirname + '/client_options.js'));
var home, search, rocketbar, system, server;

var providers;

suiteSetup(function(done) {
Server.create(__dirname + '/fixtures/', function(err, _server) {
server = _server;
done();
});
});

setup(function() {
home = client.loader.getAppClass('verticalhome');
system = client.loader.getAppClass('system');
search = client.loader.getAppClass('search');
rocketbar = new Rocketbar(client);
system.waitForStartup();

providers = [{
title: 'first',
urlTemplate: server.url('sample.html'),
suggestionsUrlTemplate: server.url('suggestions_one.json')
}];

client.settings.set('search.providers', providers);
client.settings.set('search.urlTemplate', providers[0].urlTemplate);
});

test('Test suggestions', function() {

home.waitForLaunch();
home.focusRocketBar();

rocketbar.enterText('sometext');
search.goToResults();
// Ensure we get 2 results (hardcoded in the provider results)
client.waitFor(function() {
return client.findElements(search.Selectors.suggestions).length === 2;
});

// Ensure clicking on a result opens the browser correctly
var first = client.helper.waitForElement('#suggestions li');
first.click();

client.switchToFrame();
rocketbar.switchToBrowserFrame(providers[0].urlTemplate);
});

});
10 changes: 8 additions & 2 deletions shared/js/lazy_loader.js
Expand Up @@ -68,12 +68,18 @@ var LazyLoader = (function() {
* Retrieves content of JSON file.
*
* @param {String} file Path to JSON file
* @param {String} mozSystem If xhr should use mozSystem permissions

This comment has been minimized.

Copy link
@KevinGrandon

KevinGrandon Jan 18, 2015

seems like a Boolean and not a String.

* @return {Promise} A promise that resolves to the JSON content
* or null in case of invalid path. Rejects if an error occurs.
*/
getJSON: function(file) {
getJSON: function(file, mozSystem) {
return new Promise(function(resolve, reject) {
var xhr = new XMLHttpRequest();
var xhr;
if (mozSystem) {
xhr = new XMLHttpRequest({mozSystem: true});
} else {
xhr = new XMLHttpRequest();
}
xhr.open('GET', file, true);
xhr.responseType = 'json';

Expand Down

0 comments on commit 1105180

Please sign in to comment.