Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A collection of fixes and additions #23

Closed
wants to merge 15 commits into from

Conversation

msteinmn
Copy link
Contributor

I created a number of fixes and some additions you might find useful. They are separated by commits, with a few bug fixes in later commits.

The additions are summarized like that:

Allow nodes to be React components and not just styled divs
Adjust for chart offset origin different than 0,0 and scrolling
Added a resize observer to detect node size changes and re-render ports and links
Node size now adjusts properly, re-rendering ports and links on node size change
Allow nodes to be rotated with ports rotating accordingly. New prop orientation on the node defines the angle of rotation of the node (ports rotate automatically, the node needs to be given specific css to rotate it)
Exported actions so that it can be used with the FlowChart component
Added a validation function (needs some more work) for new links
Added nodeProps prop to Flowchart so that data can be sent to e.g. autocomplete menu options inside nodes (spread to Node and InnerNode)
Added boolean snap prop to Flowchart to enable snap to grid for node placements

…not 0,0, then offsetX,offsetY need to be subtraced when a new node is placed. Also added a scroll listener so that updateSize is called on scroll.
…server. Allows custom InnerNodes to change size, with ports and links re-rendering.
…d. This change was made so that onPortPositionChange is called on every re-render. This is needed for example for nodes that are rotated, i.e. a node rotated 45 deg to stand on a corner.
…ther nodes should snap to the defined grid on move or drop. The grid remains hardwired at 20px defined in CanvasOuter.default.
…w key called orientation that specifies the angle of rotation in deg. the custom node definition has to add the necessary CSS to rotate the node. The code here rotates the position of the ports.
…ed on rules implemented in validateLink.ts. Would like to make this validation file customizable outside the package.
…nt. Added that in error to the input node object.
… InnerNode. This allows data to be made available in e.g. dropdown menus in node components.
@nathan-haines
Copy link

These are some great fixes, would love to utilize these

@MrBlenny
Copy link
Owner

Yep, sorry. Had a busy couple of weeks. Hoping to get it merged this weekend.

@MrBlenny
Copy link
Owner

I've added a few more changes here. I'm not quite sure how to make the rotation changes work. Any chance you can make the CustomNodeInner demo work?

@msteinmn
Copy link
Contributor Author

I like the way you made the config object much more generic and added the validation function for links into it. Very nice.

For the rotation to work three things have to be true:

  1. the node object inside the chart prop has to include a field 'orientation' like so: orientation: -45,
    So every time you create a rotated node it has to include this prop, where 45 is the angle in deg.
  2. the outer custom node then has to include css to rotate it, like so:

class Node extends React.PureComponent {
render() {
const { nodeProps, node, children, isSelected, style, ...otherProps } = this.props;
const selectedStyle = isSelected ?
{ boxShadow: '8px 8px 12px rgba(0,0,0,.5)' } : null;

    const { transform, ...otherStyles } = style;
    return (
      <div
        style={{
          transform: `${transform} rotate(${-node.orientation}deg)`,
          ...otherStyles,
          ...styles.outerNode,
          ...selectedStyle,
        }}
        {...otherProps}
      >
        {children}
      </div>
    );
}

}

  1. the custom inner node has to be rotated back so that its content remains horizontal, like so:

const NodeInner = ({ classes, node, nodeProps }) => {
return (
<div className={classes.innerContent} style={{ transform: rotate(${node.orientation}deg) }}>
{other inner node content}

)
}

On another note I remember you mentioned somewhere that you convert the resulting chart object into an array for storage and further processing. Could you share the function that does this conversion back and forth and the format you use?

@msteinmn
Copy link
Contributor Author

msteinmn commented Jul 18, 2019

And one more thing: I also introduced additional port types to provide styles so that the ports align properly in the corners of the now rotated node like so:

cornerLeft: {
height: '80%',
marginTop: '20%',
top: '6px',
right: '80px',
flexDirection: 'column',
justifyContent: 'flex-end',
'> div': {
margin: '3px 0',
},
},
cornerRight: {
height: '80%',
top: '-3px',
right: '-6px',
flexDirection: 'column',
justifyContent: 'flex-start',
'> div': {
margin: '3px 0',
},
},
cornerTop: {
height: '80%',
marginBottom: '20%',
top: '-6px',
right: '80px',
flexDirection: 'column',
justifyContent: 'flex-start',
'> div': {
margin: '3px 0',
},
},
cornerBottom: {
height: '100%',
top: '0px',
right: '-6px',
flexDirection: 'column',
justifyContent: 'flex-end',
'> div': {
margin: '3px 0',
},
},

This can also be used for non-rotated nodes to align ports in a corner.

import Draggable from 'react-draggable'
import ResizeObserver from 'react-resize-observer'
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe there should be a helper that uses either native ResizeObserver (see Canvas.wrapper.tsx) or this lib?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cannot help with that.

@msteinmn
Copy link
Contributor Author

msteinmn commented Aug 9, 2019

Do you need me to add anything to the info provided above?

@sosafe-tobias-justinger

Would be great if you could merge. :)

@MrBlenny
Copy link
Owner

Merged as part of #40

@MrBlenny MrBlenny closed this Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants