From c998dac78d2606ebb4154d688277ff654aab0fb4 Mon Sep 17 00:00:00 2001
From: "estade@chromium.org"
<estade@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 15 Dec 2010 22:11:37 +0000
Subject: [PATCH] [tabbed options] more work on content settings exceptions
lists
- tie "edit" mode to "selected"
-- selecting enters into edit mode
-- blurring exits edit mode and deselects
- fix up layout within rows (faux columns)
- add a special "add new exception" row (this should be at the bottom of the table, but it's currently at the top)
- change behavior when the user attempts to commit an invalid exception. Instead of trying to regrab focus, just revert the row to its previous state.
TODO: list should be sized dynamically, i.e. no scrollbar.
TODO: list should be in sub-sub dialog
TODO: rows should have delete X
BUG=64153
TEST=manual
Review URL: http://codereview.chromium.org/5699004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69332 0039d316-1c4b-4281-b951-d872f2087c98
---
chrome/app/generated_resources.grd | 5 +-
.../options/content_settings_handler.cc | 2 +
.../resources/options/content_settings.js | 10 +-
.../content_settings_exceptions_area.css | 10 +-
.../content_settings_exceptions_area.js | 316 +++++++++++-------
.../resources/options/options_page.css | 2 +-
.../browser/resources/shared/js/cr/ui/list.js | 6 +-
7 files changed, 220 insertions(+), 131 deletions(-)
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 110f36b7b7ecf..e6237af033f66 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -5329,9 +5329,12 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_EXCEPTIONS_OTR_LABEL" desc="A label informing the user that the table below the label shows incognito-only exceptions">
Exceptions below only apply to the current incognito session.
</message>
- <message translateable="false" name="IDS_EXCEPTIONS_PATTERN_EXAMPLE" desc="Placeholder text when the user adds a new exception" >
+ <message translateable="false" name="IDS_EXCEPTIONS_PATTERN_EXAMPLE" desc="Example pattern when the user is adding a new exception" >
[*.]example.com
</message>
+ <message name="IDS_EXCEPTIONS_ADD_NEW_INSTRUCTIONS" desc="Placeholder text before the user adds a new exception" >
+ Add a new exception pattern
+ </message>
<message name="IDS_EXCEPTIONS_NOT_SET_BUTTON" desc="A label to display in the exception page's action column when a site's content setting has not yet been set for a given domain.">
Not set
</message>
diff --git a/chrome/browser/dom_ui/options/content_settings_handler.cc b/chrome/browser/dom_ui/options/content_settings_handler.cc
index cc5a6b1cd9d21..3e5f8d37ba9a3 100644
--- a/chrome/browser/dom_ui/options/content_settings_handler.cc
+++ b/chrome/browser/dom_ui/options/content_settings_handler.cc
@@ -198,6 +198,8 @@ void ContentSettingsHandler::GetLocalizedValues(
l10n_util::GetStringUTF16(IDS_EXCEPTIONS_OTR_LABEL));
localized_strings->SetString("examplePattern",
l10n_util::GetStringUTF16(IDS_EXCEPTIONS_PATTERN_EXAMPLE));
+ localized_strings->SetString("addNewExceptionInstructions",
+ l10n_util::GetStringUTF16(IDS_EXCEPTIONS_ADD_NEW_INSTRUCTIONS));
// Cookies filter.
localized_strings->SetString("cookies_tab_label",
diff --git a/chrome/browser/resources/options/content_settings.js b/chrome/browser/resources/options/content_settings.js
index 96c22793830b5..efc85e84d277e 100644
--- a/chrome/browser/resources/options/content_settings.js
+++ b/chrome/browser/resources/options/content_settings.js
@@ -94,7 +94,7 @@ cr.define('options', function() {
document.querySelector('div[contentType=' + type + ']' +
' list[mode=normal]');
- exceptionsList.clear();
+ exceptionsList.reset();
for (var i = 0; i < list.length; i++) {
exceptionsList.addException(list[i]);
}
@@ -108,7 +108,7 @@ cr.define('options', function() {
exceptionsList.parentNode.classList.remove('hidden');
- exceptionsList.clear();
+ exceptionsList.reset();
for (var i = 0; i < list.length; i++) {
exceptionsList.addException(list[i]);
}
@@ -129,7 +129,7 @@ cr.define('options', function() {
var otrLists = document.querySelectorAll('list[mode=otr]');
for (var i = 0; i < otrLists.length; i++) {
- otrLists[i].clear();
+ otrLists[i].reset();
otrLists[i].parentNode.classList.add('hidden');
}
};
@@ -154,8 +154,8 @@ cr.define('options', function() {
ContentSettings.patternValidityCheckComplete =
function(type, mode, pattern, valid) {
var exceptionsList =
- document.querySelector('div[contentType=' + type + '][mode=' + mode +
- '] list');
+ document.querySelector('div[contentType=' + type + '] ' +
+ 'list[mode=' + mode + ']');
exceptionsList.patternValidityCheckComplete(pattern, valid);
};
diff --git a/chrome/browser/resources/options/content_settings_exceptions_area.css b/chrome/browser/resources/options/content_settings_exceptions_area.css
index ee8ca82f7a48f..63fc87b5863c3 100644
--- a/chrome/browser/resources/options/content_settings_exceptions_area.css
+++ b/chrome/browser/resources/options/content_settings_exceptions_area.css
@@ -4,11 +4,13 @@ Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
*/
-.exceptionInput {
- /* TODO(estade): need something here to make it fill the remaining space. */
+.exceptionPattern {
+ display: -webkit-box;
+ -webkit-box-flex: 1;
}
.exceptionSetting {
- float: right;
- margin-right: 30px;
+ display: inline-block;
+ width: 100px;
+ margin-right: 20px;
}
diff --git a/chrome/browser/resources/options/content_settings_exceptions_area.js b/chrome/browser/resources/options/content_settings_exceptions_area.js
index f727476bee28a..70775599055e4 100644
--- a/chrome/browser/resources/options/content_settings_exceptions_area.js
+++ b/chrome/browser/resources/options/content_settings_exceptions_area.js
@@ -39,21 +39,27 @@ cr.define('options.contentSettings', function() {
decorate: function() {
ListItem.prototype.decorate.call(this);
- // Labels for display mode.
- var patternLabel = cr.doc.createElement('span');
- patternLabel.textContent = this.pattern;
- this.appendChild(patternLabel);
-
- var settingLabel = cr.doc.createElement('span');
- settingLabel.textContent = this.settingForDisplay();
- settingLabel.className = 'exceptionSetting';
- this.appendChild(settingLabel);
+ // Labels for display mode. |pattern| will be null for the 'add new
+ // exception' row.
+ if (this.pattern) {
+ var patternLabel = cr.doc.createElement('span');
+ patternLabel.textContent = this.pattern;
+ patternLabel.className = 'exceptionPattern';
+ this.appendChild(patternLabel);
+ this.patternLabel = patternLabel;
+
+ var settingLabel = cr.doc.createElement('span');
+ settingLabel.textContent = this.settingForDisplay();
+ settingLabel.className = 'exceptionSetting';
+ this.appendChild(settingLabel);
+ this.settingLabel = settingLabel;
+ }
// Elements for edit mode.
var input = cr.doc.createElement('input');
input.type = 'text';
this.appendChild(input);
- input.className = 'exceptionInput hidden';
+ input.className = 'exceptionPattern hidden';
var select = cr.doc.createElement('select');
var optionAllow = cr.doc.createElement('option');
@@ -93,26 +99,23 @@ cr.define('options.contentSettings', function() {
// empty input.
this.inputIsValid = true;
- this.patternLabel = patternLabel;
- this.settingLabel = settingLabel;
this.input = input;
this.select = select;
this.optionAllow = optionAllow;
this.optionBlock = optionBlock;
this.updateEditables();
- if (!this.pattern)
- input.value = templateData.examplePattern;
var listItem = this;
- this.ondblclick = function(event) {
+
+ this.addEventListener('selectedChange', function(event) {
// Editing notifications and geolocation is disabled for now.
if (listItem.contentType == 'notifications' ||
listItem.contentType == 'location')
return;
- listItem.editing = true;
- };
+ listItem.editing = listItem.selected;
+ });
// Handle events on the editable nodes.
input.oninput = function(event) {
@@ -132,32 +135,14 @@ cr.define('options.contentSettings', function() {
case 'U+001B': // Esc
// Reset the inputs.
listItem.updateEditables();
- if (listItem.pattern)
- listItem.maybeSetPatternValid(listItem.pattern, true);
+ listItem.setPatternValid(true);
case 'Enter':
- if (listItem.parentNode)
- listItem.parentNode.focus();
+ listItem.ownerDocument.activeElement.blur();
}
}
- function handleBlur(e) {
- // When the blur event happens we do not know who is getting focus so we
- // delay this a bit since we want to know if the other input got focus
- // before deciding if we should exit edit mode.
- var doc = e.target.ownerDocument;
- window.setTimeout(function() {
- var activeElement = doc.activeElement;
- if (!listItem.contains(activeElement)) {
- listItem.editing = false;
- }
- }, 50);
- }
-
input.addEventListener('keydown', handleKeydown);
- input.addEventListener('blur', handleBlur);
-
select.addEventListener('keydown', handleKeydown);
- select.addEventListener('blur', handleBlur);
},
/**
@@ -199,20 +184,12 @@ cr.define('options.contentSettings', function() {
},
/**
- * Update this list item to reflect whether the input is a valid pattern
- * if |pattern| matches the text currently in the input.
- * @param {string} pattern The pattern.
+ * Update this list item to reflect whether the input is a valid pattern.
* @param {boolean} valid Whether said pattern is valid in the context of
* a content exception setting.
*/
- maybeSetPatternValid: function(pattern, valid) {
- // Don't do anything for messages where we are not the intended recipient,
- // or if the response is stale (i.e. the input value has changed since we
- // sent the request to analyze it).
- if (pattern != this.input.value)
- return;
-
- if (valid)
+ setPatternValid: function(valid) {
+ if (valid || !this.input.value)
this.input.setCustomValidity('');
else
this.input.setCustomValidity(' ');
@@ -220,11 +197,19 @@ cr.define('options.contentSettings', function() {
this.inputValidityKnown = true;
},
+ /**
+ * Set the <input> to its original contents. Used when the user quits
+ * editing.
+ */
+ resetInput: function() {
+ this.input.value = this.pattern;
+ },
+
/**
* Copy the data model values to the editable nodes.
*/
updateEditables: function() {
- this.input.value = this.pattern;
+ this.resetInput();
if (this.setting == 'allow')
this.optionAllow.selected = true;
@@ -236,6 +221,17 @@ cr.define('options.contentSettings', function() {
this.optionAsk.selected = true;
},
+ /**
+ * Fiddle with the display of elements of this list item when the editing
+ * mode changes.
+ */
+ toggleVisibilityForEditing: function() {
+ this.patternLabel.classList.toggle('hidden');
+ this.settingLabel.classList.toggle('hidden');
+ this.input.classList.toggle('hidden');
+ this.select.classList.toggle('hidden');
+ },
+
/**
* Whether the user is currently able to edit the list item.
* @type {boolean}
@@ -248,54 +244,41 @@ cr.define('options.contentSettings', function() {
if (oldEditing == editing)
return;
- var listItem = this;
- var pattern = this.pattern;
- var setting = this.setting;
- var patternLabel = this.patternLabel;
- var settingLabel = this.settingLabel;
var input = this.input;
- var select = this.select;
- var optionAllow = this.optionAllow;
- var optionBlock = this.optionBlock;
- var optionSession = this.optionSession;
- var optionAsk = this.optionAsk;
-
- // Just delete this row if it was added via the Add button.
- if (!editing && !pattern && !input.value) {
- var model = listItem.parentNode.dataModel;
- model.splice(model.indexOf(listItem.dataItem), 1);
- return;
- }
- // Check that we have a valid pattern and if not we do not change the
- // editing mode.
- if (!editing && (!this.inputValidityKnown || !this.inputIsValid)) {
- input.focus();
- input.select();
- return;
- }
-
- patternLabel.classList.toggle('hidden');
- settingLabel.classList.toggle('hidden');
- input.classList.toggle('hidden');
- select.classList.toggle('hidden');
-
- var doc = this.ownerDocument;
- var area = doc.querySelector('div[contentType=' +
- listItem.contentType + '][mode=' + listItem.mode + ']');
- area.enableAddAndEditButtons(!editing);
+ this.toggleVisibilityForEditing();
if (editing) {
this.setAttribute('editing', '');
cr.ui.limitInputWidth(input, this, 20);
- input.focus();
- input.select();
+ // When this is called in response to the selectedChange event,
+ // the list grabs focus immediately afterwards. Thus we must delay
+ // our focus grab.
+ window.setTimeout(function() {
+ input.focus();
+ input.select();
+ }, 50);
+
+ // TODO(estade): should we insert example text here for the AddNewRow
+ // input?
} else {
this.removeAttribute('editing');
+ // Check that we have a valid pattern and if not we do not, abort
+ // changes to the exception.
+ if (!this.inputValidityKnown || !this.inputIsValid) {
+ this.updateEditables();
+ this.setPatternValid(true);
+ return;
+ }
+
var newPattern = input.value;
var newSetting;
+ var optionAllow = this.optionAllow;
+ var optionBlock = this.optionBlock;
+ var optionSession = this.optionSession;
+ var optionAsk = this.optionAsk;
if (optionAllow.selected)
newSetting = 'allow';
else if (optionBlock.selected)
@@ -305,25 +288,102 @@ cr.define('options.contentSettings', function() {
else if (optionAsk && optionAsk.selected)
newSetting = 'ask';
- // Empty edit - do nothing.
- if (pattern == newPattern && newSetting == this.setting)
- return;
+ this.finishEdit(newPattern, newSetting);
+ }
+ },
- this.pattern = patternLabel.textContent = newPattern;
- this.setting = newSetting;
- settingLabel.textContent = this.settingForDisplay();
+ /**
+ * Editing is complete; update the model.
+ * @type {string} newPattern The pattern that the user entered.
+ * @type {string} newSetting The setting the user chose.
+ */
+ finishEdit: function(newPattern, newSetting) {
+ // Empty edit - do nothing.
+ if (newPattern == this.pattern && newSetting == this.setting)
+ return;
- if (pattern != this.pattern) {
- chrome.send('removeExceptions',
- [this.contentType, this.mode, pattern]);
- }
+ this.patternLabel.textContent = newPattern;
+ this.settingLabel.textContent = this.settingForDisplay();
+ var oldPattern = this.pattern;
+ this.pattern = newPattern;
+ this.setting = newSetting;
- chrome.send('setException',
- [this.contentType, this.mode, this.pattern, this.setting]);
+ if (oldPattern != newPattern) {
+ chrome.send('removeExceptions',
+ [this.contentType, this.mode, oldPattern]);
}
+
+ chrome.send('setException',
+ [this.contentType, this.mode, newPattern, newSetting]);
}
};
+ /**
+ * Creates a new list item for the Add New Item row, which doesn't represent
+ * an actual entry in the exceptions list but allows the user to add new
+ * exceptions.
+ * @param {string} contentType The type of the list.
+ * @param {string} mode The browser mode, 'otr' or 'normal'.
+ * @param {boolean} enableAskOption Whether to show an 'ask every time'
+ * option in the select.
+ * @constructor
+ * @extends {cr.ui.ExceptionsListItem}
+ */
+ function ExceptionsAddRowListItem(contentType, mode, enableAskOption) {
+ var el = cr.doc.createElement('li');
+ el.mode = mode;
+ el.contentType = contentType;
+ el.enableAskOption = enableAskOption;
+ el.dataItem = [];
+ el.__proto__ = ExceptionsAddRowListItem.prototype;
+ el.decorate();
+
+ return el;
+ }
+
+ ExceptionsAddRowListItem.prototype = {
+ __proto__: ExceptionsListItem.prototype,
+
+ decorate: function() {
+ ExceptionsListItem.prototype.decorate.call(this);
+
+ this.input.placeholder = templateData.addNewExceptionInstructions;
+ this.input.classList.remove('hidden');
+ this.select.classList.remove('hidden');
+
+ // Do we always want a default of allow?
+ this.setting = 'allow';
+ },
+
+ /**
+ * Clear the <input> and let the placeholder text show again.
+ */
+ resetInput: function() {
+ this.input.value = '';
+ },
+
+ /**
+ * No elements show or hide when going into edit mode, so do nothing.
+ */
+ toggleVisibilityForEditing: function() {
+ // No-op.
+ },
+
+ /**
+ * Editing is complete; update the model. As long as the pattern isn't
+ * empty, we'll just add it.
+ * @type {string} newPattern The pattern that the user entered.
+ * @type {string} newSetting The setting the user chose.
+ */
+ finishEdit: function(newPattern, newSetting) {
+ if (newPattern == '')
+ return;
+
+ chrome.send('setException',
+ [this.contentType, this.mode, newPattern, newSetting]);
+ },
+ };
+
/**
* Creates a new exceptions list.
* @constructor
@@ -340,11 +400,29 @@ cr.define('options.contentSettings', function() {
decorate: function() {
List.prototype.decorate.call(this);
- this.dataModel = new ArrayDataModel([]);
+ this.contentType = this.parentNode.getAttribute('contentType');
+ this.mode = this.getAttribute('mode');
+
+ var exceptionList = this;
+ function handleBlur(e) {
+ // When the blur event happens we do not know who is getting focus so we
+ // delay this a bit until we know if the new focus node is outside the
+ // list.
+ var doc = e.target.ownerDocument;
+ window.setTimeout(function() {
+ var activeElement = doc.activeElement;
+ if (!exceptionList.contains(activeElement))
+ exceptionList.selectionModel.clear();
+ }, 50);
+ }
+
+ this.addEventListener('blur', handleBlur, true);
// Whether the exceptions in this list allow an 'Ask every time' option.
this.enableAskOption = (this.contentType == 'plugins' &&
templateData.enable_click_to_play);
+
+ this.reset();
},
/**
@@ -352,10 +430,16 @@ cr.define('options.contentSettings', function() {
* @param {Object} entry The element from the data model for this row.
*/
createItem: function(entry) {
- return new ExceptionsListItem(this.contentType,
- this.mode,
- this.enableAskOption,
- entry);
+ if (entry) {
+ return new ExceptionsListItem(this.contentType,
+ this.mode,
+ this.enableAskOption,
+ entry);
+ } else {
+ return new ExceptionsAddRowListItem(this.contentType,
+ this.mode,
+ this.enableAskOption);
+ }
},
/**
@@ -364,16 +448,6 @@ cr.define('options.contentSettings', function() {
*/
addException: function(entry) {
this.dataModel.push(entry);
-
- // When an empty row is added, put it into editing mode.
- if (!entry['displayPattern'] && !entry['setting']) {
- var index = this.dataModel.length - 1;
- var sm = this.selectionModel;
- sm.anchorIndex = sm.leadIndex = sm.selectedIndex = index;
- this.scrollIndexIntoView(index);
- var li = this.getListItemByIndex(index);
- li.editing = true;
- }
},
/**
@@ -384,18 +458,23 @@ cr.define('options.contentSettings', function() {
* a content exception setting.
*/
patternValidityCheckComplete: function(pattern, valid) {
- for (var i = 0; i < this.dataModel.length; i++) {
- var listItem = this.getListItemByIndex(i);
- if (listItem)
- listItem.maybeSetPatternValid(pattern, valid);
+ var listItems = this.items;
+ for (var i = 0; i < listItems.length; i++) {
+ var listItem = listItems[i];
+ // Don't do anything for messages for the item if it is not the intended
+ // recipient, or if the response is stale (i.e. the input value has
+ // changed since we sent the request to analyze it).
+ if (pattern == listItem.input.value)
+ listItem.setPatternValid(valid);
}
},
/**
* Removes all exceptions from the js model.
*/
- clear: function() {
- this.dataModel = new ArrayDataModel([]);
+ reset: function() {
+ // The null creates the Add New Exception row.
+ this.dataModel = new ArrayDataModel([null]);
},
/**
@@ -434,6 +513,7 @@ cr.define('options.contentSettings', function() {
return {
ExceptionsListItem: ExceptionsListItem,
+ ExceptionsAddRowListItem: ExceptionsAddRowListItem,
ExceptionsList: ExceptionsList,
};
});
diff --git a/chrome/browser/resources/options/options_page.css b/chrome/browser/resources/options/options_page.css
index 491b54393c11f..e7aa4593a511d 100644
--- a/chrome/browser/resources/options/options_page.css
+++ b/chrome/browser/resources/options/options_page.css
@@ -322,7 +322,7 @@ section > div:only-of-type label {
}
.hidden {
- display: none;
+ display: none !important;
}
.touch-slider {
diff --git a/chrome/browser/resources/shared/js/cr/ui/list.js b/chrome/browser/resources/shared/js/cr/ui/list.js
index 5db46a3cddcdf..749d0ea88aeac 100644
--- a/chrome/browser/resources/shared/js/cr/ui/list.js
+++ b/chrome/browser/resources/shared/js/cr/ui/list.js
@@ -223,12 +223,14 @@ cr.define('cr.ui', function() {
},
/**
- * The HTML elements representing the items. This is just all the element
+ * The HTML elements representing the items. This is just all the list item
* children but subclasses may override this to filter out certain elements.
* @type {HTMLCollection}
*/
get items() {
- return this.children;
+ return Array.prototype.filter.call(this.children, function(child) {
+ return !child.classList.contains('spacer');
+ });
},
batchCount_: 0,
--
GitLab