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

add style prop to hexagon #13

Merged
merged 2 commits into from Nov 13, 2017

Conversation

Projects
None yet
2 participants
@cachilders
Contributor

cachilders commented Nov 13, 2017

I added a style prop to the Hexagon component for my own project and thought it might be useful to others. It explicitly anticipates a string in the 'fill:red;stroke:1;etc' style, but will render any old style object as well. Btw, I love this library. Thanks for sharing it.

@Hellenic

This comment has been minimized.

Show comment
Hide comment
@Hellenic

Hellenic Nov 13, 2017

Owner

Hey!
Cheers, sounds like a addition that could be useful. I guess for the most parts passing in a className would do the trick, but I guess sometimes it's handy to override it with styles, or just pass hex-specific styles.

However, isn't style prop supposed to be an object? Now the prop type is set to be a string.
Also, if could update the snapshot (or even add this in to a test). Once the test passes I'll be happy to merge this in.

Owner

Hellenic commented Nov 13, 2017

Hey!
Cheers, sounds like a addition that could be useful. I guess for the most parts passing in a className would do the trick, but I guess sometimes it's handy to override it with styles, or just pass hex-specific styles.

However, isn't style prop supposed to be an object? Now the prop type is set to be a string.
Also, if could update the snapshot (or even add this in to a test). Once the test passes I'll be happy to merge this in.

@cachilders

This comment has been minimized.

Show comment
Hide comment
@cachilders

cachilders Nov 13, 2017

Contributor

No lie, I went back and added those changes, pushed, and just now saw your comment. I went with the string just because it seems preferred by the SVG docs I scanned in MDB, but I personally used an object and felt like a hypocrite.

Contributor

cachilders commented Nov 13, 2017

No lie, I went back and added those changes, pushed, and just now saw your comment. I went with the string just because it seems preferred by the SVG docs I scanned in MDB, but I personally used an object and felt like a hypocrite.

@cachilders

This comment has been minimized.

Show comment
Hide comment
@cachilders

cachilders Nov 13, 2017

Contributor

screen shot 2017-11-13 at 12 48 54 am

Fwiw, my implementation needed programmatically derived fills, so it could be more of an edge use.
Contributor

cachilders commented Nov 13, 2017

screen shot 2017-11-13 at 12 48 54 am

Fwiw, my implementation needed programmatically derived fills, so it could be more of an edge use.

@Hellenic Hellenic merged commit ed4c913 into Hellenic:master Nov 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment