Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Sources Tree View #157

Merged
merged 4 commits into from
May 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
public/js/lib/tree.js
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"babel-cli": "^6.7.5",
"babel-polyfill": "^6.7.4",
"babel-preset-stage-0": "^6.5.0",
"classnames": "^2.2.5",
"co": "=4.6.0",
"codemirror": "^5.1.0",
"express": "^4.13.4",
Expand Down
2 changes: 1 addition & 1 deletion public/js/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function navigate() {
// *previous* page as well. For now, emulate the current debugger
// behavior by not showing sources loaded by bfcache.
// return dispatch(sources.loadSources());
}
};
}

module.exports = Object.assign(
Expand Down
2 changes: 1 addition & 1 deletion public/js/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function debugTab(tab, actions) {
actions.newSource(packet.source);
});

yield actions.loadSources();
actions.loadSources();
});
}

Expand Down
60 changes: 55 additions & 5 deletions public/js/components/Sources.css
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
.sources-panel {
flex: 1;
display: flex;
flex-direction: column;
overflow: hidden;
}

.sources-header {
height: 30px;
flex: 0 0 30px;
border-bottom: 1px solid var(--theme-gray);
}

.sources-list {
font-size: 0.75em;
line-height: 2;
list-style: none;
padding-left: 20px;
flex: 1;
display: flex;
overflow: hidden;
font-size: 0.9em;
padding-left: 10px;
}

