Skip to content

Commit

Permalink
[chrome:omnibox]: Refactoring HTML generation part 2/3, create Output…
Browse files Browse the repository at this point in the history
…ResultsTable helper class.

This is the second of 3 CL's to replace the global static helper functions which manipulate the DOM with classes. In this CL, we replace the function addResultTableToOutput with OutputResultsTable, which is responsible for generating HTML tables from the combined and each individual providers' results.

Bug: 891303
Change-Id: Ie14196a712039b9ca80a1b9376afb6ed8e3dafee
Reviewed-on: https://chromium-review.googlesource.com/c/1294016
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603301}
  • Loading branch information
manuk authored and Commit Bot committed Oct 27, 2018
1 parent 3c28159 commit 1bdf97d
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 55 deletions.
7 changes: 7 additions & 0 deletions chrome/browser/resources/omnibox/omnibox.html
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@
<div id="contents"></div>
</template>

<template id="results-table-template">
<table class="autocomplete-results-table">
<tbody class="results-table-body">
</tbody>
</table>
</template>

<omnibox-inputs id="omnibox-inputs" class="section"></omnibox-inputs>
<omnibox-output id="omnibox-output" class="section"></omnibox-output>
</body>
Expand Down
61 changes: 9 additions & 52 deletions chrome/browser/resources/omnibox/omnibox.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@
* for each autocomplete match. The input parameter is an OmniboxResultMojo.
*/
function addResultToOutput(result) {
const inDetailedMode = omniboxInputs.$$('show-details').checked;

// Output the result-level features in detailed mode and in
// show incomplete results mode. We do the latter because without
// these result-level features, one can't make sense of each
Expand Down Expand Up @@ -114,7 +116,9 @@

// Add combined/merged result table.
const p = document.createElement('p');
p.appendChild(addResultTableToOutput(result.combinedResults));
const table = new omnibox_output.OutputResultsTable(result.combinedResults)
.render(inDetailedMode);
p.appendChild(table);
omniboxOutput.addOutput(p);

// Move forward only if you want to display per provider results.
Expand All @@ -136,61 +140,14 @@
if (providerResults.results.length === 0)
return;
const p = document.createElement('p');
p.appendChild(addResultTableToOutput(providerResults.results));
const table =
new omnibox_output.OutputResultsTable(providerResults.results)
.render(inDetailedMode);
p.appendChild(table);
omniboxOutput.addOutput(p);
});
}

/**
* @param {Object} result an array of AutocompleteMatchMojos.
* @return {Element} that is a user-readable HTML
* representation of this object.
*/
function addResultTableToOutput(result) {
const inDetailedMode = omniboxInputs.$$('show-details').checked;
// Create a table to hold all the autocomplete items.
const table = document.createElement('table');
table.className = 'autocomplete-results-table';
table.appendChild(createAutocompleteResultTableHeader());
// Loop over every autocomplete item and add it as a row in the table.
result.forEach(autocompleteSuggestion => {
const row = new omnibox_output.OutputMatch(autocompleteSuggestion)
.render(inDetailedMode);
table.appendChild(row);
});
return table;
}

/**
* Returns an HTML Element of type table row that contains the
* headers we'll use for labeling the columns. If we're in
* detailedMode, we use all the headers. If not, we only use ones
* marked displayAlways.
*/
function createAutocompleteResultTableHeader() {
const row = document.createElement('tr');
const inDetailedMode = omniboxInputs.$$('show-details').checked;
omnibox_output.PROPERTY_OUTPUT_ORDER.forEach(property => {
if (inDetailedMode || property.displayAlways) {
const headerCell = document.createElement('th');
if (property.url !== '') {
// Wrap header text in URL.
const linkNode = document.createElement('a');
linkNode.href = property.url;
linkNode.textContent = property.header;
headerCell.appendChild(linkNode);
} else {
// Output header text without a URL.
headerCell.textContent = property.header;
headerCell.className = 'table-header';
headerCell.title = property.tooltip;
}
row.appendChild(headerCell);
}
});
return row;
}

/**
* Appends a paragraph node containing text to the parent node.
*/
Expand Down
10 changes: 9 additions & 1 deletion chrome/browser/resources/omnibox/omnibox_element.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class OmniboxElement extends HTMLElement {
constructor(templateId) {
super();
this.attachShadow({mode: 'open'});
const template = $(templateId).content.cloneNode(true);
const template = OmniboxElement.getTemplate(templateId);
this.shadowRoot.appendChild(template);
}

Expand All @@ -24,4 +24,12 @@ class OmniboxElement extends HTMLElement {
$$(id) {
return this.shadowRoot.getElementById(id);
}

/**
* @param {string} templateId
* @return {Element}
*/
static getTemplate(templateId) {
return $(templateId).content.cloneNode(true);
}
}
32 changes: 30 additions & 2 deletions chrome/browser/resources/omnibox/omnibox_output.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,35 @@ cr.define('omnibox_output', function() {
}
}

/** Helps track and render a list of results. */
class OutputResultsTable {
/** @param {Array<!mojom.AutocompleteMatch>} results */
constructor(results) {
/** @type {Array<OutputMatch>} */
this.matches = results.map(match => new OutputMatch(match));
}

/**
* Creates a HTML Node representing this data.
* @param {boolean} showDetails
* @return {Node}
*/
render(showDetails) {
const resultsTable = OmniboxElement.getTemplate('results-table-template');
// The additional properties column only needs be displayed if at least
// one of the results have additional properties.
const showAdditionalPropertiesHeader = this.matches.some(
match => match.showAdditionalProperties(showDetails));
resultsTable.querySelector('.results-table-body')
.appendChild(OutputMatch.renderHeader_(
showDetails, showAdditionalPropertiesHeader));
this.matches.forEach(
match => resultsTable.querySelector('.results-table-body')
.appendChild(match.render(showDetails)));
return resultsTable;
}
}

/** Helps track and render a single match. */
class OutputMatch {
/** @param {!mojom.AutocompleteMatch} match */
Expand Down Expand Up @@ -351,8 +380,7 @@ cr.define('omnibox_output', function() {
// TODO(manukh) use shorthand object creation if/when approved
// https://chromium.googlesource.com/chromium/src/+/master/styleguide/web/es6.md#object-literal-extensions
return {
PROPERTY_OUTPUT_ORDER: PROPERTY_OUTPUT_ORDER,
OmniboxOutput: OmniboxOutput,
OutputMatch: OutputMatch,
OutputResultsTable: OutputResultsTable,
};
});

0 comments on commit 1bdf97d

Please sign in to comment.