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 the ability to highlight multiple series with setSelection #805

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add the ability to highlight multiple series with setSelection #805

wants to merge 1 commit into from

Conversation

kchengsf
Copy link

@kchengsf kchengsf commented Dec 6, 2016

I'd like to add the ability to highlight multiple series in setSelection.

I've added a test and updated relevant documentation parts to accommodate this, however there are quite a few ways this could be implemented. Please let me know if a different approach makes more sense. In particular, repeatedly calling _renderLineChart is cleaner but requires iteration through all series each call. Moving the logic downward requires the method to understand the nuance between an array vs string option.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 90.006% when pulling b9b21c0 on kchengsf:master into 623dd1d on danvk:master.

@danvk
Copy link
Owner

danvk commented Dec 27, 2016

@klausw any thoughts on this? Seems fine to me other than style guide issues.

@klausw
Copy link
Contributor

klausw commented Dec 28, 2016

I think this looks good, and it's functionality I've been wanting myself. I don't have a strong preference about the implementation. There may be useful followups such as restricting interactive highlighting to switch among the selected series, assuming that this isn't already working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants