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

Remove box ungrouping magic #3445

Merged
merged 1 commit into from
Jan 21, 2019
Merged

Remove box ungrouping magic #3445

merged 1 commit into from
Jan 21, 2019

Conversation

etpinard
Copy link
Contributor

fixes #3390 as per @nicolaskruchten 's #3390 (comment)

To be merged into #3234 to avoid conflicts in box/cross_trace_calc.js

cc @plotly/plotly_js

... and thus allow single-box groups!
@etpinard etpinard added bug something broken status: reviewable labels Jan 17, 2019
@antoinerg
Copy link
Contributor

@etpinard I am not too familiar with box trace but I am a bit confused by the result of the new baseline you introduced in this PR. According to the description of boxmode:
If *group*, the boxes are plotted next to one another centered around the shared location.. If I only have one box at a given location, I would expect the shared location to be the box's center position. Yet, the boxes in baseline box_single-group.json are not aligned anymore with the tick labels 🤔 Is this normal?

@etpinard
Copy link
Contributor Author

I would expect the shared location to be the box's center position. Yet, the boxes in baseline box_single-group.json are not aligned anymore with the tick labels Is this normal?

See discussion in #3390 (comment)

@antoinerg
Copy link
Contributor

See discussion in #3390 (comment)

I think I understand after a careful second read. I agree, let's 🔪 that magic!

💃

@etpinard etpinard merged commit 2517ed0 into joyplots2 Jan 21, 2019
@etpinard etpinard deleted the rm-box-ungrouping-magic branch January 21, 2019 21:01
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