ul.sources-list {
Expand Down Expand Up @@ -51,3 +55,49 @@ ul.sources-list {
.sources-list .source-item.selected .label {
color: var(--theme-selection-color);
}

.arrow svg {
fill: var(--theme-splitter-color);
margin-right: 3px;
margin-top: 3px;
transform: rotate(-90deg);
transition: transform 0.25s ease;
width: 10px;
}

.arrow.expanded svg {
transform: rotate(0deg);
}

.arrow.hidden {
display: none;
}

.tree {
-webkit-user-select: none;
-moz-user-select: none;
-ms-user-select: none;
-o-user-select: none;
user-select: none;

flex: 1;
white-space: nowrap;
overflow: auto;
}

.tree button {
display: block;
}

.tree .node {
padding: 5px;
}

.tree .node.focused {
color: white;
background-color: var(--theme-selection-background);
}

.tree .node.focused svg {
fill: white;
}
184 changes: 135 additions & 49 deletions public/js/components/Sources.js
Original file line number Diff line number Diff line change
@@ -1,70 +1,156 @@
"use strict";

const React = require("react");
const { DOM: dom, PropTypes } = React;
const { bindActionCreators } = require("redux");
const { connect } = require("react-redux");
const classnames = require("classnames");
const ImPropTypes = require("react-immutable-proptypes");
const ManagedTree = React.createFactory(require("./util/ManagedTree"));
const { Set } = require("immutable");
const actions = require("../actions");
const { getSelectedSource } = require("../selectors");
const { DOM: dom } = React;
const Isvg = React.createFactory(require("react-inlinesvg"));
const { getSelectedSource, getSources } = require("../selectors");
const {
createNode, nodeHasChildren, nodeName,
nodeContents, nodePath, createParentMap,
addToTree
} = require("../util/sources-tree.js");

require("./Sources.css");

function isSelected(selectedSource, source) {
return selectedSource && selectedSource.get("actor") == source.get("actor");
}
// This is inline because it's much faster. We need to revisit how we
// load SVGs, at least for components that render them several times.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

Copy link
Contributor Author

@jlongster jlongster May 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I didn't do this prematurely; I was using inline-svg before but it clearly slowed down the tree because it does an AJAX request for every single SVG, even if it's the same one. The library does not support caching, so I implemented it myself but still hit another problem. I think when you expanded a node, the arrows would not be in the children for a split second and it would "flash" into appearance, bumping the text to the right.

Anyway, this was very straight-forward and immediately worked really well. We may want to look into a SVG->React conversion tool.

let Arrow = (props) => {
return dom.span(
props,
dom.svg(
{ viewBox: "0 0 16 16" },
dom.path({ d: "M8 13.4c-.5 0-.9-.2-1.2-.6L.4 5.2C0 4.7-.1 4.3.2 3.7S1 3 1.6 3h12.8c.6 0 1.2.1 1.4.7.3.6.2 1.1-.2 1.6l-6.4 7.6c-.3.4-.7.5-1.2.5z" }) // eslint-disable-line max-len
)
);
};
Arrow = React.createFactory(Arrow);

function renderSource({ source, selectSource, selectedSource }) {
const pathname = source.get("pathname");
const selectedClass = isSelected(selectedSource, source) ? "selected" : "";
let SourcesTree = React.createClass({
propTypes: {
sources: ImPropTypes.map.isRequired,
selectSource: PropTypes.func.isRequired
},

return dom.li(
{ onClick: () => selectSource(source.toJS()),
className: `source-item ${selectedClass}`,
style: { paddingLeft: "20px" },
key: source.get("url") },
makeInitialState(props) {
const tree = createNode("root", "", []);
for (let source of props.sources.valueSeq()) {
addToTree(tree, source);
}

Isvg({ src: "images/angle-brackets.svg" }),
dom.span({ className: "label" }, pathname)
);
}
return { sourceTree: tree,
parentMap: createParentMap(tree),
focusedItem: null };
},

/**
* Takes a sources object indexed by actor and
* returns a sources object indexed by source domain.
*
* @returns Object
*/
function groupSourcesByDomain(sources) {
return sources.valueSeq()
.filter(source => !!source.get("url"))
.groupBy(source => (new URL(source.get("url"))).hostname);
}
getInitialState() {
return this.makeInitialState(this.props);
},

componentWillReceiveProps(nextProps) {
if (nextProps.sources !== this.props.sources) {
if (nextProps.sources.size === 0) {
this.setState(this.makeInitialState(nextProps));
return;
}

const next = Set(nextProps.sources.valueSeq());
const prev = Set(this.props.sources.valueSeq());
const newSet = next.subtract(prev);

const tree = this.state.sourceTree;
for (let source of newSet) {
addToTree(tree, source);
}

this.setState({ sourceTree: tree,
parentMap: createParentMap(tree) });
}
},

focusItem(item) {
this.setState({ focusedItem: item });
},

selectItem(item) {
if (!nodeHasChildren(item)) {
this.props.selectSource(nodeContents(item).toJS());
}
},

renderItem(item, depth, focused, _, expanded, { setExpanded }) {
const arrow = Arrow({
className: classnames(
"arrow",
{ expanded: expanded,
hidden: !nodeHasChildren(item) }
),
onClick: e => {
e.stopPropagation();
setExpanded(item, !expanded);
}
});

return dom.div(
{ className: classnames("node", { focused }),
style: { marginLeft: depth * 15 + "px" },
onClick: () => this.selectItem(item),
onDoubleClick: e => {
setExpanded(item, !expanded);
} },
arrow,
nodeName(item)
);
},

render() {
const { focusedItem, sourceTree, parentMap } = this.state;

const tree = ManagedTree({
getParent: item => {
return parentMap.get(item);
},
getChildren: item => {
if (nodeHasChildren(item)) {
return nodeContents(item);
}
return [];
},
getRoots: () => nodeContents(sourceTree),
getKey: (item, i) => nodePath(item),
itemHeight: 30,
autoExpandDepth: 2,
onFocus: this.focusItem,
renderItem: this.renderItem
});

return dom.div({
className: "sources-list",
onKeyDown: e => {
if (e.keyCode === 13 && focusedItem) {
this.selectItem(focusedItem);
}
}
}, tree);
}
});
SourcesTree = React.createFactory(SourcesTree);

function Sources({ sources, selectSource, selectedSource }) {
const sourcesByDomain = groupSourcesByDomain(sources);

return dom.div({ className: "sources-panel" },
dom.div(
{ className: "sources-header" }
),
dom.ul(
{ className: "sources-list" },
sourcesByDomain.keySeq().map((domain) => {
return dom.li({ key: domain, className: "domain" },
Isvg({ src: "images/globe.svg" }),
dom.span({ className: "label" }, domain),
dom.ul(null,
sourcesByDomain.get(domain).map(source => renderSource({
source, selectSource, selectedSource }))
)
);
})
)
return dom.div(
{ className: "sources-panel" },
dom.div({ className: "sources-header" }),
SourcesTree({ sources, selectSource })
);
}

module.exports = connect(
state => ({ selectedSource: getSelectedSource(state) }),
state => ({ selectedSource: getSelectedSource(state),
sources: getSources(state) }),
dispatch => bindActionCreators(actions, dispatch)
)(Sources);
58 changes: 58 additions & 0 deletions public/js/components/util/ManagedTree.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
"use strict";

const React = require("react");
const Tree = React.createFactory(require("../../lib/tree"));

let ManagedTree = React.createClass({
propTypes: Tree.propTypes,

getInitialState() {
return { expanded: new WeakMap(),
focusedItem: null };
},

setExpanded(item, expanded) {
const e = this.state.expanded;
e.set(item, expanded);
this.setState({ expanded: e });

if (expanded && this.props.onExpand) {
this.props.onExpand(item);
} else if (!expanded && this.props.onCollapse) {
this.props.onCollapse(item);
}
},

focusItem(item) {
if (this.state.focused !== item) {
this.setState({ focusedItem: item });

if (this.props.onFocus) {
this.props.onFocus(item);
}
}
},

render() {
const { expanded, focusedItem } = this.state;

const props = Object.assign({}, this.props, {
isExpanded: item => expanded.get(item),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some code comments here would be really helpful on why this ManagedTree is needed and how some of the handlers are flowing through. It's a little hard to un-pack what is going on here, even though I know you mentioned to me briefly that you needed this. I'm finding the props on here to be particularly confusing. It somewhat breaks the metaphor in my head of how props get passed in from above, because this component is essentially shadowing the the parent's props. The only two properties that are overwritten are the onFocus and renderItem. Could these be different names, more explicit names to make this clearer?

I don't know if things could be refactored a little bit to make it clearer what's doing what... It could just be a matter of being more vigorous with comments for some context.

Copy link
Contributor Author

@jlongster jlongster May 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's essentially a higher-order component. It's quite useful to do this sort of stuff in React, but how we use it definitely should be documented.

The Tree component that we use has no component state (well, right now I think it does track the window height but I want to remove that). It doesn't keep track of the current focused item, or which nodes are expanded. It's all up to the consumer to handle that. It's nice to do it that way for several reasons: it makes tests easier to write, and it allows the consumer to store that info somewhere else like redux if they want to.

But it puts a lot of burden on the user, especially if they are trying to learn how to use the tree. This is a common problem with components: who manages the state? A good technique is to offer two variants of the same component: one that doesn't manage the state, and one that does (here, we are calling it "managed").

So we're just wrapping the Tree widget (so we can't change the prop names, we are passing all of that into the Tree) and handling the necessary state for it to work.

In practice, React has more going on than just "props passed down, events go up". For ease of use it's local component state is really nice. Also, there's really no reason it has to be separated from Redux state. At some point React should allow the consumer to "mount" the internal state of any component somewhere else; you can read about it in this React issue.

Anywhere, we're still figuring out the best patterns here and we'll definitely document them more.

focused: focusedItem,

onExpand: item => this.setExpanded(item, true),
onCollapse: item => this.setExpanded(item, false),
onFocus: this.focusItem,

renderItem: (...args) => {
return this.props.renderItem(...args, {
setExpanded: this.setExpanded
});
}
});

return Tree(props);
}
});

module.exports = ManagedTree;
Loading