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

add Highmaps support #18

Closed
wants to merge 1 commit into from
Closed

add Highmaps support #18

wants to merge 1 commit into from

Conversation

nash716
Copy link

@nash716 nash716 commented Aug 14, 2015

I want to use Highmaps with React.
This allows users to render maps.

Usage:

var Highcharts = require('react-highcharts');
require('react-highcharts/highmaps');
<Highcharts type="map" config={config}></Highcharts>

@kirjs
Copy link
Owner

kirjs commented Aug 14, 2015

Hi Nash, Thanks a lot for the PR, did you have a chance to test it?

@joshhornby
Copy link
Contributor

Won't this cause this issue?

#9

@kirjs
Copy link
Owner

kirjs commented Aug 14, 2015

I guess that's something I'd have to do with more.js to close #9
That would be an API change and would require version bump.

Feel free to do a PR, or I'll try to do that over the weekend

@kirjs
Copy link
Owner

kirjs commented Aug 15, 2015

upd: Josh, I misread your comment.
Highcharts is a bit confusing in this way: more and map, are add-ons to highcharts, while highstock is a separate entity.
I just have to get it all together and figure out all possible permutations, something I'm planning to do over the weekend.

@nash716
Copy link
Author

nash716 commented Aug 26, 2015

Hi @kirjs, thanks for your comment, and I'm sorry for the late reply.
I tested my code in my project by installing react-highcharts with npm i nash716/react-highcharts

I think that my code doesn't cause #9.

a.jsx

var Highcharts = require('react-highcharts');

b.jsx

var Highcharts = require('react-highcharts');
require('react-highcharts/highmaps');

c.jsx

var Highcharts = require('react-highcharts');
require('react-highcharts/highmaps');

react-highcharts/highmaps was required twice. On the other hand, a.jsx requires react-highcharts only. It works without any errors.

But the big overhaul project is in progress (#20), I think I should wait for it to finish.
What do you think?

@kirjs
Copy link
Owner

kirjs commented Aug 26, 2015

Hi Nash, no worries, I agree on #20, let's get it in, and see how the merge would look like.

@nash716 nash716 closed this Aug 27, 2015
@thientran1707
Copy link

@kirjs can you update the readme.md to show how to use highmap with a map?

@kirjs kirjs reopened this Sep 14, 2015
@kirjs
Copy link
Owner

kirjs commented Sep 14, 2015

The pull request has not been merged yet.
@nash716 Can you get it on top of master and see if it needs some updating?

@nash716
Copy link
Author

nash716 commented Sep 15, 2015

Hi,
I've been tied up with work, couldn't grasp the change.
And what is worse, I'm busy for a while...
I'll make time to update my code.

@thientran1707
If you are in a hurry, you can use this feature for now.
Please run $ npm i nash716/react-highcharts.
Readme is here.

@gmeroz
Copy link

gmeroz commented Sep 26, 2015

Hi,
Any chance this pull request will be merged soon?
Thanks,
Gil

@nash716
Copy link
Author

nash716 commented Oct 7, 2015

I tried to update my code, but some problems were found.

highmaps-release doesn't contain map module.
highmaps-release contains highmaps.src.js, and highmaps.src.js has Highcharts.
The map module is not available.
Codes like below will cause this ( #39 )

a.jsx

var Highcharts = require('react-highcharts');

b.jsx

var Highmaps = require('react-highcharts/highmaps');

@kirjs
Copy link
Owner

kirjs commented Oct 18, 2015

Fixed in #45

@kirjs kirjs closed this Oct 18, 2015
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.

None yet

5 participants