From 36955358355136978385ac8bb829807648c462e2 Mon Sep 17 00:00:00 2001
From: "stuartmorgan@chromium.org"
<stuartmorgan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Thu, 19 Aug 2010 17:30:05 +0000
Subject: DOMUI prefs: Use a List instead of a select for startup pages.
Refactors common elements (favicon source, CSS styling) from the subpages and the main page into shared locations. Also fixes a few small existing mistakes in comments and function naming.
BUG=48713
TEST=DOMUI prefs should show favicons for startup pages.
Review URL: http://codereview.chromium.org/3112017
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56703 0039d316-1c4b-4281-b951-d872f2087c98
---
chrome/browser/dom_ui/add_startup_page_handler.cc | 14 +----
chrome/browser/dom_ui/browser_options_handler.cc | 18 +++++-
.../dom_ui/search_engine_manager_handler.cc | 10 ----
chrome/browser/resources/options.html | 1 +
.../resources/options/add_startup_page_overlay.css | 9 ---
.../options/add_startup_page_recent_pages_list.js | 7 ++-
.../browser/resources/options/browser_options.html | 23 +++++---
.../browser/resources/options/browser_options.js | 61 ++++++++-------------
.../resources/options/browser_options_page.css | 7 ++-
.../options/browser_options_startup_page_list.js | 64 ++++++++++++++++++++++
chrome/browser/resources/options/options_page.css | 13 +++++
.../resources/options/search_engine_manager.css | 9 ---
.../options/search_engine_manager_engine_list.js | 1 +
13 files changed, 141 insertions(+), 96 deletions(-)
create mode 100644 chrome/browser/resources/options/browser_options_startup_page_list.js
diff --git a/chrome/browser/dom_ui/add_startup_page_handler.cc b/chrome/browser/dom_ui/add_startup_page_handler.cc
index 14a4419..5e222c6 100644
--- a/chrome/browser/dom_ui/add_startup_page_handler.cc
+++ b/chrome/browser/dom_ui/add_startup_page_handler.cc
@@ -6,12 +6,9 @@
#include "app/l10n_util.h"
#include "base/basictypes.h"
-#include "base/singleton.h"
#include "base/string_number_conversions.h"
#include "base/utf_string_conversions.h"
#include "base/values.h"
-#include "chrome/browser/chrome_thread.h"
-#include "chrome/browser/dom_ui/dom_ui_favicon_source.h"
#include "chrome/browser/possible_url_model.h"
#include "chrome/browser/pref_service.h"
#include "chrome/browser/profile.h"
@@ -42,19 +39,10 @@ void AddStartupPageHandler::GetLocalizedValues(
}
void AddStartupPageHandler::Initialize() {
- Profile* profile = dom_ui_->GetProfile();
- // Create our favicon data source.
- ChromeThread::PostTask(
- ChromeThread::IO, FROM_HERE,
- NewRunnableMethod(
- Singleton<ChromeURLDataManager>::get(),
- &ChromeURLDataManager::AddDataSource,
- make_scoped_refptr(new DOMUIFavIconSource(profile))));
-
url_table_model_.reset(new PossibleURLModel());
if (url_table_model_.get()) {
url_table_model_->SetObserver(this);
- url_table_model_->Reload(profile);
+ url_table_model_->Reload(dom_ui_->GetProfile());
}
}
diff --git a/chrome/browser/dom_ui/browser_options_handler.cc b/chrome/browser/dom_ui/browser_options_handler.cc
index c4d5209..387ebdd 100644
--- a/chrome/browser/dom_ui/browser_options_handler.cc
+++ b/chrome/browser/dom_ui/browser_options_handler.cc
@@ -7,10 +7,13 @@
#include "app/l10n_util.h"
#include "base/basictypes.h"
#include "base/scoped_ptr.h"
+#include "base/singleton.h"
#include "base/string_number_conversions.h"
#include "base/utf_string_conversions.h"
#include "base/values.h"
+#include "chrome/browser/chrome_thread.h"
#include "chrome/browser/custom_home_pages_table_model.h"
+#include "chrome/browser/dom_ui/dom_ui_favicon_source.h"
#include "chrome/browser/dom_ui/options_managed_banner_handler.h"
#include "chrome/browser/net/url_fixer_upper.h"
#include "chrome/browser/profile.h"
@@ -92,6 +95,14 @@ void BrowserOptionsHandler::RegisterMessages() {
}
void BrowserOptionsHandler::Initialize() {
+ // Create our favicon data source.
+ ChromeThread::PostTask(
+ ChromeThread::IO, FROM_HERE,
+ NewRunnableMethod(
+ Singleton<ChromeURLDataManager>::get(),
+ &ChromeURLDataManager::AddDataSource,
+ make_scoped_refptr(new DOMUIFavIconSource(dom_ui_->GetProfile()))));
+
UpdateDefaultBrowserState();
UpdateStartupPages();
UpdateSearchEngines();
@@ -246,15 +257,16 @@ void BrowserOptionsHandler::UpdateStartupPages() {
}
void BrowserOptionsHandler::OnModelChanged() {
- // TODO(stuartmorgan): Add support for showing favicons.
ListValue startup_pages;
int page_count = startup_custom_pages_table_model_->RowCount();
+ std::vector<GURL> urls = startup_custom_pages_table_model_->GetURLs();
for (int i = 0; i < page_count; ++i) {
DictionaryValue* entry = new DictionaryValue();
entry->SetString("title", WideToUTF16Hack(
- startup_custom_pages_table_model_->GetText(i, 0)));
+ startup_custom_pages_table_model_->GetText(i, 0)));
+ entry->SetString("url", urls[i].spec());
entry->SetString("tooltip", WideToUTF16Hack(
- startup_custom_pages_table_model_->GetTooltip(i)));
+ startup_custom_pages_table_model_->GetTooltip(i)));
startup_pages.Append(entry);
}
diff --git a/chrome/browser/dom_ui/search_engine_manager_handler.cc b/chrome/browser/dom_ui/search_engine_manager_handler.cc
index 9b65346c..18b8ab0 100644
--- a/chrome/browser/dom_ui/search_engine_manager_handler.cc
+++ b/chrome/browser/dom_ui/search_engine_manager_handler.cc
@@ -6,11 +6,9 @@
#include "app/l10n_util.h"
#include "base/callback.h"
-#include "base/singleton.h"
#include "base/string_number_conversions.h"
#include "base/utf_string_conversions.h"
#include "base/values.h"
-#include "chrome/browser/dom_ui/dom_ui_favicon_source.h"
#include "chrome/browser/profile.h"
#include "chrome/browser/search_engines/keyword_editor_controller.h"
#include "chrome/browser/search_engines/template_url_table_model.h"
@@ -27,14 +25,6 @@ SearchEngineManagerHandler::~SearchEngineManagerHandler() {
}
void SearchEngineManagerHandler::Initialize() {
- // Create our favicon data source.
- ChromeThread::PostTask(
- ChromeThread::IO, FROM_HERE,
- NewRunnableMethod(
- Singleton<ChromeURLDataManager>::get(),
- &ChromeURLDataManager::AddDataSource,
- make_scoped_refptr(new DOMUIFavIconSource(dom_ui_->GetProfile()))));
-
controller_.reset(new KeywordEditorController(dom_ui_->GetProfile()));
controller_->table_model()->SetObserver(this);
}
diff --git a/chrome/browser/resources/options.html b/chrome/browser/resources/options.html
index ec35e37..7b0818a 100644
--- a/chrome/browser/resources/options.html
+++ b/chrome/browser/resources/options.html
@@ -47,6 +47,7 @@
<script src="options/advanced_options.js"></script>
<script src="options/autofill_options.js"></script>
<script src="options/browser_options.js"></script>
+<script src="options/browser_options_startup_page_list.js"></script>
<script src="options/clear_browser_data_overlay.js"></script>
<script src="options/content_settings.js"></script>
<script src="options/content_settings_exceptions_area.js"></script>
diff --git a/chrome/browser/resources/options/add_startup_page_overlay.css b/chrome/browser/resources/options/add_startup_page_overlay.css
index 18c7de6..54d367e 100644
--- a/chrome/browser/resources/options/add_startup_page_overlay.css
+++ b/chrome/browser/resources/options/add_startup_page_overlay.css
@@ -19,15 +19,6 @@
-webkit-box-flex: 1;
}
-#addStartupRecentPageList .title {
- background-position: left;
- background-repeat: no-repeat;
- -webkit-padding-start: 20px;
-}
-html[dir=rtl] #addStartupRecentPageList .title {
- background-position: right;
-}
-
#addStartupRecentPageList .url {
display: none;
color: hsl(0, 0%, 70%);
diff --git a/chrome/browser/resources/options/add_startup_page_recent_pages_list.js b/chrome/browser/resources/options/add_startup_page_recent_pages_list.js
index af61a74..8e7df7f 100644
--- a/chrome/browser/resources/options/add_startup_page_recent_pages_list.js
+++ b/chrome/browser/resources/options/add_startup_page_recent_pages_list.js
@@ -7,8 +7,8 @@ cr.define('options.add_startup_page', function() {
const ListItem = cr.ui.ListItem;
/**
- * Creates a new search engine list item.
- * @param {Object} searchEnigne The search engine this represents.
+ * Creates a new recent page list item.
+ * @param {Object} pageInfo The page this item represents.
* @constructor
* @extends {cr.ui.ListItem}
*/
@@ -20,7 +20,7 @@ cr.define('options.add_startup_page', function() {
}
/**
- * Decorates an element as a search engine list item.
+ * Decorates an element as a recent page list item.
* @param {!HTMLElement} el The element to decorate.
*/
RecentPageListItem.decorate = function(el) {
@@ -37,6 +37,7 @@ cr.define('options.add_startup_page', function() {
var titleEl = this.ownerDocument.createElement('span');
titleEl.className = 'title';
+ titleEl.classList.add('favicon-cell');
titleEl.textContent = this.pageInfo['title'];
titleEl.style.backgroundImage = url('chrome://favicon/' +
this.pageInfo['url']);
diff --git a/chrome/browser/resources/options/browser_options.html b/chrome/browser/resources/options/browser_options.html
index 7c3eb14..d9ae42a 100644
--- a/chrome/browser/resources/options/browser_options.html
+++ b/chrome/browser/resources/options/browser_options.html
@@ -13,14 +13,19 @@
pref="session.restore_on_startup" value="4"><span
i18n-content="startupShowPages"></span></label>
<div class="suboption">
- <select id="startupPages" size="3" multiple>
- </select><br>
- <button id="startupAddButton"
- i18n-content="startupAddButton"></button>
- <button id="startupRemoveButton"
- i18n-content="startupRemoveButton"></button>
- <button id="startupUseCurrentButton"
- i18n-content="startupUseCurrent"></button>
+ <div class="left-side-table">
+ <div>
+ <list id="startupPages"></list>
+ </div>
+ <div>
+ <div><button id="startupAddButton"
+ i18n-content="startupAddButton"></button></div>
+ <div><button id="startupRemoveButton"
+ i18n-content="startupRemoveButton"></button></div>
+ <div><button id="startupUseCurrentButton"
+ i18n-content="startupUseCurrent"></button></div>
+ </div>
+ </div>
</div>
</div>
</section>
@@ -47,7 +52,7 @@
<h3 i18n-content="defaultSearchGroupName"></h3>
<div>
<select id="defaultSearchEngine"
- onchange="BrowserOptions.getInstance().setDefaultBrowser()">
+ onchange="BrowserOptions.getInstance().setDefaultSearchEngine()">
</select>
<button id="defaultSearchManageEnginesButton"
i18n-content="defaultSearchManageEnginesLink"></button>
diff --git a/chrome/browser/resources/options/browser_options.js b/chrome/browser/resources/options/browser_options.js
index 6aac858..d4f725d 100644
--- a/chrome/browser/resources/options/browser_options.js
+++ b/chrome/browser/resources/options/browser_options.js
@@ -3,8 +3,9 @@
// found in the LICENSE file.
cr.define('options', function() {
-
- var OptionsPage = options.OptionsPage;
+ const OptionsPage = options.OptionsPage;
+ const ArrayDataModel = cr.ui.ArrayDataModel;
+ const ListSelectionModel = cr.ui.ListSelectionModel;
//
// BrowserOptions class
@@ -28,16 +29,11 @@ cr.define('options', function() {
OptionsPage.prototype.initializePage.call(this);
// Wire up controls.
- var self = this;
- $('startupPages').onchange = function(event) {
- self.updateRemoveButtonState_();
- };
$('startupAddButton').onclick = function(event) {
OptionsPage.showOverlay('addStartupPageOverlay');
};
- $('startupRemoveButton').onclick = function(event) {
- self.removeSelectedStartupPages_();
- };
+ $('startupRemoveButton').onclick = cr.bind(
+ this.removeSelectedStartupPages_, this);
$('startupUseCurrentButton').onclick = function(event) {
chrome.send('setStartupPagesToCurrentPages');
};
@@ -50,6 +46,17 @@ cr.define('options', function() {
};
}
+ var list = $('startupPages');
+ options.browser_options.StartupPageList.decorate(list);
+ list.selectionModel = new ListSelectionModel;
+
+ list.selectionModel.addEventListener(
+ 'change', cr.bind(this.updateRemoveButtonState_, this));
+
+ this.addEventListener('visibleChange', function(event) {
+ $('startupPages').redraw();
+ });
+
// Initialize control enabled states.
Preferences.getInstance().addEventListener('session.restore_on_startup',
cr.bind(this.updateCustomStartupPageControlStates_, this));
@@ -115,14 +122,6 @@ cr.define('options', function() {
},
/**
- * Clears the startup page list.
- * @private
- */
- clearStartupPages_: function() {
- $('startupPages').textContent = '';
- },
-
- /**
* Returns true if the custom startup page control block should
* be enabled.
* @private
@@ -139,17 +138,7 @@ cr.define('options', function() {
* @param {Array} pages List of startup pages.
*/
updateStartupPages_: function(pages) {
- // TODO(stuartmorgan): Replace <select> with a DOMUI List.
- this.clearStartupPages_();
- pageList = $('startupPages');
- pageCount = pages.length;
- for (var i = 0; i < pageCount; i++) {
- var page = pages[i];
- var option = new Option(page['title']);
- option.title = page['tooltip'];
- pageList.appendChild(option);
- }
-
+ $('startupPages').dataModel = new ArrayDataModel(pages);
this.updateRemoveButtonState_();
},
@@ -160,7 +149,6 @@ cr.define('options', function() {
*/
updateCustomStartupPageControlStates_: function() {
var disable = !this.shouldEnableCustomStartupPageControls_();
- $('startupPages').disabled = disable;
$('startupAddButton').disabled = disable;
$('startupUseCurrentButton').disabled = disable;
this.updateRemoveButtonState_();
@@ -174,7 +162,7 @@ cr.define('options', function() {
updateRemoveButtonState_: function() {
var groupEnabled = this.shouldEnableCustomStartupPageControls_();
$('startupRemoveButton').disabled = !groupEnabled ||
- ($('startupPages').selectedIndex == -1);
+ ($('startupPages').selectionModel.selectedIndex == -1);
},
/**
@@ -182,13 +170,8 @@ cr.define('options', function() {
* @private
*/
removeSelectedStartupPages_: function() {
- var pageSelect = $('startupPages');
- var optionCount = pageSelect.options.length;
- var selections = [];
- for (var i = 0; i < optionCount; i++) {
- if (pageSelect.options[i].selected)
- selections.push(String(i));
- }
+ var selections =
+ $('startupPages').selectionModel.selectedIndexes.map(String);
chrome.send('removeStartupPages', selections);
},
@@ -197,7 +180,7 @@ cr.define('options', function() {
* @private
*/
addStartupPage_: function(url) {
- var firstSelection = $('startupPages').selectedIndex;
+ var firstSelection = $('startupPages').selectionModel.selectedIndex;
chrome.send('addStartupPage', [url, String(firstSelection)]);
},
@@ -213,7 +196,7 @@ cr.define('options', function() {
/**
* Set the default search engine based on the popup selection.
*/
- setDefaultBrowser: function() {
+ setDefaultSearchEngine: function() {
var engineSelect = $('defaultSearchEngine');
var selectedIndex = engineSelect.selectedIndex;
if (selectedIndex >= 0) {
diff --git a/chrome/browser/resources/options/browser_options_page.css b/chrome/browser/resources/options/browser_options_page.css
index b3d5a39..3021276 100644
--- a/chrome/browser/resources/options/browser_options_page.css
+++ b/chrome/browser/resources/options/browser_options_page.css
@@ -1,4 +1,9 @@
-#startupPages, #homepageURL {
+#startupPages {
+ border: solid 1px #999999;
+ height: 100px;
+}
+
+#homepageURL {
width: 100%;
}
diff --git a/chrome/browser/resources/options/browser_options_startup_page_list.js b/chrome/browser/resources/options/browser_options_startup_page_list.js
new file mode 100644
index 0000000..85afdc9
--- /dev/null
+++ b/chrome/browser/resources/options/browser_options_startup_page_list.js
@@ -0,0 +1,64 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+cr.define('options.browser_options', function() {
+ const List = cr.ui.List;
+ const ListItem = cr.ui.ListItem;
+
+ /**
+ * Creates a new startup page list item.
+ * @param {Object} pageInfo The page this item represents.
+ * @constructor
+ * @extends {cr.ui.ListItem}
+ */
+ function StartupPageListItem(pageInfo) {
+ var el = cr.doc.createElement('div');
+ el.pageInfo_ = pageInfo;
+ StartupPageListItem.decorate(el);
+ return el;
+ }
+
+ /**
+ * Decorates an element as a startup page list item.
+ * @param {!HTMLElement} el The element to decorate.
+ */
+ StartupPageListItem.decorate = function(el) {
+ el.__proto__ = StartupPageListItem.prototype;
+ el.decorate();
+ };
+
+ StartupPageListItem.prototype = {
+ __proto__: ListItem.prototype,
+
+ /** @inheritDoc */
+ decorate: function() {
+ ListItem.prototype.decorate.call(this);
+
+ var titleEl = this.ownerDocument.createElement('span');
+ titleEl.className = 'title';
+ titleEl.classList.add('favicon-cell');
+ titleEl.textContent = this.pageInfo_['title'];
+ titleEl.style.backgroundImage = url('chrome://favicon/' +
+ this.pageInfo_['url']);
+ titleEl.title = this.pageInfo_['tooltip'];
+
+ this.appendChild(titleEl);
+ },
+ };
+
+ var StartupPageList = cr.ui.define('list');
+
+ StartupPageList.prototype = {
+ __proto__: List.prototype,
+
+ /** @inheritDoc */
+ createItem: function(pageInfo) {
+ return new StartupPageListItem(pageInfo);
+ },
+ };
+
+ return {
+ StartupPageList: StartupPageList
+ };
+});
diff --git a/chrome/browser/resources/options/options_page.css b/chrome/browser/resources/options/options_page.css
index a845f7f..13020f0 100644
--- a/chrome/browser/resources/options/options_page.css
+++ b/chrome/browser/resources/options/options_page.css
@@ -214,6 +214,10 @@ section > h3 {
font-size: 100%;
}
+section > div:only-of-type {
+ -webkit-box-flex: 1;
+}
+
.option {
margin-top: 0;
}
@@ -287,3 +291,12 @@ list > .heading:hover {
.left-side-table > :last-child button {
width: 100%;
}
+
+.favicon-cell {
+ background-position: left;
+ background-repeat: no-repeat;
+ -webkit-padding-start: 20px;
+}
+html[dir=rtl] .favicon-cell {
+ background-position: right;
+}
diff --git a/chrome/browser/resources/options/search_engine_manager.css b/chrome/browser/resources/options/search_engine_manager.css
index 4634880..67e9a8d 100644
--- a/chrome/browser/resources/options/search_engine_manager.css
+++ b/chrome/browser/resources/options/search_engine_manager.css
@@ -11,15 +11,6 @@
-webkit-box-flex: 1;
}
-#searchEngineList .name {
- background-position: left;
- background-repeat: no-repeat;
- -webkit-padding-start: 20px;
-}
-html[dir=rtl] #searchEngineList .name {
- background-position: right;
-}
-
#searchEngineList .keyword {
color: #666666;
}
diff --git a/chrome/browser/resources/options/search_engine_manager_engine_list.js b/chrome/browser/resources/options/search_engine_manager_engine_list.js
index 144a13d..0c634b1 100644
--- a/chrome/browser/resources/options/search_engine_manager_engine_list.js
+++ b/chrome/browser/resources/options/search_engine_manager_engine_list.js
@@ -46,6 +46,7 @@ cr.define('options.search_engines', function() {
} else {
var nameEl = this.ownerDocument.createElement('div');
nameEl.className = 'name';
+ nameEl.classList.add('favicon-cell');
nameEl.textContent = engine['name'];
nameEl.style.backgroundImage = url('chrome://favicon/iconurl/' +
engine['iconURL']);
--
cgit v1.1