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 options.interaction where options.hover and options.tooltips extend from #5889

Closed
wants to merge 1 commit into from

Conversation

kurkle
Copy link
Member

@kurkle kurkle commented Dec 5, 2018

Change controllers to provide that instead of hover.

Implements #5861

Pen to play with: https://codepen.io/kurkle/pen/yQdVjm

Add interaction options that both hover and tooltips extend from. Change defaults to provide that instead of hover.
@simonbrunel
Copy link
Member

Thanks @kurkle for your contribution. We tagged this feature for next major release because it may introduce breaking changes for external code. I see you added some logic to generate tooltips / hover from interactions but I think it doesn't work if you try to update interactions.* with new values and call chart.update() because tooltips and hover would already exist.

Also, it's not consistent with our fallback logic: we don't generate missing options but compute final values in place (e.g. if not tooltip.mode then fallback to interactions.mode). So for now, I would close this PR and plan this feature for v3.

@kurkle
Copy link
Member Author

kurkle commented Dec 5, 2018

@simonbrunel oh, I now see. With consistent (and proper) fallback logic it would introduce breaking changes for external code. So closing this is ok for me.

@kurkle kurkle closed this Dec 5, 2018
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.

2 participants