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

push hover labels in to the bar group extent in compare mode #2414

Merged
merged 1 commit into from
Feb 27, 2018

Conversation

alexcjohnson
Copy link
Collaborator

Fixes #2350

Adds an extra step to Bar.setPositions to record the position extent of each bar group, which includes two loops over the data: one to find the min and max position of the trace group, and a second to calculate the extents. I did it this way because calculating this on the fly in hoverPoints is fairly awkward, you either have to loop over traces once for each trace (repetitious) or you could modify the hover data after finishing hoverPoints, manipulating all the bar traces in the hover data to match their position axis coordinates (adding type-dependent logic to Fx.hover directly)... this way seemed cleaner. There may be ways to move some of these calculations into existing loops in setPositions but the way these are set up this seems like it would entail a lot of duplication, and anyway I don't expect this to be a noticeable overhead for bars.

cc @etpinard

@etpinard
Copy link
Contributor

👍 for putting the hard parts in setPositions as opposed to beefing up hoverPoints.

💃 💃 🗡️ for your third bug fix of the day.

@alexcjohnson alexcjohnson merged commit 315272b into master Feb 27, 2018
@alexcjohnson alexcjohnson deleted the bar-hover-width branch February 27, 2018 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants