Radar mouseover #138 - Githubissues

HumbleSoftware / Flotr2

Graphs and Charts for Canvas in JavaScript.

http://www.humblesoftware.com/flotr2/

MIT License

2.45k stars 528 forks source link

Radar mouseover #138 #141

Closed acs closed 11 years ago

acs commented 12 years ago

Work for #138. I have to modify a bit hit plugin so in radar graph I have access to all the data series and can select the correct vertex to be drawn. I don't like the solution but it is a short path to talk with you about the correct solution for graphs with several data series to be displayed and the hit plugin.

acs commented 12 years ago

Some comments about this pull request:

If two radar graph share points, which is informed by hit of it? I think that the two should be, so they can also fire for example "click" events. Taking a look to hit plugin, only one point is selected as the closest so it is the same here.
In the closest selection we get the first point inside the correct distance not the closest one.

cesutherland commented 12 years ago

Thanks for the contrib! I'll review more closely as soon as I'm able.

Take a look at the make file for build instructions. Please don't commit any built files, but it'll run the linting for you with smoosh and help you catch the kinds of errors above.

acs commented 12 years ago

Thanks Carl, I wait for the complete review and instructions for avoiding this issues in the future. I am able to build flotr2 with smoosh but I have not used jasmin to add new test cases. Any clues for improving the development process will be of great value!

acs commented 12 years ago

Carl, how is going the review?

Cheers!

cesutherland commented 12 years ago

I don't have any tests for hit functionality currently :-( so no need to worry about that.

For now follow any instructions from the smoosh linting and add the commits to the PR please!

On Mon, Aug 13, 2012 at 2:47 AM, Alvaro del Castillo < notifications@github.com> wrote:

Carl, how is going the review?

Cheers!

— Reply to this email directly or view it on GitHubhttps://github.com/HumbleSoftware/Flotr2/pull/141#issuecomment-7684440.

acs commented 11 years ago

Hi Carl,

I have now a complete working env with smoosh and jasmine-headless-webkit to create and pass tests.

My plan is as you said, follow smoosh linting, commit the need changes and take a look to tests in order to cover this new feature with tests. I have not used jasmine before but I hope I can manage it.

We are now using radar graphs in our reports:

http://bitergia.com/public/previews/VizGrimoireJS/report/radar.html

and we need the interactivity :)

acs commented 11 years ago

Carl, I have fixed the two JSHint extra commas. I am now looking for adding tests to the new feature, but I have not found tests for radar at all. Should I add tests for radar in general, and specific for this new feature? I have jasmine-headless-webkit working in my Ubuntu 12.10 after some hacking.

cesutherland commented 11 years ago

Alvaro, it would be great to have tests or radar in general. There ought to be at least one regression test. You should be able to run the tests in a browser as well with flotr2/spec/

-Carl

On Sun, Dec 23, 2012 at 7:24 PM, Alvaro del Castillo < notifications@github.com> wrote:

Carl, I have fixed the two JSHint extra commas. I am now looking for adding tests to the new feature, but I have not found tests for radar at all. Should I add tests for radar in general, and specific for this new feature? I have jasmine-headless-webkit working in my Ubuntu 12.10 after some hacking.

— Reply to this email directly or view it on GitHubhttps://github.com/HumbleSoftware/Flotr2/pull/141#issuecomment-11652417.

acs commented 11 years ago

Carl, after studying the test systems (really cool, specially the toImageDiffEqual matcher, I have seen that the basic radar test is already there. The new code is for interactivity with the radar chart so I am not sure if you want a test for that. I am have not seem how interactivity is tested in flotr2.

The actual basic radar test is passed.

Cheers