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

Enhancement: allow React context and default chart context property - e.g. from function arguments #38

Closed
chillyistkult opened this issue Jul 25, 2018 · 7 comments

Comments

@chillyistkult
Copy link

I want to trigger the loading animation, how do I trigger showLoading()?

@KacperMadej
Copy link

You could access chart instance through context of chart's load event to call showLoading() and hideLoading() on it.

@chillyistkult
Copy link
Author

chillyistkult commented Jul 26, 2018

@KacperMadej How do I access react context within this events? For example to check if this.props.loading is true?

events: {
   render() {
      if (this.props.loading) { // How?
         this.showLoading();
      } else {
         this.hideLoading();
      }
   },
},

@KacperMadej
Copy link

Have you tried fat arrow function? But then this will not be the chart, so you need to choose, or use a different event to store chart reference in react context.

@chillyistkult
Copy link
Author

chillyistkult commented Jul 27, 2018

I tried a lot of things, including arrow function, but with no success. This problem unfortunately applies to any function I would like to use within Highchart context.

I think the right way to do it would be if Chart is just a parameter of the callback function, instead of providing it via this context.

events: {
   render: (chart) => {
      if (this.props.loading) {
         chart.showLoading();
      } else {
         chart.hideLoading();
      }
   },
},

@highcharts highcharts deleted a comment from sebastianbochan Jul 30, 2018
@pawelfus
Copy link

That's gonna break backward compatibility (events.render is a callback directly from Highcharts core), confusing much more users than react users. Probably this wrapper could take care of it, but why don't we simply..

Define:

var propLoading = this.props.loading;

Then use it:

events: {
   render: () {
      if (propLoading) {
         this.showLoading();
      } else {
         this.hideLoading();
      }
   },
},

Internal note:
In general, if we decide to replace context in render, then we should replace context for all callbacks to keep them consistent.

@chillyistkult
Copy link
Author

In general, if we decide to replace context in render, then we should replace context for all callbacks to keep them consistent.

Please, it would make so much sense!

@KacperMadej KacperMadej changed the title How to call showLoading()? Enhancement: allow React context and default chart context property - e.g. from function arguments Aug 14, 2018
@ppotaczek
Copy link
Contributor

Currently, the both contexts (React component and chart) in chart functions can be accessed by binding the right this, for example:

constructor(props) {
  this.state = {
    options: {
      chart: {
        events: {
          render: function() {
            const chart = this.chart;
            const component = this;
            ...
          }.bind(this)
        }
      },
      ...
    }
  };
}

chartCallback(chart){
  this.chart = chart;
}

render() {
  return (
    <HighchartsReact
      highcharts={Highcharts}
      options={this.state.options}
      callback={ this.chartCallback.bind(this) }
    />
  )
}

Live demo: https://stackblitz.com/edit/react-e7gcju?file=index.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants