jchen006 / shenanigans

Recipe hacking
4 stars 0 forks source link

Converting radar #105

Closed snallapa closed 5 years ago

snallapa commented 5 years ago

Radar component with fixed API

jchen006 commented 5 years ago

@snallapa very nice job and fantastic work but just some feedback Based on this photo

screen shot 2019-02-13 at 12 27 59 am

(1) Can we so something about the size since it just feels kind of clunky that I need to rescroll (2) Is there any possible we can color code the selection against the color that we use for the radar graph I think way way it would be easier for people to follow.

snallapa commented 5 years ago

new changes made to address issues above. this was much harder than I thought lol but it worked out

jchen006 commented 5 years ago

Nice job! A little bit slight interaction is off. When I click the "X" should remove the representation on the graph. Second the sizing of the search and select is a little off. Spacing is going to be little major because I'm hoping display the actual value breakdowns on the side somewhere. Hence I think it's something to point out. Also is there a way to signify when something is 'onHover'? screen shot 2019-02-16 at 12 15 09 am You're welcome to merge but we'll have to keep this as a known issue for all of these issues or feature update.

snallapa commented 5 years ago

I think spacing can be fixed when that comes up. I fixed the X issue and yeah you can get when you over over points as well with onHover callback for the Radar component