Skip to content
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 stroke properties to discrete color legend (#860) #994

Merged

Conversation

@Wattenberger
Copy link
Contributor

Wattenberger commented Oct 8, 2018

Added the ability to set stroke properties (strokeDasharray, strokeStyle, strokeWidth) of Discrete Color Legend items to match Line Series components. At first I considered mimicking the colors attribute and letting users add a strokeWidths array, but it seemed to over-complicate the api, so I settled with grabbing those properties from an item's object.

The legend items now render an svg path instead of a 2x14px div, which was necessary to match the dash-array property, but it also might help keep the legend consistent with the chart.

I edited one of the legend examples to show the implementation - happy to remove it if it's too heavy-handed.

image

Happy to hear any feedback, thanks!

strokeDasharray, strokeStyle, strokeWidth
Copy link
Collaborator

mcnuttandrew left a comment

You are on fire! I think this is a fine solution to this problem. Would you mind adding a test to support this change?

update tests to use svg properties, add tests for stroke dasharray and stroke width, and filter out unused styles
@Wattenberger

This comment has been minimized.

Copy link
Contributor Author

Wattenberger commented Oct 8, 2018

Oh shoot! Tests, of course!

I updated the tests to match the svg styles/elements and added stroke-width and stroke-dasharray tests. Thanks for the speedy response!

@mcnuttandrew

This comment has been minimized.

Copy link
Collaborator

mcnuttandrew commented Oct 8, 2018

Looks good! Thanks again

@mcnuttandrew mcnuttandrew merged commit 1d350bb into uber:master Oct 8, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.