Skip to content

Commit

Permalink
Misc refactor improvements (#231)
Browse files Browse the repository at this point in the history
* Misc improvements in functions of collapse.helper.js

* Remove some unncessary comments in utils.js

* Add some comments in graph.builder and misc refactoring

* Small misc improvements in graph.helper

* Remove redundant comment in marker.helper

* Add some comments on optional chaining usage

* Add ./vscode/settings.json

* Add babel plugin for optional chaining

* Use optional chaining where it makes sense

* Improve flakiness of e2e collapsible graph tests

* Fix bug with onNodePositionChange. It was not reseting the value of the draggedNode on the state

* Undo bad styles decision on sandbox

* Clear all timers in componentWillUnmont

* Improve id extraction from d3 links

* Improve exports. Not using default exports any more

* Update README Contributions section
  • Loading branch information
danielcaldas committed Sep 14, 2019
1 parent f556553 commit d4bb534
Show file tree
Hide file tree
Showing 20 changed files with 203 additions and 160 deletions.
3 changes: 2 additions & 1 deletion .babelrc
Expand Up @@ -2,6 +2,7 @@
"presets": ["@babel/preset-env", "@babel/preset-react"],
"plugins": [
"@babel/plugin-proposal-class-properties",
"@babel/plugin-proposal-object-rest-spread"
"@babel/plugin-proposal-object-rest-spread",
"@babel/plugin-proposal-optional-chaining",
]
}
4 changes: 2 additions & 2 deletions .eslintrc.js
Expand Up @@ -17,7 +17,7 @@ module.exports = {
jsx: true,
},
},
plugins: ["standard", "promise", "react", "jest", "cypress"],
plugins: ["standard", "promise", "react", "jest", "cypress", "babel"],
env: {
browser: true,
},
Expand All @@ -27,7 +27,7 @@ module.exports = {
camelcase: "error",
"keyword-spacing": "error",
"max-len": ["error", 120, 4, { ignoreComments: true }],
"max-lines": ["error", { max: 400, skipComments: true }],
"max-lines": ["error", { max: 450, skipComments: true }],
"newline-after-var": ["error", "always"],
"no-nested-ternary": "error",
"no-useless-constructor": "error",
Expand Down
3 changes: 3 additions & 0 deletions .vscode/settings.json
@@ -0,0 +1,3 @@
{
"javascript.validate.enable": false
}
10 changes: 9 additions & 1 deletion README.md
Expand Up @@ -148,7 +148,15 @@ const onNodePositionChange = function(nodeId, x, y) {

Contributions are welcome fell free to submit new ideas/features, just open an issue or send me an email or something. If you are more a _hands on_ person, just submit a pull request. Before jumping into coding, please take at the contribution guidelines [CONTRIBUTING.md](https://github.com/danielcaldas/react-d3-graph/blob/master/CONTRIBUTING.md).

To run react-d3-graph in development mode you just need to run `npm run dev` and the interactive sandbox will reload with the changes to the library code, that way you can test your changes not only through unit test but also through a real life example. It's that simple.
To run react-d3-graph in development mode you just need to run `npm run dev` and the interactive sandbox will reload with the changes to the library code, that way you can test your changes not only through unit test but also through a real life example. It's that simple. The development workflow usually should follow the steps:

- Create a branch prefixed with `fix/` for bug fixes, `feature/` for new features, `chore/` or `refactor/` for refactoring or tolling and CI/CD related tasks.
- Make sure you are up to date running `npm install`.
- Run `npm run dev`.
- Do you changes inside the folder `src` and the interactive sandbox consumes your changes in real time
with webpack-dev-server.
- You can run tests locally with `npm run test` (for unit tests) or `npm run functional:local` for e2e tests.
- After you're done open the Pull Request and describe the changes you've made.

## Alternatives (Not what you where looking for?)

Expand Down
16 changes: 16 additions & 0 deletions cypress/integration/graph.directed.e2e.js
Expand Up @@ -64,9 +64,15 @@ describe("[rd3g-graph-directed]", function() {
this.link34PO = new LinkPO(3);
});

afterEach(function() {
this.sandboxPO.exitFullScreenMode();
});

it("should behave correctly when directed is disabled after collapsed node", function() {
const toggledLine = this.link12PO.getLine();

this.sandboxPO.fullScreenMode().click();

// Check the leaf node & link is present
this.node2PO.getPath().should("be.visible");
toggledLine.should("be.visible");
Expand All @@ -87,10 +93,14 @@ describe("[rd3g-graph-directed]", function() {
this.link14PO.getLine().should("be.visible");
this.link34PO.getLine().should("be.visible");

this.sandboxPO.exitFullScreenMode();

// Disable "directed"
cy.contains("directed").scrollIntoView();
this.sandboxPO.getFieldInput("directed").click();

this.sandboxPO.fullScreenMode().click();

// Check if other nodes and links are still visible
this.node1PO.getPath().should("be.visible");
this.node2PO.getPath().should("be.visible");
Expand All @@ -104,6 +114,8 @@ describe("[rd3g-graph-directed]", function() {
});

it("should behave correctly when collapsible is disabled after collapsible node", function() {
this.sandboxPO.fullScreenMode().click();

const toggledLine = this.link12PO.getLine();

// Check the leaf node & link is present
Expand All @@ -126,10 +138,14 @@ describe("[rd3g-graph-directed]", function() {
this.link14PO.getLine().should("be.visible");
this.link34PO.getLine().should("be.visible");

this.sandboxPO.exitFullScreenMode();

// Disable "collapsible"
cy.contains("collapsible").scrollIntoView();
this.sandboxPO.getFieldInput("collapsible").click();

this.sandboxPO.fullScreenMode().click();

// The previously hidden node should reappear
this.node2PO.getPath().should("be.visible");
toggledLine.should("be.visible");
Expand Down
6 changes: 6 additions & 0 deletions cypress/integration/graph.e2e.js
Expand Up @@ -163,6 +163,12 @@ describe("[rd3g-graph] graph tests", function() {
this.node3PO = new NodePO(3);
this.node4PO = new NodePO(4);
this.link12PO = new LinkPO(0);

this.sandboxPO.fullScreenMode().click();
});

afterEach(function() {
this.sandboxPO.exitFullScreenMode();
});

it("should collapse leaf nodes", function() {
Expand Down
2 changes: 2 additions & 0 deletions package.json
Expand Up @@ -41,6 +41,7 @@
"devDependencies": {
"@babel/core": "7.6.0",
"@babel/plugin-proposal-class-properties": "7.5.5",
"@babel/plugin-proposal-optional-chaining": "7.6.0",
"@babel/preset-env": "7.6.0",
"@babel/preset-react": "7.0.0",
"@cypress/webpack-preprocessor": "4.1.0",
Expand All @@ -52,6 +53,7 @@
"documentation": "12.1.2",
"eslint": "6.3.0",
"eslint-config-recommended": "4.0.0",
"eslint-plugin-babel": "5.3.0",
"eslint-plugin-cypress": "2.6.1",
"eslint-plugin-jest": "22.17.0",
"eslint-plugin-prettier": "3.1.0",
Expand Down
16 changes: 8 additions & 8 deletions sandbox/Sandbox.jsx
Expand Up @@ -7,11 +7,11 @@ import "./styles.css";

import defaultConfig from "../src/components/graph/graph.config";
import { Graph } from "../src";
import utils from "./utils";
import reactD3GraphUtils from "../src/utils";
import { generateFormSchema, loadDataset, setValue } from "./utils";
import { isDeepEqual, merge } from "../src/utils";
import { JsonTree } from "react-editable-json-tree";

const sandboxData = utils.loadDataset();
const sandboxData = loadDataset();

/**
* This is a sample integration of react-d3-graph, in this particular case all the rd3g config properties
Expand All @@ -27,7 +27,7 @@ export default class Sandbox extends React.Component {

const { config: configOverride, data, fullscreen } = sandboxData;
const config = Object.assign(defaultConfig, configOverride);
const schemaProps = utils.generateFormSchema(config, "", {});
const schemaProps = generateFormSchema(config, "", {});

const schema = {
type: "object",
Expand Down Expand Up @@ -179,7 +179,7 @@ export default class Sandbox extends React.Component {

for (let k of Object.keys(data.formData)) {
// Set value mapping correctly for config object of react-d3-graph
utils.setValue(config, k, data.formData[k]);
setValue(config, k, data.formData[k]);
// Set new values for schema of jsonform
schemaPropsValues[k] = {};
schemaPropsValues[k]["default"] = data.formData[k];
Expand All @@ -191,7 +191,7 @@ export default class Sandbox extends React.Component {
refreshGraph = data => {
const { config, schemaPropsValues } = this._buildGraphConfig(data);

this.state.schema.properties = reactD3GraphUtils.merge(this.state.schema.properties, schemaPropsValues);
this.state.schema.properties = merge(this.state.schema.properties, schemaPropsValues);

this.setState({
config,
Expand All @@ -215,7 +215,7 @@ export default class Sandbox extends React.Component {
resetGraphConfig = () => {
const generatedConfig = {};

const schemaProps = utils.generateFormSchema(defaultConfig, "", {});
const schemaProps = generateFormSchema(defaultConfig, "", {});

const schema = {
type: "object",
Expand Down Expand Up @@ -445,7 +445,7 @@ export default class Sandbox extends React.Component {

class JSONContainer extends React.Component {
shouldComponentUpdate(nextProps) {
return !this.props.staticData && !reactD3GraphUtils.isDeepEqual(nextProps.data, this.props.data);
return !this.props.staticData && !isDeepEqual(nextProps.data, this.props.data);
}

render() {
Expand Down
2 changes: 1 addition & 1 deletion sandbox/styles.css
Expand Up @@ -57,7 +57,7 @@
.container__form {
grid-column: 5/ 6;
grid-row: 1 / 4;
min-width: 250px;
min-width: 400px;
z-index: 3;
}

Expand Down
10 changes: 3 additions & 7 deletions sandbox/utils.js
Expand Up @@ -2,7 +2,7 @@
import queryString from "query-string";
import { LINE_TYPES } from "../src/components/link/link.const";
import DEFAULT_CONFIG from "../src/components/graph/graph.config";
import utils from "../src/utils";
import { merge } from "../src/utils";

/**
* This two functions generate the react-jsonschema-form
Expand Down Expand Up @@ -57,7 +57,7 @@ function loadDataset() {
try {
const data = require(`./data/${dataset}/${dataset}.data`);
const datasetConfig = require(`./data/${dataset}/${dataset}.config`);
const config = utils.merge(DEFAULT_CONFIG, datasetConfig);
const config = merge(DEFAULT_CONFIG, datasetConfig);

return { data, config, fullscreen };
} catch (error) {
Expand Down Expand Up @@ -90,8 +90,4 @@ function setValue(obj, access, value) {
access.length > 1 ? setValue(obj[access.shift()], access, value) : (obj[access[0]] = value);
}

export default {
generateFormSchema,
loadDataset,
setValue,
};
export { generateFormSchema, loadDataset, setValue };

0 comments on commit d4bb534

Please sign in to comment.