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

Make histogram interactive in DomainSlider #1265

Merged
merged 3 commits into from
Nov 8, 2022

Conversation

loichuder
Copy link
Member

@loichuder loichuder commented Nov 7, 2022

That could have been as simple as adding this line.

But due to airbnb/visx#1174, the point positions when dragging are wrongly computed as the domain slider root div is translated via transform. The purpose of the transform was to center the tooltip with respect to the slider.

Therefore, I reimplemented the centering by adding a flex container around the slider container and the tooltip to center them. This allows to remove transform from the histogram hierarchy of parents to work around airbnb/visx#1174.

@loichuder
Copy link
Member Author

There appears to be a little mismatch between the two implementations:

edit_domain diff

@axelboc axelboc self-requested a review November 8, 2022 10:42
@axelboc
Copy link
Contributor

axelboc commented Nov 8, 2022

The div inside which the tooltip used to be has margin-right: -0.375rem;. So moving it out of this element shifts the tooltip to the left by this amount, which means that it is now more centered than it was before 💯 😄

@loichuder
Copy link
Member Author

/approve

@loichuder loichuder merged commit f6a1152 into main Nov 8, 2022
@loichuder loichuder deleted the interactive-histogram-slider branch November 8, 2022 13:10
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