Bug 1152441 - Part 1: Enhance the Tree component for use in performance waterfall r?fitzgen
MozReview-Commit-ID: 753IKgJF6Oj
--- a/devtools/client/shared/components/test/mochitest/test_tree_04.html
+++ b/devtools/client/shared/components/test/mochitest/test_tree_04.html
@@ -10,46 +10,114 @@ Test that we only render visible tree it
<link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
</head>
<body>
<pre id="test">
<script src="head.js" type="application/javascript;version=1.8"></script>
<script type="application/javascript;version=1.8">
window.onload = Task.async(function* () {
try {
+ function getSpacerHeights() {
+ return {
+ top: document.querySelector(".tree > div:first-of-type").clientHeight,
+ bottom: document.querySelector(".tree > div:last-of-type").clientHeight,
+ };
+ }
+
+ const ITEM_HEIGHT = 3;
+
const ReactDOM = browserRequire("devtools/client/shared/vendor/react-dom");
const React = browserRequire("devtools/client/shared/vendor/react");
const Tree = React.createFactory(browserRequire("devtools/client/shared/components/tree"));
- const tree = ReactDOM.render(Tree(TEST_TREE_INTERFACE), window.document.body);
+
+ const tree = ReactDOM.render(
+ Tree(Object.assign({}, TEST_TREE_INTERFACE, { itemHeight: ITEM_HEIGHT })),
+ window.document.body);
TEST_TREE.expanded = new Set("ABCDEFGHIJKLMNO".split(""));
+
yield setState(tree, {
- height: 3,
- scroll: 1
+ height: 3 * ITEM_HEIGHT,
+ scroll: 1 * ITEM_HEIGHT
});
isRenderedTree(document.body.textContent, [
"A:false",
"-B:false",
"--E:false",
"---K:false",
"---L:false",
- ], "Tree should show the second, third, and fourth items + buffer of 1 item at the end");
+ ], "Tree should show the 2nd, 3rd, and 4th items + buffer of 1 item at each end");
+
+ let spacers = getSpacerHeights();
+ is(spacers.top, 0, "Top spacer has the correct height");
+ is(spacers.bottom, 10 * ITEM_HEIGHT, "Bottom spacer has the correct height");
yield setState(tree, {
- height: 2,
- scroll: 3
+ height: 2 * ITEM_HEIGHT,
+ scroll: 3 * ITEM_HEIGHT
});
isRenderedTree(document.body.textContent, [
"--E:false",
"---K:false",
"---L:false",
"--F:false",
- ], "Tree should show the third and fourth item + buffer of 1 item at each end");
+ ], "Tree should show the 4th and 5th item + buffer of 1 item at each end");
+
+ spacers = getSpacerHeights();
+ is(spacers.top, 2 * ITEM_HEIGHT, "Top spacer has the correct height");
+ is(spacers.bottom, 9 * ITEM_HEIGHT, "Bottom spacer has the correct height");
+
+ // Set height to 2 items + 1 pixel at each end, scroll so that 4 items are visible
+ // (2 fully, 2 partially with 1 visible pixel)
+ yield setState(tree, {
+ height: 2 * ITEM_HEIGHT + 2,
+ scroll: 3 * ITEM_HEIGHT - 1
+ });
+
+ isRenderedTree(document.body.textContent, [
+ "-B:false",
+ "--E:false",
+ "---K:false",
+ "---L:false",
+ "--F:false",
+ "--G:false",
+ ], "Tree should show the 4 visible items + buffer of 1 item at each end");
+
+ spacers = getSpacerHeights();
+ is(spacers.top, 1 * ITEM_HEIGHT, "Top spacer has the correct height");
+ is(spacers.bottom, 8 * ITEM_HEIGHT, "Bottom spacer has the correct height");
+
+ yield setState(tree, {
+ height: 20 * ITEM_HEIGHT,
+ scroll: 0
+ });
+
+ isRenderedTree(document.body.textContent, [
+ "A:false",
+ "-B:false",
+ "--E:false",
+ "---K:false",
+ "---L:false",
+ "--F:false",
+ "--G:false",
+ "-C:false",
+ "--H:false",
+ "--I:false",
+ "-D:false",
+ "--J:false",
+ "M:false",
+ "-N:false",
+ "--O:false",
+ ], "Tree should show all rows");
+
+ spacers = getSpacerHeights();
+ is(spacers.top, 0, "Top spacer has zero height");
+ is(spacers.bottom, 0, "Bottom spacer has zero height");
} catch(e) {
ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
} finally {
SimpleTest.finish();
}
});
</script>
</pre>
--- a/devtools/client/shared/components/tree.js
+++ b/devtools/client/shared/components/tree.js
@@ -543,61 +543,70 @@ module.exports = createClass({
}
this._focus(parentIndex, parent);
}),
render() {
const traversal = this._dfsFromRoots();
- // 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);
+ // 'begin' and 'end' are the index of the first (at least partially) visible item
+ // and the index after the last (at least partially) visible item, respectively.
+ // `NUMBER_OF_OFFSCREEN_ITEMS` is removed from `begin` and added to `end` so that
+ // the top and bottom of the page are filled with the `NUMBER_OF_OFFSCREEN_ITEMS`
+ // previous and next items respectively, which helps the user to see fewer empty
+ // gaps when scrolling quickly.
+ const { itemHeight } = this.props;
+ const { scroll, height } = this.state;
+ const begin = Math.max(((scroll / itemHeight) | 0) - NUMBER_OF_OFFSCREEN_ITEMS, 0);
+ const end = Math.ceil((scroll + height) / itemHeight) + NUMBER_OF_OFFSCREEN_ITEMS;
const toRender = traversal.slice(begin, end);
+ const topSpacerHeight = begin * itemHeight;
+ const bottomSpacerHeight = Math.max(traversal.length - end, 0) * itemHeight;
const nodes = [
dom.div({
key: "top-spacer",
style: {
padding: 0,
margin: 0,
- height: begin * this.props.itemHeight + "px"
+ height: topSpacerHeight + "px"
}
})
];
for (let i = 0; i < toRender.length; i++) {
- let { item, depth } = toRender[i];
+ const index = begin + i;
+ const first = index == 0;
+ const last = index == traversal.length - 1;
+ const { item, depth } = toRender[i];
nodes.push(TreeNode({
key: this.props.getKey(item),
- index: begin + i,
- item: item,
- depth: depth,
+ index,
+ first,
+ last,
+ item,
+ depth,
renderItem: this.props.renderItem,
focused: this.props.focused === item,
expanded: this.props.isExpanded(item),
hasChildren: !!this.props.getChildren(item).length,
onExpand: this._onExpand,
onCollapse: this._onCollapse,
onFocus: () => this._focus(begin + i, item),
+ onFocusedNodeUnmount: () => this.refs.tree && this.refs.tree.focus(),
}));
}
nodes.push(dom.div({
key: "bottom-spacer",
style: {
padding: 0,
margin: 0,
- height: (traversal.length - 1 - end) * this.props.itemHeight + "px"
+ height: bottomSpacerHeight + "px"
}
}));
return dom.div(
{
className: "tree",
ref: "tree",
onKeyDown: this._onKeyDown,
@@ -657,16 +666,30 @@ const TreeNode = createFactory(createCla
},
componentDidUpdate() {
if (this.props.focused) {
this.refs.button.focus();
}
},
+ componentWillUnmount() {
+ // If this node is being destroyed and has focus, transfer the focus manually
+ // to the parent tree component. Otherwise, the focus will get lost and keyboard
+ // navigation in the tree will stop working. This is a workaround for a XUL bug.
+ // See bugs 1259228 and 1152441 for details.
+ // DE-XUL: Remove this hack once all usages are only in HTML documents.
+ if (this.props.focused) {
+ this.refs.button.blur();
+ if (this.props.onFocusedNodeUnmount) {
+ this.props.onFocusedNodeUnmount();
+ }
+ }
+ },
+
_buttonAttrs: {
ref: "button",
style: {
opacity: 0,
width: "0 !important",
height: "0 !important",
padding: "0 !important",
outline: "none",
@@ -682,23 +705,35 @@ const TreeNode = createFactory(createCla
const arrow = ArrowExpander({
item: this.props.item,
expanded: this.props.expanded,
visible: this.props.hasChildren,
onExpand: this.props.onExpand,
onCollapse: this.props.onCollapse,
});
- let isOddRow = this.props.index % 2;
+ let classList = [ "tree-node", "div" ];
+ if (this.props.index % 2) {
+ classList.push("tree-node-odd");
+ }
+ if (this.props.first) {
+ classList.push("tree-node-first");
+ }
+ if (this.props.last) {
+ classList.push("tree-node-last");
+ }
+
return dom.div(
{
- className: `tree-node div ${isOddRow ? "tree-node-odd" : ""}`,
+ className: classList.join(" "),
onFocus: this.props.onFocus,
onClick: this.props.onFocus,
onBlur: this.props.onBlur,
+ "data-expanded": this.props.expanded ? "" : undefined,
+ "data-depth": this.props.depth,
style: {
padding: 0,
margin: 0
}
},
this.props.renderItem(this.props.item,
this.props.depth,