Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(327)

Unified Diff: ui/webui/resources/js/cr/ui/list.js

Issue 475633006: Typecheck JS files for chrome://extensions (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@D_asserts_codingconvention
Patch Set: looked twice Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: ui/webui/resources/js/cr/ui/list.js
diff --git a/ui/webui/resources/js/cr/ui/list.js b/ui/webui/resources/js/cr/ui/list.js
index 91df98de53466534e19737e5a41693d7f65864bd..a820b3ad163e51fc57346b7b7cff7fbcc7e11eb5 100644
--- a/ui/webui/resources/js/cr/ui/list.js
+++ b/ui/webui/resources/js/cr/ui/list.js
@@ -21,7 +21,6 @@ cr.define('cr.ui', function() {
* false if the mouseevent was generated over a border or a scrollbar.
* @param {!HTMLElement} el The element to test the event with.
* @param {!Event} e The mouse event.
- * @param {boolean} Whether the mouse event was inside the viewport.
Dan Beam 2014/08/21 18:27:48 this should be @return
Vitaly Pavlenko 2014/08/22 01:43:41 Done.
*/
function inViewport(el, e) {
var rect = el.getBoundingClientRect();
@@ -53,11 +52,11 @@ cr.define('cr.ui', function() {
* is needed. Note that lead item is allowed to have a different height, to
* accommodate lists where a single item at a time can be expanded to show
* more detail.
- * @type {{height: number, marginTop: number, marginBottom:number,
- * width: number, marginLeft: number, marginRight:number}}
+ * @type {?{height: number, marginTop: number, marginBottom: number,
+ * width: number, marginLeft: number, marginRight: number}}
* @private
*/
- measured_: undefined,
+ measured_: null,
/**
* Whether or not the list is autoexpanding. If true, the list resizes
@@ -85,14 +84,13 @@ cr.define('cr.ui', function() {
/**
* Function used to create grid items.
- * @type {function(): !ListItem}
+ * @type {function(new:cr.ui.ListItem, string)}
* @private
*/
itemConstructor_: cr.ui.ListItem,
/**
* Function used to create grid items.
Dan Beam 2014/08/21 18:27:48 * @type {function(new:cr.ui.ListItem, string)}
Vitaly Pavlenko 2014/08/22 01:43:41 Done.
- * @type {function(): !ListItem}
*/
get itemConstructor() {
return this.itemConstructor_;
@@ -242,7 +240,7 @@ cr.define('cr.ui', function() {
/**
* Convenience alias for selectionModel.selectedItems
- * @type {!Array<*>}
+ * @type {!Array.<*>}
*/
get selectedItems() {
var indexes = this.selectionModel.selectedIndexes;
@@ -313,7 +311,8 @@ cr.define('cr.ui', function() {
this.selectionModel = new ListSelectionModel(length);
this.addEventListener('dblclick', this.handleDoubleClick_);
- this.addEventListener('mousedown', handleMouseDown);
+ this.addEventListener('mousedown', /** @type {function(Event)} */(
+ handleMouseDown));
this.addEventListener('dragstart', handleDragStart, true);
this.addEventListener('mouseup', this.handlePointerDownUp_);
this.addEventListener('keydown', this.handleKeyDown);
@@ -380,7 +379,7 @@ cr.define('cr.ui', function() {
},
/**
- * @return {{height: number, width: number}} The height and width
+ * @return {?{height: number, width: number}} The height and width
* of default item, measuring it if necessary.
* @private
*/
@@ -397,8 +396,8 @@ cr.define('cr.ui', function() {
* @param {ListItem=} opt_item The list item to use to do the measuring. If
* this is not provided an item will be created based on the first value
* in the model.
- * @return {{height: number, marginTop: number, marginBottom:number,
- * width: number, marginLeft: number, marginRight:number}}
+ * @return {?{height: number, marginTop: number, marginBottom: number,
+ * width: number, marginLeft: number, marginRight: number}}
* The height and width of the item, taking
* margins into account, and the top, bottom, left and right margins
* themselves.
@@ -406,7 +405,7 @@ cr.define('cr.ui', function() {
measureItem: function(opt_item) {
var dataModel = this.dataModel;
if (!dataModel || !dataModel.length)
- return 0;
+ return null;
var item = opt_item || this.cachedMeasuredItem_ ||
this.createItem(dataModel.item(0));
if (!opt_item) {
@@ -462,7 +461,7 @@ cr.define('cr.ui', function() {
if (this.disabled)
return;
- var target = e.target;
+ var target = /** @type {HTMLElement} */(e.target);
target = this.getListItemAncestor(target);
Dan Beam 2014/08/21 18:27:48 nit: instead of reusing a var here, i'd say change
Vitaly Pavlenko 2014/08/22 01:43:41 Done.
if (target)
@@ -478,7 +477,7 @@ cr.define('cr.ui', function() {
if (this.disabled)
return;
- var target = e.target;
+ var target = /** @type {HTMLElement} */(e.target);
// If the target was this element we need to make sure that the user did
// not click on a border or a scrollbar.
@@ -513,7 +512,7 @@ cr.define('cr.ui', function() {
* @private
*/
handleElementBlur_: function(e) {
- if (!this.contains(e.relatedTarget))
+ if (!this.contains(assertInstanceof(e.relatedTarget, Element)))
this.hasElementFocus = false;
},
@@ -521,26 +520,23 @@ cr.define('cr.ui', function() {
* Returns the list item element containing the given element, or null if
* it doesn't belong to any list item element.
* @param {HTMLElement} element The element.
- * @return {ListItem} The list item containing |element|, or null.
+ * @return {cr.ui.ListItem} The list item containing |element|, or null.
*/
getListItemAncestor: function(element) {
var container = element;
while (container && container.parentNode != this) {
container = container.parentNode;
}
- return container;
+ return assertInstanceof(container, cr.ui.ListItem);
},
/**
* Handle a keydown event.
* @param {Event} e The keydown event.
- * @return {boolean} Whether the key event was handled.
*/
handleKeyDown: function(e) {
- if (this.disabled)
- return;
-
- return this.selectionController_.handleKeyDown(e);
+ if (!this.disabled)
+ this.selectionController_.handleKeyDown(e);
},
/**
@@ -554,7 +550,7 @@ cr.define('cr.ui', function() {
/**
* Callback from the selection model. We dispatch {@code change} events
* when the selection changes.
- * @param {!Event} e Event with change info.
+ * @param {!Event} ce Event with change info.
* @private
*/
handleOnChange_: function(ce) {
@@ -606,7 +602,7 @@ cr.define('cr.ui', function() {
var self = this;
window.setTimeout(function() {
self.scrollIndexIntoView(pe.newValue);
- });
+ }, 0);
}
}
},
@@ -801,6 +797,7 @@ cr.define('cr.ui', function() {
* @return {!ListItem} The newly created list item.
*/
createItem: function(value) {
+ assert(typeof value === "string");
Dan Beam 2014/08/21 18:27:48 assertInstanceOf(value, string);
Vitaly Pavlenko 2014/08/22 01:43:41 Unfortunately, no. "string" is not defined, and
var item = new this.itemConstructor_(value);
item.label = value;
item.id = this.uniqueIdPrefix_ + '-' + this.nextUniqueIdSuffix_++;
@@ -1062,7 +1059,7 @@ cr.define('cr.ui', function() {
this.firstIndex_ = 0;
this.lastIndex_ = 0;
this.remainingSpace_ = this.clientHeight != 0;
- this.mergeItems(0, 0, {}, {});
+ this.mergeItems(0, 0);
return;
}
@@ -1276,10 +1273,11 @@ cr.define('cr.ui', function() {
/**
* Mousedown event handler.
- * @this {List}
+ * @this {cr.ui.List}
* @param {MouseEvent} e The mouse event object.
*/
function handleMouseDown(e) {
+ e.target = /** @type {!HTMLElement} */(e.target);
var listItem = this.getListItemAncestor(e.target);
var wasSelected = listItem && listItem.selected;
this.handlePointerDownUp_(e);
@@ -1307,10 +1305,11 @@ cr.define('cr.ui', function() {
* Dragstart event handler.
* If there is an item at starting position of drag operation and the item
* is not selected, select it.
- * @this {List}
- * @param {MouseEvent} e The event object for 'dragstart'.
+ * @this {cr.ui.List}
+ * @param {Event} e The event object for 'dragstart'.
*/
function handleDragStart(e) {
+ e = /** @type {MouseEvent} */(e);
var element = e.target.ownerDocument.elementFromPoint(e.clientX, e.clientY);
var listItem = this.getListItemAncestor(element);
if (!listItem)

Powered by Google App Engine
This is Rietveld 408576698