From beeebc95e3884306f77b4f7741c559a2ce18cc63 Mon Sep 17 00:00:00 2001 From: Daniel Caldas <11733994+danielcaldas@users.noreply.github.com> Date: Sat, 11 Nov 2017 20:12:01 +0000 Subject: [PATCH] Refactor/improve code structure (#35) * Move validateGraphData to Graph/helper.jsx * Improve docs for validateGraphData * Make node symbols constants shared accross components * Move initializeGraphState to Graph/helper * Update docs. Use destructuring for method initializeGraphState * Update graph/config docs * Small :bug: fix * Use lowercase naming for components * Add docs lint task in check npm script * tmp directories * Revert tmp directories --- package.json | 2 +- sandbox/Sandbox.jsx | 2 +- src/components/{Graph => graph}/config.js | 18 ++--- src/components/{Graph => graph}/const.js | 12 +-- src/components/{Graph => graph}/helper.jsx | 89 +++++++++++++++++++++- src/components/{Graph => graph}/index.jsx | 72 +---------------- src/components/{Link => link}/index.jsx | 0 src/components/node/const.js | 13 ++++ src/components/{Node => node}/helper.js | 0 src/components/{Node => node}/index.jsx | 0 src/{components/Node => }/const.js | 12 ++- src/index.js | 6 +- test/Graph.test.js | 2 +- test/Link.test.js | 2 +- test/Node.test.js | 2 +- 15 files changed, 128 insertions(+), 104 deletions(-) rename src/components/{Graph => graph}/config.js (92%) rename src/components/{Graph => graph}/const.js (59%) rename src/components/{Graph => graph}/helper.jsx (81%) rename src/components/{Graph => graph}/index.jsx (80%) rename src/components/{Link => link}/index.jsx (100%) create mode 100644 src/components/node/const.js rename src/components/{Node => node}/helper.js (100%) rename src/components/{Node => node}/index.jsx (100%) rename src/{components/Node => }/const.js (50%) diff --git a/package.json b/package.json index ba5e599d8..05dd7c37a 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "author": "Daniel Caldas", "license": "MIT", "scripts": { - "check": "npm run lint && npm run test", + "check": "npm run docs:lint && npm run lint && npm run test", "dev": "node_modules/.bin/webpack-dev-server -d --content-base sandbox --inline --hot --port 3002", "dist": "npm run check && node_modules/.bin/npm-run-all --parallel dist:*", "dist:rd3g": "rm -rf dist/ && webpack --config webpack.config.dist.js -p --display-modules", diff --git a/sandbox/Sandbox.jsx b/sandbox/Sandbox.jsx index 5d8cdd256..7ec779e53 100644 --- a/sandbox/Sandbox.jsx +++ b/sandbox/Sandbox.jsx @@ -4,7 +4,7 @@ import Form from 'react-jsonschema-form'; import './styles.css'; -import defaultConfig from '../src/components/Graph/config'; +import defaultConfig from '../src/components/graph/config'; import { Graph } from '../src'; import data from './data'; import Utils from './utils'; diff --git a/src/components/Graph/config.js b/src/components/graph/config.js similarity index 92% rename from src/components/Graph/config.js rename to src/components/graph/config.js index 66cc37be9..e9c4abc7b 100644 --- a/src/components/Graph/config.js +++ b/src/components/graph/config.js @@ -37,7 +37,7 @@ * highlighted. If the value is set to **1** the selected node and his 1st degree connections will be highlighted. If * the value is set to **2** the selected node will be highlighted as well as the 1st and 2nd common degree connections. * @param {number} [highlightOpacity=1] - this value is used to highlight nodes in the network. The lower - * the value the more the less highlighted nodes will be visible (related to **highlightBehavior**). + * the value the more the less highlighted nodes will be visible (related to *highlightBehavior*). * @param {number} [maxZoom=8] - max zoom that can be performed against the graph. * @param {number} [minZoom=0.1] - min zoom that can be performed against the graph. * @param {boolean} [panAndZoom=false] - 🚅🚅🚅 pan and zoom effect when performing zoom in the graph, @@ -48,7 +48,7 @@ * from the given nodes positions by rd3g), no coordinates will be calculated by rd3g or subjacent d3 modules. * @param {number} [width=800] - the width of the (svg) area where the graph will be rendered. *
- * @param {Object} node node object is explained in next section. + * @param {Object} node node object is explained in next section. ⬇️ *

Node level configurations

* @param {string} [node.color='#d3d3d3'] - 🔍🔍🔍 this is the color that will be applied to the node if no **color property** * is found inside the node itself (yes **you can pass a property 'color' inside the node and that color will override the @@ -78,15 +78,15 @@ * - 'triangle' * - 'wye' * - * **[note]** react-d3-graph will map this values to d3 symbols ({@link https://github.com/d3/d3-shape#symbols}) + * **[note]** react-d3-graph will map this values to [d3 symbols](https://github.com/d3/d3-shape#symbols) * @param {string} [node.highlightColor='SAME'] - color for all highlighted nodes (use string 'SAME' if you * want the node to keep its color in highlighted state). - * @param {number} [node.highlightFontSize=10] - node.fontSize in highlighted state. - * @param {string} [node.highlightFontWeight='normal'] - node.fontWeight in highlighted state. - * @param {string} [node.highlightStrokeColor='SAME'] - node.strokeColor in highlighted state. - * @param {number} [node.highlightStrokeWidth=1.5] - node.strokeWidth in highlighted state. + * @param {number} [node.highlightFontSize=10] - fontSize in highlighted state. + * @param {string} [node.highlightFontWeight='normal'] - fontWeight in highlighted state. + * @param {string} [node.highlightStrokeColor='SAME'] - strokeColor in highlighted state. + * @param {number} [node.highlightStrokeWidth=1.5] - strokeWidth in highlighted state. *
- * @param {Object} link link object is explained in the next section. + * @param {Object} link link object is explained in the next section. ⬇️ *

Link level configurations

* @param {string} [link.color='#d3d3d3'] - the color for links. * @param {number} [link.opacity=1] - the default opacity value for links. @@ -97,7 +97,7 @@ * strokeWidth += (linkValue * strokeWidth) / 10; * ``` * @param {number} [link.strokeWidth=1.5] - strokeWidth for all links. - * @param {string} [link.highlightColor='#d3d3d3'] - links color in highlight state. + * @param {string} [link.highlightColor='#d3d3d3'] - links' color in highlight state. * * @example * // A simple config that uses some properties diff --git a/src/components/Graph/const.js b/src/components/graph/const.js similarity index 59% rename from src/components/Graph/const.js rename to src/components/graph/const.js index 9ba3dcafc..e505cacfb 100644 --- a/src/components/Graph/const.js +++ b/src/components/graph/const.js @@ -1,3 +1,5 @@ +import CONST from '../../const'; + export default { COORDS_SEPARATOR: ',', FORCE_IDEAL_STRENGTH: -100, // @TODO: Expose as configurable, @@ -10,13 +12,5 @@ export default { }, LINK_CLASS_NAME: 'link', NODE_CLASS_NAME: 'node', - SYMBOLS: { // FIXME: Repeated SYMBOLS constant - CIRCLE: 'circle', - CROSS: 'cross', - DIAMOND: 'diamond', - SQUARE: 'square', - STAR: 'star', - TRIANGLE: 'triangle', - WYE: 'wye' - } + ...CONST }; diff --git a/src/components/Graph/helper.jsx b/src/components/graph/helper.jsx similarity index 81% rename from src/components/Graph/helper.jsx rename to src/components/graph/helper.jsx index ba5a60db8..dab29a02d 100644 --- a/src/components/Graph/helper.jsx +++ b/src/components/graph/helper.jsx @@ -3,6 +3,17 @@ * @description * Offers a series of methods that isolate logic of Graph component. */ +/** + * @typedef {Object} Link + * @property {string} source - the node id of the source in the link. + * @property {string} target - the node id of the target in the link. + * @memberof Graph/helper + */ +/** + * @typedef {Object} Node + * @property {string} id - the id of the node. + * @memberof Graph/helper + */ import React from 'react'; import { @@ -13,9 +24,12 @@ import { } from 'd3-force'; import CONST from './const'; +import DEFAULT_CONFIG from './config'; +import ERRORS from '../../err'; -import Link from '../Link/'; -import Node from '../Node/'; +import Link from '../link/'; +import Node from '../node/'; +import utils from '../../utils'; /** * Build some Link properties based on given parameters. @@ -285,6 +299,59 @@ function createForceSimulation(width, height) { .force('y', fry); } +/** + * Incapsulates common procedures to initialize graph. + * @param {Object} props - Graph component props, object that holds data, id and config. + * @param {Object} props.data - Data object holds links (array of **Link**) and nodes (array of **Node**). + * @param {string} props.id - the graph id. + * @param {Object} props.config - same as {@link #buildGraph|config in buildGraph}. + * @param {Object} state - Graph component current state (same format as returned object on this function). + * @returns a fully (re)initialized graph state object. + * @memberof Graph/helper + */ +function initializeGraphState({data, id, config}, state) { + let graph; + + validateGraphData(data); + + if (state && state.nodes && state.links && state.nodeIndexMapping) { + // absorve existent positining + graph = { + nodes: data.nodes.map(n => Object.assign({}, n, state.nodes[n.id])), + links: {} + }; + } else { + graph = { + nodes: data.nodes.map(n => Object.assign({}, n)), + links: {} + }; + } + + graph.links = data.links.map(l => Object.assign({}, l)); + + let newConfig = Object.assign({}, utils.merge(DEFAULT_CONFIG, config || {})); + let {nodes, nodeIndexMapping} = initializeNodes(graph.nodes); + let links = initializeLinks(graph.links); // Matrix of graph connections + const {nodes: d3Nodes, links: d3Links} = graph; + const formatedId = id.replace(/ /g, '_'); + const simulation = createForceSimulation(newConfig.width, newConfig.height); + + return { + id: formatedId, + config: newConfig, + nodeIndexMapping, + links, + d3Links, + nodes, + d3Nodes, + highlightedNode: '', + simulation, + newGraphElements: false, + configUpdated: false, + transform: 1 + }; +} + /** * Receives a matrix of the graph with the links source and target as concrete node instances and it transforms it * in a lightweight matrix containing only links with source and target being strings representative of some node id @@ -345,9 +412,27 @@ function initializeNodes(graphNodes) { return { nodes, nodeIndexMapping }; } +/** + * Some integraty validations on links and nodes structure. If some validation fails the function will + * throw an error. + * @param {Object} data - Same as {@link #initializeGraphState|data in initializeGraphState}. + * @memberof Graph/helper + */ +function validateGraphData(data) { + data.links.forEach(l => { + if (!data.nodes.find(n => n.id === l.source)) { + utils.throwErr(this.constructor.name, `${ERRORS.INVALID_LINKS} - ${l.source} is not a valid node id`); + } + if (!data.nodes.find(n => n.id === l.target)) { + utils.throwErr(this.constructor.name, `${ERRORS.INVALID_LINKS} - ${l.target} is not a valid node id`); + } + }); +} + export default { buildGraph, createForceSimulation, + initializeGraphState, initializeLinks, initializeNodes }; diff --git a/src/components/Graph/index.jsx b/src/components/graph/index.jsx similarity index 80% rename from src/components/Graph/index.jsx rename to src/components/graph/index.jsx index 7b3f7f0fc..835d999b1 100644 --- a/src/components/Graph/index.jsx +++ b/src/components/graph/index.jsx @@ -190,7 +190,7 @@ export default class Graph extends React.Component { /** * This method resets all nodes fixed positions by deleting the properties fx (fixed x) - * and fy (fixed y). Next a simulation is triggered in order to force nodes to go back + * and fy (fixed y). Following this, a simulation is triggered in order to force nodes to go back * to their original positions (or at least new positions according to the d3 force parameters). */ resetNodesPositions = () => { @@ -216,72 +216,6 @@ export default class Graph extends React.Component { */ restartSimulation = () => !this.state.config.staticGraph && this.state.simulation.restart(); - /** - * Some integraty validations on links and nodes structure. - * @param {Object} data - */ - _validateGraphData(data) { - // @TODO: Move function to helper.jsx - data.links.forEach(l => { - if (!data.nodes.find(n => n.id === l.source)) { - utils.throwErr(this.constructor.name, `${ERRORS.INVALID_LINKS} - ${l.source} is not a valid node id`); - } - if (!data.nodes.find(n => n.id === l.target)) { - utils.throwErr(this.constructor.name, `${ERRORS.INVALID_LINKS} - ${l.target} is not a valid node id`); - } - }); - } - - /** - * Incapsulates common procedures to initialize graph. - * @param {Object} data - * @param {Array.} data.nodes - nodes of the graph to be created. - * @param {Array.} data.links - links that connect data.nodes. - * @returns {Object} - */ - _initializeGraphState(data) { - let graph; - - this._validateGraphData(data); - - if (this.state && this.state.nodes && this.state.links && this.state.nodeIndexMapping) { - // absorve existent positining - graph = { - nodes: data.nodes.map(n => Object.assign({}, n, this.state.nodes[n.id])), - links: {} - }; - } else { - graph = { - nodes: data.nodes.map(n => Object.assign({}, n)), - links: {} - }; - } - - graph.links = data.links.map(l => Object.assign({}, l)); - - let config = Object.assign({}, utils.merge(DEFAULT_CONFIG, this.props.config || {})); - let {nodes, nodeIndexMapping} = graphHelper.initializeNodes(graph.nodes); - let links = graphHelper.initializeLinks(graph.links); // Matrix of graph connections - const {nodes: d3Nodes, links: d3Links} = graph; - const id = this.props.id.replace(/ /g, '_'); - const simulation = graphHelper.createForceSimulation(config.width, config.height); - - return { - id, - config, - nodeIndexMapping, - links, - d3Links, - nodes, - d3Nodes, - highlightedNode: '', - simulation, - newGraphElements: false, - configUpdated: false, - transform: 1 - }; - } - /** * Sets d3 tick function and configures other d3 stuff such as forces and drag events. */ @@ -310,7 +244,7 @@ export default class Graph extends React.Component { utils.throwErr(this.constructor.name, ERRORS.GRAPH_NO_ID_PROP); } - this.state = this._initializeGraphState(this.props.data); + this.state = graphHelper.initializeGraphState(this.props, this.state); } componentWillReceiveProps(nextProps) { @@ -322,7 +256,7 @@ export default class Graph extends React.Component { } const configUpdated = !utils.isDeepEqual(nextProps.config, this.state.config); - const state = newGraphElements ? this._initializeGraphState(nextProps.data) : this.state; + const state = newGraphElements ? graphHelper.initializeGraphState(nextProps, this.state) : this.state; const config = configUpdated ? utils.merge(DEFAULT_CONFIG, nextProps.config || {}) : this.state.config; // In order to properly update graph data we need to pause eventual d3 ongoing animations diff --git a/src/components/Link/index.jsx b/src/components/link/index.jsx similarity index 100% rename from src/components/Link/index.jsx rename to src/components/link/index.jsx diff --git a/src/components/node/const.js b/src/components/node/const.js new file mode 100644 index 000000000..1f6a9cc10 --- /dev/null +++ b/src/components/node/const.js @@ -0,0 +1,13 @@ +import CONFIG from '../graph/config'; +import CONST from '../../const'; + +export default { + ARC: { + START_ANGLE: 0, + END_ANGLE: 2 * Math.PI + }, + DEFAULT_NODE_SIZE: CONFIG.node.size, + NODE_LABEL_DX: '.90em', + NODE_LABEL_DY: '.35em', + ...CONST +}; diff --git a/src/components/Node/helper.js b/src/components/node/helper.js similarity index 100% rename from src/components/Node/helper.js rename to src/components/node/helper.js diff --git a/src/components/Node/index.jsx b/src/components/node/index.jsx similarity index 100% rename from src/components/Node/index.jsx rename to src/components/node/index.jsx diff --git a/src/components/Node/const.js b/src/const.js similarity index 50% rename from src/components/Node/const.js rename to src/const.js index 1f593ef6c..e8e42d02c 100644 --- a/src/components/Node/const.js +++ b/src/const.js @@ -1,11 +1,9 @@ +/** + * @ignore + * These are common keywords used across rd3g, thus being placed in a more abstract level + * in the tree directory. + */ export default { - ARC: { - START_ANGLE: 0, - END_ANGLE: 2 * Math.PI - }, - DEFAULT_NODE_SIZE: 80, // FIXME: This value should come from ../Graph/config.js - NODE_LABEL_DX: '.90em', - NODE_LABEL_DY: '.35em', SYMBOLS: { CIRCLE: 'circle', CROSS: 'cross', diff --git a/src/index.js b/src/index.js index 05fc96d24..ff7adc381 100644 --- a/src/index.js +++ b/src/index.js @@ -1,6 +1,6 @@ -import Graph from './components/Graph'; -import Node from './components/Node'; -import Link from './components/Link'; +import Graph from './components/graph'; +import Node from './components/node'; +import Link from './components/link'; export { Graph, diff --git a/test/Graph.test.js b/test/Graph.test.js index 15c1b98c2..02b82ce83 100644 --- a/test/Graph.test.js +++ b/test/Graph.test.js @@ -1,7 +1,7 @@ import React from 'react'; import renderer from 'react-test-renderer'; -import Graph from '../src/components/Graph'; +import Graph from '../src/components/graph'; import graphMock from './graph.mock.js'; describe('Graph Component', () => { diff --git a/test/Link.test.js b/test/Link.test.js index 559b930ba..1803e8e3c 100644 --- a/test/Link.test.js +++ b/test/Link.test.js @@ -1,7 +1,7 @@ import React from 'react'; import renderer from 'react-test-renderer'; -import Link from '../src/components/Link'; +import Link from '../src/components/link'; describe('Link Component', () => { let that = {}; diff --git a/test/Node.test.js b/test/Node.test.js index 71f258e04..58ef0a170 100644 --- a/test/Node.test.js +++ b/test/Node.test.js @@ -1,7 +1,7 @@ import React from 'react'; import renderer from 'react-test-renderer'; -import Node from '../src/components/Node'; +import Node from '../src/components/node'; describe('Node Component', () => { let that = {};