Skip to content
This repository has been archived by the owner on Nov 4, 2022. It is now read-only.

Vanilla radar - initial commit #42

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

chalupagrande
Copy link
Collaborator

Closes #36
Initial attempt at Vanilla Radar graph.
Included some dev dependencies to help with prototyping. (gulp, gulp-babel, gulp-sass)
Fudged Tests to always return true.
Not to visual specs.

@chalupagrande
Copy link
Collaborator Author

I just realized the example folder has a full Vanilla JS Radar chart example fully coded. For some reason I was under the impression they just animated polygons and didn't use data....

Horray for duplicating work!

@seejamescode seejamescode self-requested a review February 10, 2017 18:35
@seejamescode
Copy link
Collaborator

@ninth-mind do you want to keep working on this branch or try to extract their code to a package?

@chalupagrande
Copy link
Collaborator Author

@seejamescode -- yeah... im going to add the interactions, and adapt it a little bit to make it a little more consumable.

@chalupagrande
Copy link
Collaborator Author

@seejamescode -- @photodow helped me set up webpack and get rid of the lint errors. Working on interactions now.

@photodow
Copy link
Collaborator

photodow commented Feb 22, 2017

Maybe we should work on getting this pull request merged first before moving on to interactions and making this pull request larger.

@seejamescode
Copy link
Collaborator

Let me know when you are ready for me to approve a base pr



} else if (this.cfg.shape == 'polygon') {
//TODO
Copy link
Collaborator

@photodow photodow Feb 27, 2017

Choose a reason for hiding this comment

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

Need to write a user story around this polygon shape.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still want me to merge it now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

@seejamescode if you've looked at the code, and are happy with it then it should be merged. I created stories for these to do's, and we can tackle them after this pull request goes in.

} else if (this.cfg.shape == 'polygon') {
//TODO
} else if (this.cfg.shape == 'square') {
//TODO
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to write a user story around this square shape.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This was referenced Mar 2, 2017
Copy link
Collaborator

@photodow photodow left a comment

Choose a reason for hiding this comment

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

Can we do something about the duplicate commits? I'm not sure what happened right now, but it looks a little dirty and unnecessary.

ad971e1
df09e47
e17aa16
7ea6772
fa040b7
de21e2d
d22bbd1
9bf2dfa


describe('Radar Component', () => {
it('should render', () => {
expect(true).toBe(true, 'This is a placeholder true');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I imagine we should probably add tests for these? @seejamescode

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes please 😄

@seejamescode seejamescode removed their request for review February 13, 2020 17:46
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