Factor out TableRow as a reusable abstraction (#652)

We currently have two components which create rows in our PagerankTable:
the `NodeRow` and `ConnectionRow`. Work on #502 will result in the
addition of a new one, the `AggregationRow`. It's time to stop
duplicating logic (and testing) of the shared behavior for these rows,
like depth-based styling, row expansion/collapse, etc. This commit pulls
all the common logic to rendering rows into a single, thoroughly tested
`TableRow` component.

There is one observable change in the UI: when a connection percentage
is not available (i.e. for NodeRows), we now leave the column empty
rather than placing a dash in the column. I think this is visually
cleaner.

Test plan: Unit tests pass, and this part of the code base is thoroughly
tested, so that's a pretty reliable indicator. I also poked around the
live PagerankTable in the cred explorer, just to be safe.
This commit is contained in:
Dandelion Mané 2018-08-13 12:31:16 -07:00 committed by GitHub
parent f1c5d3756d
commit c5e20f9400
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 364 additions and 250 deletions

View File

@ -7,14 +7,9 @@ import type {NodeAddressT} from "../../../core/graph";
import type {Connection} from "../../../core/attribution/graphToMarkovChain";
import type {ScoredConnection} from "../../../core/attribution/pagerankNodeDecomposition";
import {DynamicAdapterSet} from "../../adapters/adapterSet";
import {TableRow} from "./TableRow";
import {
edgeVerb,
nodeDescription,
scoreDisplay,
type SharedProps,
type RowState,
} from "./shared";
import {edgeVerb, nodeDescription, type SharedProps} from "./shared";
type ConnectionRowListProps = {|
+depth: number,
@ -54,14 +49,7 @@ type ConnectionRowProps = {|
+sharedProps: SharedProps,
|};
export class ConnectionRow extends React.PureComponent<
ConnectionRowProps,
RowState
> {
constructor() {
super();
this.state = {expanded: false};
}
export class ConnectionRow extends React.PureComponent<ConnectionRowProps> {
render() {
const {
sharedProps,
@ -70,45 +58,25 @@ export class ConnectionRow extends React.PureComponent<
scoredConnection: {connection, source, sourceScore, connectionScore},
} = this.props;
const {pnd, adapters} = sharedProps;
const {expanded} = this.state;
const {score: targetScore} = NullUtil.get(pnd.get(target));
const connectionProportion = connectionScore / targetScore;
const connectionPercent = (connectionProportion * 100).toFixed(2);
const connectionView = (
<ConnectionView connection={connection} adapters={adapters} />
);
return (
<React.Fragment>
<tr
key="self"
style={{backgroundColor: `rgba(0,143.4375,0,${1 - 0.9 ** depth})`}}
>
<td style={{display: "flex", alignItems: "flex-start"}}>
<button
style={{
marginRight: 5,
marginLeft: 15 * depth,
}}
onClick={() => {
this.setState(({expanded}) => ({
expanded: !expanded,
}));
}}
>
{expanded ? "\u2212" : "+"}
</button>
<ConnectionView connection={connection} adapters={adapters} />
</td>
<td style={{textAlign: "right"}}>{connectionPercent}%</td>
<td style={{textAlign: "right"}}>{scoreDisplay(sourceScore)}</td>
</tr>
{expanded && (
<ConnectionRowList
key="children"
depth={depth + 1}
node={source}
sharedProps={sharedProps}
/>
)}
</React.Fragment>
<TableRow
depth={depth}
description={connectionView}
connectionProportion={connectionProportion}
score={sourceScore}
>
<ConnectionRowList
depth={depth + 1}
node={source}
sharedProps={sharedProps}
/>
</TableRow>
);
}
}

View File

@ -6,7 +6,8 @@ import * as NullUtil from "../../../util/null";
import type {Connection} from "../../../core/attribution/graphToMarkovChain";
import {ConnectionRowList, ConnectionRow, ConnectionView} from "./Connection";
import {example, COLUMNS} from "./sharedTestUtils";
import {example} from "./sharedTestUtils";
import {TableRow} from "./TableRow";
require("../../testUtil").configureEnzyme();
@ -79,93 +80,66 @@ describe("app/credExplorer/pagerankTable/Connection", () => {
(sc) => sc.source === nodes.fooAlpha
);
expect(alphaConnections).toHaveLength(1);
const connection = alphaConnections[0];
const {source} = connection;
const scoredConnection = alphaConnections[0];
const depth = 2;
const component = (
<ConnectionRow
depth={depth}
target={target}
scoredConnection={connection}
scoredConnection={scoredConnection}
sharedProps={sharedProps}
/>
);
const element = shallow(component);
return {element, depth, target, source, connection, sharedProps};
const row = shallow(component).find(TableRow);
return {row, depth, target, scoredConnection, sharedProps};
}
it("renders the right number of columns", async () => {
expect((await setup()).element.find("td")).toHaveLength(COLUMNS().length);
});
it("has proper depth-based styling", async () => {
const {element} = await setup();
expect({
buttonStyle: element.find("button").prop("style"),
trStyle: element.find("tr").prop("style"),
}).toMatchSnapshot();
});
it("renders the source view", async () => {
const {element, sharedProps, connection} = await setup();
const descriptionColumn = COLUMNS().indexOf("Description");
expect(descriptionColumn).not.toEqual(-1);
const view = element
.find("td")
.at(descriptionColumn)
.find("ConnectionView");
expect(view).toHaveLength(1);
expect(view.props()).toEqual({
adapters: sharedProps.adapters,
connection: connection.connection,
describe("instantiates a TableRow", () => {
it("with the correct depth", async () => {
const {row, depth} = await setup();
expect(row.props().depth).toBe(depth);
});
it("with the sourceScore", async () => {
const {row, scoredConnection} = await setup();
expect(row.props().score).toBe(scoredConnection.sourceScore);
});
it("with the connectionProportion", async () => {
const {row, target, scoredConnection, sharedProps} = await setup();
const targetScore = NullUtil.get(sharedProps.pnd.get(target)).score;
expect(row.props().connectionProportion).toBe(
scoredConnection.connectionScore / targetScore
);
});
it("with a ConnectionView as description", async () => {
const {row, sharedProps, scoredConnection} = await setup();
const {adapters} = sharedProps;
const description = row.props().description;
const cv = shallow(description).instance();
expect(cv).toBeInstanceOf(ConnectionView);
expect(cv.props.connection).toEqual(scoredConnection.connection);
expect(cv.props.adapters).toEqual(adapters);
});
describe("with a ConnectionRowList as children", () => {
function getChildren(row) {
const children = row.props().children;
return shallow(children).instance();
}
it("which is a ConnectionRowList", async () => {
const {row} = await setup();
expect(getChildren(row)).toBeInstanceOf(ConnectionRowList);
});
it("which has incremented depth", async () => {
const {row, depth} = await setup();
expect(getChildren(row).props.depth).toBe(depth + 1);
});
it("which has the connection source as its node target", async () => {
const {row, scoredConnection} = await setup();
expect(getChildren(row).props.node).toBe(scoredConnection.source);
});
it("which has the right sharedProps", async () => {
const {row, sharedProps} = await setup();
expect(getChildren(row).props.sharedProps).toBe(sharedProps);
});
});
});
it("renders the connection percentage", async () => {
const {element, connection, sharedProps, target} = await setup();
const connectionColumn = COLUMNS().indexOf("Connection");
expect(connectionColumn).not.toEqual(-1);
const proportion =
connection.connectionScore /
NullUtil.get(sharedProps.pnd.get(target)).score;
expect(proportion).toBeGreaterThan(0.0);
expect(proportion).toBeLessThan(1.0);
const expectedText = (proportion * 100).toFixed(2) + "%";
expect(
element
.find("td")
.at(connectionColumn)
.text()
).toEqual(expectedText);
});
it("renders a score column with the source's score", async () => {
const {element, connection} = await setup();
const expectedScore = connection.sourceScore.toFixed(2);
const connectionColumn = COLUMNS().indexOf("Score");
expect(connectionColumn).not.toEqual(-1);
expect(
element
.find("td")
.at(connectionColumn)
.text()
).toEqual(expectedScore);
});
it("does not render children by default", async () => {
const {element} = await setup();
expect(element.find("ConnectionRowList")).toHaveLength(0);
});
it('has a working "expand" button', async () => {
const {element, depth, sharedProps, source} = await setup();
expect(element.find("button").text()).toEqual("+");
element.find("button").simulate("click");
expect(element.find("button").text()).toEqual("\u2212");
const crl = element.find("ConnectionRowList");
expect(crl).toHaveLength(1);
expect(crl).not.toHaveLength(0);
expect(crl.prop("sharedProps")).toEqual(sharedProps);
expect(crl.prop("depth")).toBe(depth + 1);
expect(crl.prop("node")).toBe(source);
element.find("button").simulate("click");
expect(element.find("button").text()).toEqual("+");
expect(element.find("ConnectionRowList")).toHaveLength(0);
});
});
describe("ConnectionView", () => {

View File

@ -5,13 +5,9 @@ import sortBy from "lodash.sortby";
import * as NullUtil from "../../../util/null";
import {type NodeAddressT} from "../../../core/graph";
import {TableRow} from "./TableRow";
import {
nodeDescription,
scoreDisplay,
type SharedProps,
type RowState,
} from "./shared";
import {nodeDescription, type SharedProps} from "./shared";
import {ConnectionRowList} from "./Connection";
@ -41,44 +37,21 @@ type NodeRowProps = {|
+sharedProps: SharedProps,
|};
export class NodeRow extends React.PureComponent<NodeRowProps, RowState> {
constructor() {
super();
this.state = {expanded: false};
}
export class NodeRow extends React.PureComponent<NodeRowProps> {
render() {
const {node, sharedProps} = this.props;
const {pnd, adapters} = sharedProps;
const {expanded} = this.state;
const {score} = NullUtil.get(pnd.get(node));
const description = <span>{nodeDescription(node, adapters)}</span>;
return (
<React.Fragment>
<tr key="self">
<td style={{display: "flex", alignItems: "flex-start"}}>
<button
style={{marginRight: 5}}
onClick={() => {
this.setState(({expanded}) => ({
expanded: !expanded,
}));
}}
>
{expanded ? "\u2212" : "+"}
</button>
<span>{nodeDescription(node, adapters)}</span>
</td>
<td style={{textAlign: "right"}}>{"—"}</td>
<td style={{textAlign: "right"}}>{scoreDisplay(score)}</td>
</tr>
{expanded && (
<ConnectionRowList
key="children"
depth={1}
node={node}
sharedProps={sharedProps}
/>
)}
</React.Fragment>
<TableRow
depth={0}
description={description}
connectionProportion={null}
score={score}
>
<ConnectionRowList depth={1} node={node} sharedProps={sharedProps} />
</TableRow>
);
}
}

View File

@ -4,10 +4,13 @@ import React from "react";
import {shallow} from "enzyme";
import sortBy from "lodash.sortby";
import * as NullUtil from "../../../util/null";
import {TableRow} from "./TableRow";
import {ConnectionRowList} from "./Connection";
import {type NodeAddressT, NodeAddress} from "../../../core/graph";
import {example, COLUMNS} from "./sharedTestUtils";
import {nodeDescription} from "./shared";
import {example} from "./sharedTestUtils";
import {NodeRowList, NodeRow} from "./Node";
require("../../testUtil").configureEnzyme();
@ -85,68 +88,51 @@ describe("app/credExplorer/pagerankTable/Node", () => {
const sharedProps = {adapters, pnd, maxEntriesPerList: 123};
const node = nodes.bar1;
const component = <NodeRow node={node} sharedProps={sharedProps} />;
const element = shallow(component);
return {element, node, sharedProps};
const row = shallow(component).find(TableRow);
return {row, node, sharedProps};
}
it("renders the right number of columns", async () => {
expect((await setup()).element.find("td")).toHaveLength(COLUMNS().length);
});
it("renders the node description", async () => {
const {element} = await setup();
const expectedDescription = 'bar: NodeAddress["bar","a","1"]';
const descriptionColumn = COLUMNS().indexOf("Description");
expect(descriptionColumn).not.toEqual(-1);
expect(
element
.find("td")
.at(descriptionColumn)
.find("span")
.text()
).toEqual(expectedDescription);
});
it("renders an empty connection column", async () => {
const {element} = await setup();
const connectionColumn = COLUMNS().indexOf("Connection");
expect(connectionColumn).not.toEqual(-1);
expect(
element
.find("td")
.at(connectionColumn)
.text()
).toEqual("—");
});
it("renders a score column with the node's score", async () => {
const {element, sharedProps, node} = await setup();
const {score} = NullUtil.get(sharedProps.pnd.get(node));
const expectedScore = score.toFixed(2);
const connectionColumn = COLUMNS().indexOf("Score");
expect(connectionColumn).not.toEqual(-1);
expect(
element
.find("td")
.at(connectionColumn)
.text()
).toEqual(expectedScore);
});
it("does not render children by default", async () => {
const {element} = await setup();
expect(element.find("ConnectionRowList")).toHaveLength(0);
});
it('has a working "expand" button', async () => {
const {element, sharedProps, node} = await setup();
expect(element.find("button").text()).toEqual("+");
element.find("button").simulate("click");
expect(element.find("button").text()).toEqual("\u2212");
const crl = element.find("ConnectionRowList");
expect(crl).toHaveLength(1);
expect(crl.prop("sharedProps")).toEqual(sharedProps);
expect(crl.prop("depth")).toBe(1);
expect(crl.prop("node")).toBe(node);
element.find("button").simulate("click");
expect(element.find("button").text()).toEqual("+");
expect(element.find("ConnectionRowList")).toHaveLength(0);
describe("instantiates a TableRow", () => {
it("with the correct depth", async () => {
const {row} = await setup();
expect(row.props().depth).toBe(0);
});
it("with the node's score", async () => {
const {row, node, sharedProps} = await setup();
const score = NullUtil.get(sharedProps.pnd.get(node)).score;
expect(row.props().score).toBe(score);
});
it("with no connectionProportion", async () => {
const {row} = await setup();
expect(row.props().connectionProportion).not.toEqual(expect.anything());
});
it("with the node description", async () => {
const {row, sharedProps, node} = await setup();
const {adapters} = sharedProps;
const description = shallow(row.props().description);
expect(description.text()).toEqual(nodeDescription(node, adapters));
});
describe("with a ConnectionRowList as children", () => {
function getChildren(row) {
const children = row.props().children;
return shallow(children).instance();
}
it("which is a ConnectionRowList", async () => {
const {row} = await setup();
expect(getChildren(row)).toBeInstanceOf(ConnectionRowList);
});
it("which has depth=1", async () => {
const {row} = await setup();
expect(getChildren(row).props.depth).toBe(1);
});
it("which has the node as its node", async () => {
const {row, node} = await setup();
expect(getChildren(row).props.node).toBe(node);
});
it("which has the right sharedProps", async () => {
const {row, sharedProps} = await setup();
expect(getChildren(row).props.sharedProps).toBe(sharedProps);
});
});
});
});
});

View File

@ -0,0 +1,74 @@
// @flow
import React, {type Node as ReactNode} from "react";
type TableRowProps = {|
+depth: number,
// The node that goes in the Description column
+description: ReactNode,
// What proportion should be formatted in the connection column
+connectionProportion: ?number,
// The score to format and display
+score: number,
// Children to show when the row is expanded
+children: ReactNode,
|};
type TableRowState = {|
expanded: boolean,
|};
export function scoreDisplay(score: number) {
return score.toFixed(2);
}
export class TableRow extends React.PureComponent<
TableRowProps,
TableRowState
> {
constructor() {
super();
this.state = {expanded: false};
}
render() {
const {
depth,
description,
connectionProportion,
score,
children,
} = this.props;
const {expanded} = this.state;
const percent =
connectionProportion == null
? ""
: (connectionProportion * 100).toFixed(2) + "%";
return (
<React.Fragment>
<tr
key="self"
style={{backgroundColor: `rgba(0,143.4375,0,${1 - 0.9 ** depth})`}}
>
<td style={{display: "flex", alignItems: "flex-start"}}>
<button
style={{
marginRight: 5,
marginLeft: 15 * depth,
}}
onClick={() => {
this.setState(({expanded}) => ({
expanded: !expanded,
}));
}}
>
{expanded ? "\u2212" : "+"}
</button>
{description}
</td>
<td style={{textAlign: "right"}}>{percent}</td>
<td style={{textAlign: "right"}}>{scoreDisplay(score)}</td>
</tr>
{expanded && children}
</React.Fragment>
);
}
}

View File

@ -0,0 +1,102 @@
// @flow
import React from "react";
import {shallow} from "enzyme";
import {TableRow} from "./TableRow";
import {COLUMNS} from "./sharedTestUtils";
require("../../testUtil").configureEnzyme();
describe("app/credExplorer/pagerankTable/TableRow", () => {
function example(depth: number = 3) {
return shallow(
<TableRow
depth={depth}
description={<span data-test-description={true} />}
connectionProportion={0.5}
score={133.7}
children={<div data-test-children={true} />}
/>
);
}
it("has depth-based color", () => {
for (const depth of [0, 1, 2]) {
const el = example(depth);
const style = el.find("tr").props().style;
expect({depth, style}).toMatchSnapshot();
}
});
it("has depth-based indentation", () => {
for (const depth of [0, 1, 2]) {
const el = example(depth);
const style = el
.find("td")
.at(0)
.find("button")
.props().style;
expect({depth, style}).toMatchSnapshot();
}
});
it("expand button toggles symbol based on expansion state", () => {
const el = example();
el.setState({expanded: false});
expect(el.find("button").text()).toEqual("+");
el.setState({expanded: true});
expect(el.find("button").text()).toEqual("\u2212");
});
it("clicking the expand button toggles expansion state", () => {
const el = example();
el.setState({expanded: false});
el.find("button").simulate("click");
expect(el.state().expanded).toBe(true);
el.find("button").simulate("click");
expect(el.state().expanded).toBe(false);
});
it("defaults to not expanded", () => {
const el = example();
expect(el.state().expanded).toBe(false);
});
it("displays children only when expanded", () => {
const el = example();
el.setState({expanded: false});
expect(el.find({"data-test-children": true})).toHaveLength(0);
el.setState({expanded: true});
expect(el.find({"data-test-children": true})).toHaveLength(1);
});
it("has the correct number of columns", () => {
const el = example();
expect(el.find("td")).toHaveLength(COLUMNS().length);
});
it("displays formatted connectionPercentage in the correct column", () => {
const index = COLUMNS().indexOf("Connection");
expect(index).not.toEqual(-1);
const td = example()
.find("td")
.at(index);
expect(td.text()).toEqual("50.00%");
});
it("displays empty column when connectionProportion not set", () => {
const index = COLUMNS().indexOf("Connection");
expect(index).not.toEqual(-1);
const el = example();
el.setProps({connectionProportion: null});
const td = el.find("td").at(index);
expect(td.text()).toEqual("");
});
it("displays formatted score in the correct column", () => {
const index = COLUMNS().indexOf("Score");
expect(index).not.toEqual(-1);
const td = example()
.find("td")
.at(index);
expect(td.text()).toEqual("133.70");
});
it("displays the description in the correct column", () => {
const index = COLUMNS().indexOf("Description");
expect(index).not.toEqual(-1);
const td = example()
.find("td")
.at(index);
expect(td.find({"data-test-description": true})).toHaveLength(1);
});
});

View File

@ -1,13 +0,0 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`app/credExplorer/pagerankTable/Connection ConnectionRow has proper depth-based styling 1`] = `
Object {
"buttonStyle": Object {
"marginLeft": 30,
"marginRight": 5,
},
"trStyle": Object {
"backgroundColor": "rgba(0,143.4375,0,0.18999999999999995)",
},
}
`;

View File

@ -0,0 +1,58 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`app/credExplorer/pagerankTable/TableRow has depth-based color 1`] = `
Object {
"depth": 0,
"style": Object {
"backgroundColor": "rgba(0,143.4375,0,0)",
},
}
`;
exports[`app/credExplorer/pagerankTable/TableRow has depth-based color 2`] = `
Object {
"depth": 1,
"style": Object {
"backgroundColor": "rgba(0,143.4375,0,0.09999999999999998)",
},
}
`;
exports[`app/credExplorer/pagerankTable/TableRow has depth-based color 3`] = `
Object {
"depth": 2,
"style": Object {
"backgroundColor": "rgba(0,143.4375,0,0.18999999999999995)",
},
}
`;
exports[`app/credExplorer/pagerankTable/TableRow has depth-based indentation 1`] = `
Object {
"depth": 0,
"style": Object {
"marginLeft": 0,
"marginRight": 5,
},
}
`;
exports[`app/credExplorer/pagerankTable/TableRow has depth-based indentation 2`] = `
Object {
"depth": 1,
"style": Object {
"marginLeft": 15,
"marginRight": 5,
},
}
`;
exports[`app/credExplorer/pagerankTable/TableRow has depth-based indentation 3`] = `
Object {
"depth": 2,
"style": Object {
"marginLeft": 30,
"marginRight": 5,
},
}
`;

View File

@ -33,16 +33,8 @@ export function edgeVerb(
return direction === "FORWARD" ? edgeType.forwardName : edgeType.backwardName;
}
export function scoreDisplay(score: number) {
return score.toFixed(2);
}
export type SharedProps = {|
+pnd: PagerankNodeDecomposition,
+adapters: DynamicAdapterSet,
+maxEntriesPerList: number,
|};
export type RowState = {|
expanded: boolean,
|};