Skip to content
This repository was archived by the owner on May 25, 2019. It is now read-only.

Fixes for id attar + add click attr for callback.#8

Open
developer88 wants to merge 1 commit intoangular-ui:masterfrom
developer88:master
Open

Fixes for id attar + add click attr for callback.#8
developer88 wants to merge 1 commit intoangular-ui:masterfrom
developer88:master

Conversation

@developer88
Copy link

First of all - with the latest version of jqplot this directive is not working for me, because it call $(element).jqplot, but according to official jqplot's documentation there is only one appropriate way - $.jqplot('elemente's id' ....);
Second - I have added chart-click attribute to pass a function that will be called when event 'jqplotDataClick' is fired.

@developer88
Copy link
Author

Smth wrong with Travis, but definitely, that is not my fault.

Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to just do elem.unbind - elem is normally a jqLite wrapped element, but with jQuery present, it will be a jQuery wrapped element.

@wesleycho
Copy link
Contributor

I will fix these Travis errors tonight - other than these minor issues I've commented on, I like this PR, and am willing to merge this and tag a new release when this is tidied up.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be a good idea to keep a reference to $.jqplot(id, data, opts) cached outside the render function.

@wesleycho
Copy link
Contributor

There should be a test added for the binding of the callback as well, to test that it properly sets the correct binding and unsets the old binding.

Copy link
Contributor

Choose a reason for hiding this comment

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

click_callback should be clickCallback, to maintain style consistency.

@tohyf
Copy link

tohyf commented Dec 20, 2015

Any time this fix will be finalized and committed ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments