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

Introduce new prop onLoad #451

Merged
merged 5 commits into from
Dec 14, 2021
Merged

Introduce new prop onLoad #451

merged 5 commits into from
Dec 14, 2021

Conversation

newsiberian
Copy link
Contributor

Hi. I need to load custom map source for Geo Chart (call google.visualization.GeoChart.setMapSource()). This must be done before the first draw() in order to load my custom maps, not default.

I found tree ways to do that.

  1. windowGoogleCharts.charts.setOnLoadCallback => this.onLoad => this.onSuccess => this.props.onLoad
  2. windowGoogleCharts.charts.setOnLoadCallback => this.props.onLoad
  3. componentDidMount (table) => this.props.onLoad

The 1st - in fact is not working for my case because my callback is called after first draw() in componentDidMount (table).
The 3rd is working, but it looks ugly, so I think the second is the best option. It looks clean and it is guaranteed to be called before first draw().

@newsiberian
Copy link
Contributor Author

@rakannimer hi. any chance for this PR to be reviewed/merged?

@codecov-commenter
Copy link

Codecov Report

Merging #451 (4c673c5) into master (3f9c784) will decrease coverage by 0.28%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #451      +/-   ##
==========================================
- Coverage   49.02%   48.74%   -0.29%     
==========================================
  Files          15       15              
  Lines         514      517       +3     
  Branches      117      119       +2     
==========================================
  Hits          252      252              
- Misses        193      194       +1     
- Partials       69       71       +2     
Impacted Files Coverage Δ
src/ReactGoogleCharts.tsx 55.35% <0.00%> (ø)
src/hooks/useLoadGoogleCharts.ts 68.96% <ø> (-7.96%) ⬇️
src/types.ts 0.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f9c784...4c673c5. Read the comment docs.

@rakannimer
Copy link
Owner

Looks like some checks are failing, @dangreen will follow up, he's been helping maintain this library.

src/ReactGoogleCharts.tsx Outdated Show resolved Hide resolved
@dangreen
Copy link
Contributor

@newsiberian Hi. Thank you for contribution. Please add test case for onLoad prop.

@dangreen dangreen merged commit 855cc4f into rakannimer:master Dec 14, 2021
@dangreen
Copy link
Contributor

@newsiberian Thank you

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.

4 participants