status-im
/
sourcecred
mirror of https://github.com/status-im/sourcecred.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Change the PagerankTable component cycle (#657) 

Currently, the PagerankTable creates components in the following
pattern:

```
NodeRow (depth=0)
 > ConnectionRowList (depth=1)
  > ConnectionRow (depth=2)
   > ConnectionRowList (depth=3)
```

This commit changes the cycle to the following:

```
NodeRow  (depth=0)
 > ConnectionRowList (depth=0)
  > ConnectionRow (depth=0)

NodeRow (padding=true, depth=1)
  > ConnectionRowList (depth=1)
    > ConnectionRow (depth=1)
```

This has some nice properties:

First, the context visually resets every time we return to a NodeRow, which
makes it feasible to change the score column to always have a context
dependent meaning:
- for a node row, the score is the score of that node.
- for a connection row, the score is the score contribution of that
connection
- (as of #502): for an aggregation row, the score is the score
contribution of that aggregation

We think this will be visually clear thanks to the padding around the
new NodeRow, along with the new color block indicating a new scope.

This design also ensures that every NodeRow has the full width available
to it (rather than getting crushed into a progressively smaller area of
the table), which will be very convenient for when we add renderers for
the nodes.

Thanks to @theliamcrawford as the idea for this came during a user study
with him.

Test plan:
The updated unit tests should be comprehensive. Also, try expanding some
rows in the cred explorer and verify that the behavior is as described.
This commit is contained in:
Dandelion Mané 2018-08-13 21:33:12 -07:00 committed by GitHub
parent 83969371d6
commit bf65860178
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 64 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/app/credExplorer/pagerankTable/Connection.js
View File

@ -8,6 +8,7 @@ import type {Connection} from "../../../core/attribution/graphToMarkovChain";
import type {ScoredConnection} from "../../../core/attribution/pagerankNodeDecomposition";
import {DynamicAdapterSet} from "../../adapters/adapterSet";
import {TableRow} from "./TableRow";
import {NodeRow} from "./Node";
import {edgeVerb, nodeDescription, type SharedProps} from "./shared";
@ -66,15 +67,16 @@ export class ConnectionRow extends React.PureComponent<ConnectionRowProps> {
);
return (
<TableRow
indent={depth}
indent={1}
depth={depth}
description={connectionView}
connectionProportion={connectionProportion}
showPadding={false}
score={sourceScore}
>
<ConnectionRowList
<NodeRow
depth={depth + 1}
showPadding={true}
node={source}
sharedProps={sharedProps}
/>

17
src/app/credExplorer/pagerankTable/Connection.test.js
View File

@ -8,6 +8,7 @@ import type {Connection} from "../../../core/attribution/graphToMarkovChain";
import {ConnectionRowList, ConnectionRow, ConnectionView} from "./Connection";
import {example} from "./sharedTestUtils";
import {TableRow} from "./TableRow";
import {NodeRow} from "./Node";
require("../../testUtil").configureEnzyme();
@ -98,6 +99,14 @@ describe("app/credExplorer/pagerankTable/Connection", () => {
const {row, depth} = await setup();
expect(row.props().depth).toBe(depth);
});
it("with indent=1", async () => {
const {row} = await setup();
expect(row.props().indent).toBe(1);
});
it("with showPadding=false", async () => {
const {row} = await setup();
expect(row.props().showPadding).toBe(false);
});
it("with the sourceScore", async () => {
const {row, scoredConnection} = await setup();
expect(row.props().score).toBe(scoredConnection.sourceScore);
@ -118,19 +127,23 @@ describe("app/credExplorer/pagerankTable/Connection", () => {
expect(cv.props.connection).toEqual(scoredConnection.connection);
expect(cv.props.adapters).toEqual(adapters);
});
describe("with a ConnectionRowList as children", () => {
describe("with a NodeRow 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);
expect(getChildren(row)).toBeInstanceOf(NodeRow);
});
it("which has incremented depth", async () => {
const {row, depth} = await setup();
expect(getChildren(row).props.depth).toBe(depth + 1);
});
it("which has padding", async () => {
const {row} = await setup();
expect(getChildren(row).props.showPadding).toBe(true);
});
it("which has the connection source as its node target", async () => {
const {row, scoredConnection} = await setup();
expect(getChildren(row).props.node).toBe(scoredConnection.source);

24
src/app/credExplorer/pagerankTable/Node.js
View File

@ -25,34 +25,46 @@ export class NodeRowList extends React.PureComponent<NodeRowListProps> {
{sortBy(nodes, (n) => -NullUtil.get(pnd.get(n)).score, (n) => n)
.slice(0, maxEntriesPerList)
.map((node) => (
<NodeRow node={node} key={node} sharedProps={sharedProps} />
<NodeRow
showPadding={false}
depth={0}
node={node}
key={node}
sharedProps={sharedProps}
/>
))}
</React.Fragment>
);
}
}
type NodeRowProps = {|
export type NodeRowProps = {|
+depth: number,
+node: NodeAddressT,
+sharedProps: SharedProps,
+showPadding: boolean,
|};
export class NodeRow extends React.PureComponent<NodeRowProps> {
render() {
const {node, sharedProps} = this.props;
const {depth, node, sharedProps, showPadding} = this.props;
const {pnd, adapters} = sharedProps;
const {score} = NullUtil.get(pnd.get(node));
const description = <span>{nodeDescription(node, adapters)}</span>;
return (
<TableRow
depth={0}
depth={depth}
indent={0}
showPadding={showPadding}
description={description}
connectionProportion={null}
score={score}
showPadding={false}
>
<ConnectionRowList depth={1} node={node} sharedProps={sharedProps} />
<ConnectionRowList
depth={depth}
node={node}
sharedProps={sharedProps}
/>
</TableRow>
);
}

36
src/app/credExplorer/pagerankTable/Node.test.js
View File

@ -11,7 +11,7 @@ import {type NodeAddressT, NodeAddress} from "../../../core/graph";
import {nodeDescription} from "./shared";
import {example} from "./sharedTestUtils";
import {NodeRowList, NodeRow} from "./Node";
import {NodeRowList, NodeRow, type NodeRowProps} from "./Node";
require("../../testUtil").configureEnzyme();
@ -55,6 +55,7 @@ describe("app/credExplorer/pagerankTable/Node", () => {
expect(rowNodes.slice().sort()).toEqual(nodes.slice().sort());
rows.forEach((row) => {
expect(row.prop("sharedProps")).toEqual(sharedProps);
expect(row.prop("depth")).toEqual(0);
});
});
it("creates up to `maxEntriesPerList` `NodeRow`s", async () => {
@ -83,18 +84,34 @@ describe("app/credExplorer/pagerankTable/Node", () => {
});
describe("NodeRow", () => {
async function setup() {
async function setup(props: $Shape<{...NodeRowProps}>) {
props = props || {};
const {pnd, adapters, nodes} = await example();
const sharedProps = {adapters, pnd, maxEntriesPerList: 123};
const node = nodes.bar1;
const component = <NodeRow node={node} sharedProps={sharedProps} />;
const row = shallow(component).find(TableRow);
const component = shallow(
<NodeRow
node={NullUtil.orElse(props.node, node)}
showPadding={NullUtil.orElse(props.showPadding, false)}
depth={NullUtil.orElse(props.depth, 0)}
sharedProps={NullUtil.orElse(props.sharedProps, sharedProps)}
/>
);
const row = component.find(TableRow);
return {row, node, sharedProps};
}
describe("instantiates a TableRow", () => {
it("with the correct depth", async () => {
const {row} = await setup();
expect(row.props().depth).toBe(0);
const {row: row0} = await setup({depth: 0});
expect(row0.props().depth).toBe(0);
const {row: row1} = await setup({depth: 1});
expect(row1.props().depth).toBe(1);
});
it("with the correct showPadding", async () => {
const {row: row0} = await setup({showPadding: true});
expect(row0.props().showPadding).toBe(true);
const {row: row1} = await setup({showPadding: false});
expect(row1.props().showPadding).toBe(false);
});
it("with the node's score", async () => {
const {row, node, sharedProps} = await setup();
@ -120,9 +137,10 @@ describe("app/credExplorer/pagerankTable/Node", () => {
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 same depth", async () => {
const {row} = await setup({depth: 13});
const crl = getChildren(row);
expect(crl.props.depth).toBe(13);
});
it("which has the node as its node", async () => {
const {row, node} = await setup();