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

onNodeSizeChange does not return value for callback #27

Open
matthax opened this issue Jul 8, 2019 · 2 comments
Open

onNodeSizeChange does not return value for callback #27

matthax opened this issue Jul 8, 2019 · 2 comments

Comments

@matthax
Copy link

matthax commented Jul 8, 2019

The callback onNodeSizeChange defined in actions.ts
does not return a value, which means you need to manually override it when trying to create state hooks for the chart component.

For example, I only want to override some event handlers so I import the already defined handlers:

import * as callbacks from '@mrblenny/react-flow-chart/src/container/actions';

// define the correct handler (return the chart)
export const onNodeSizeChange = ({ nodeId, size }) => (chart) => {
  chart.nodes[nodeId] = {
    ...chart.nodes[nodeId],
    size,
  };
  return chart
};
// overwrite the existing handler with our patched version
callbacks.onNodeSizeChange = onNodeSizeChange;

// utility to allow partial override of callbacks
export const configureChart = (setChart, overrides = {}) => (
  Object.keys(callbacks).reduce((reducer, key) => {
    const callback = overrides && typeof overrides[key] !== 'undefined' ? overrides[key] : callbacks[key];
    reducer[key] = (...args) => setChart(callback(...args));
    return reducer;
  }, {})
);

I can then define a component with my custom overrides, while preserving the other default handlers:

 const [chart, setChart] = useState(initialChart);
  const callbacks = configureChart(setChart, {
    onDeleteKey, // some custom handler
    onNodeClick, // another custom handler
  });

I can open a PR, essentially all that is required is to return the updated chart from the handler.

@matthax
Copy link
Author

matthax commented Jul 8, 2019

After reviewing the code, it's not really possible to use the current callbacks for the state hooks anyways, since they're returning the same mutated object.

It would be necessary to create a new chart instance and pass it back from the handler for the state change to actually cause a re-render. I would be happy to open a PR for that as well, but it might have negative performance implications so that's up to you.

@yuyokk
Copy link
Contributor

yuyokk commented Aug 11, 2019

After reviewing the code, it's not really possible to use the current callbacks for the state hooks anyways, since they're returning the same mutated object.

@matthax faced the same issue :( Please let me know if you found any workarounds.

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

No branches or pull requests

2 participants