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

fix for the digest bug #351

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Conversation

stvno
Copy link

@stvno stvno commented Dec 28, 2020

No description provided.

 to avoid "Digest cycle already in progress" error.
 to avoid "Digest cycle already in progress" error.
to avoid "Digest cycle already in progress" error.
@nbering
Copy link
Member

nbering commented Dec 29, 2020

Thanks for your pull request! This project hasn't been active in a while for a few reasons:

  1. It mostly just works.
  2. The upstream side of Google Charts hasn't seen much activity, and seems to be mostly in maintenance mode.
  3. AngularJS 1.x is in LTS with all support ending in December 2021.

Seeing as I haven't spent much time on the project of late, it would help me greatly to understand whether it's worth the time to cut a release if you'd include some explanation of what this is trying to solve, and how the solution works.

@@ -3,6 +3,7 @@
angular.module('googlechart')
.directive('agcOnReady', onReadyDirective);

onReadyDirective.$inject = ['$timeout'];
function onReadyDirective(){
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you're registering a dependency here, but didn't include a method argument to receive it. This should be throwing an error for $timeout is not a function when the callback actually runs.

Suggested change
function onReadyDirective(){
function onReadyDirective($timeout){

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

2 participants