diff --git a/devtools/client/shared/components/test/mochitest/chrome.ini b/devtools/client/shared/components/test/mochitest/chrome.ini index 35a64b11a525..868556998f39 100644 --- a/devtools/client/shared/components/test/mochitest/chrome.ini +++ b/devtools/client/shared/components/test/mochitest/chrome.ini @@ -14,3 +14,4 @@ support-files = [test_tree_08.html] [test_tree_09.html] [test_tree_10.html] +[test_tree_11.html] diff --git a/devtools/client/shared/components/test/mochitest/test_tree_11.html b/devtools/client/shared/components/test/mochitest/test_tree_11.html new file mode 100644 index 000000000000..a36b56aed49a --- /dev/null +++ b/devtools/client/shared/components/test/mochitest/test_tree_11.html @@ -0,0 +1,89 @@ + + + +
+ ++ + ++ + diff --git a/devtools/client/shared/components/tree.js b/devtools/client/shared/components/tree.js index 1f28a1b272f6..86b2badb03f5 100644 --- a/devtools/client/shared/components/tree.js +++ b/devtools/client/shared/components/tree.js @@ -61,7 +61,7 @@ const TreeNode = createFactory(createClass({ expanded: this.props.expanded, visible: this.props.hasChildren, onExpand: this.props.onExpand, - onCollapse: this.props.onCollapse + onCollapse: this.props.onCollapse, }); let isOddRow = this.props.index % 2; @@ -115,14 +115,17 @@ const TreeNode = createFactory(createClass({ */ function oncePerAnimationFrame(fn) { let animationId = null; + let argsToPass = null; return function (...args) { + argsToPass = args; if (animationId !== null) { return; } animationId = requestAnimationFrame(() => { + fn.call(this, ...argsToPass); animationId = null; - fn.call(this, ...args); + argsToPass = null; }); }; } @@ -235,9 +238,11 @@ const Tree = module.exports = createClass({ render() { const traversal = this._dfsFromRoots(); - // Remove 1 from `begin` and add 2 to `end` so that the top and bottom of - // the page are filled with the previous and next items respectively, - // rather than whitespace if the item is not in full view. + // Remove `NUMBER_OF_OFFSCREEN_ITEMS` from `begin` and add `2 * + // NUMBER_OF_OFFSCREEN_ITEMS` to `end` so that the top and bottom of the + // page are filled with the `NUMBER_OF_OFFSCREEN_ITEMS` previous and next + // items respectively, rather than whitespace if the item is not in full + // view. const begin = Math.max(((this.state.scroll / this.props.itemHeight) | 0) - NUMBER_OF_OFFSCREEN_ITEMS, 0); const end = begin + (2 * NUMBER_OF_OFFSCREEN_ITEMS) + ((this.state.height / this.props.itemHeight) | 0); const toRender = traversal.slice(begin, end); @@ -266,7 +271,7 @@ const Tree = module.exports = createClass({ hasChildren: !!this.props.getChildren(item).length, onExpand: this._onExpand, onCollapse: this._onCollapse, - onFocus: () => this._focus(item) + onFocus: () => this._focus(begin + i, item), })); } @@ -284,6 +289,8 @@ const Tree = module.exports = createClass({ className: "tree", ref: "tree", onKeyDown: this._onKeyDown, + onKeyPress: this._preventArrowKeyScrolling, + onKeyUp: this._preventArrowKeyScrolling, onScroll: this._onScroll, style: { padding: 0, @@ -294,6 +301,25 @@ const Tree = module.exports = createClass({ ); }, + _preventArrowKeyScrolling(e) { + switch (e.key) { + case "ArrowUp": + case "ArrowDown": + case "ArrowLeft": + case "ArrowRight": + e.preventDefault(); + e.stopPropagation(); + if (e.nativeEvent) { + if (e.nativeEvent.preventDefault) { + e.nativeEvent.preventDefault(); + } + if (e.nativeEvent.stopPropagation) { + e.nativeEvent.stopPropagation(); + } + } + } + }, + /** * Updates the state's height based on clientHeight. */ @@ -377,9 +403,31 @@ const Tree = module.exports = createClass({ /** * Sets the passed in item to be the focused item. * - * @param {Object} item + * @param {Number} index + * The index of the item in a full DFS traversal (ignoring collapsed + * nodes). Ignored if `item` is undefined. + * + * @param {Object|undefined} item + * The item to be focused, or undefined to focus no item. */ - _focus(item) { + _focus(index, item) { + if (item !== undefined) { + const itemStartPosition = index * this.props.itemHeight; + const itemEndPosition = (index + 1) * this.props.itemHeight; + + // Note that if the height of the viewport (this.state.height) is less than + // `this.props.itemHeight`, we could accidentally try and scroll both up and + // down in a futile attempt to make both the item's start and end positions + // visible. Instead, give priority to the start of the item by checking its + // position first, and then using an "else if", rather than a separate "if", + // for the end position. + if (this.state.scroll > itemStartPosition) { + this.refs.tree.scrollTo(0, itemStartPosition); + } else if ((this.state.scroll + this.state.height) < itemEndPosition) { + this.refs.tree.scrollTo(0, itemEndPosition - this.state.height); + } + } + if (this.props.onFocus) { this.props.onFocus(item); } @@ -389,7 +437,7 @@ const Tree = module.exports = createClass({ * Sets the state to have no focused item. */ _onBlur() { - this._focus(undefined); + this._focus(0, undefined); }, /** @@ -420,20 +468,16 @@ const Tree = module.exports = createClass({ return; } - // Prevent scrolling when pressing navigation keys. Guard against mocked - // events received when testing. - if (e.nativeEvent && e.nativeEvent.preventDefault) { - ViewHelpers.preventScrolling(e.nativeEvent); - } + this._preventArrowKeyScrolling(e); switch (e.key) { case "ArrowUp": this._focusPrevNode(); - return false; + return; case "ArrowDown": this._focusNextNode(); - return false; + return; case "ArrowLeft": if (this.props.isExpanded(this.props.focused) @@ -442,7 +486,7 @@ const Tree = module.exports = createClass({ } else { this._focusParentNode(); } - return false; + return; case "ArrowRight": if (!this.props.isExpanded(this.props.focused)) { @@ -450,8 +494,8 @@ const Tree = module.exports = createClass({ } else { this._focusNextNode(); } - return false; - } + return; + } }, /** @@ -463,6 +507,7 @@ const Tree = module.exports = createClass({ // doesn't exist, we're at the first node already. let prev; + let prevIndex; const traversal = this._dfsFromRoots(); const length = traversal.length; @@ -472,13 +517,14 @@ const Tree = module.exports = createClass({ break; } prev = item; + prevIndex = i; } if (prev === undefined) { return; } - this._focus(prev); + this._focus(prevIndex, prev); }), /** @@ -502,7 +548,7 @@ const Tree = module.exports = createClass({ } if (i + 1 < traversal.length) { - this._focus(traversal[i + 1].item); + this._focus(i + 1, traversal[i + 1].item); } }), @@ -512,8 +558,19 @@ const Tree = module.exports = createClass({ */ _focusParentNode: oncePerAnimationFrame(function () { const parent = this.props.getParent(this.props.focused); - if (parent) { - this._focus(parent); + if (!parent) { + return; } + + const traversal = this._dfsFromRoots(); + const length = traversal.length; + let parentIndex = 0; + for (; parentIndex < length; parentIndex++) { + if (traversal[parentIndex].item === parent) { + break; + } + } + + this._focus(parentIndex, parent); }), });