Skip to content

Fix mathjax placement in ternary aaxis titles #3513

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

Merged
merged 2 commits into from
Feb 5, 2019

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Feb 4, 2019

fixes #3455 -- cc @plotly/plotly_js

I was really hoping to get rid of this hacky block

if(svgClass[0] === 'y') {
mathjaxGroup.attr({
transform: 'rotate(' + [-90, +_context.attr('x'), +_context.attr('y')] +
') translate(' + [-newSvgW / 2, dy - newSvgH / 2] + ')'
});
newSvg.attr({x: +_context.attr('x'), y: +_context.attr('y')});
}
else if(svgClass[0] === 'l') {
newSvg.attr({x: _context.attr('x'), y: dy - (newSvgH / 2)});
}
else if(svgClass[0] === 'a') {
newSvg.attr({x: 0, y: dy});
}
else {
newSvg.attr({x: newX, y: (+_context.attr('y') + dy - newSvgH / 2)});
}

entirely, but since our MathJax test coverage is lackluster, I chose the safe road.

@etpinard etpinard added bug something broken status: reviewable labels Feb 4, 2019
@etpinard
Copy link
Contributor Author

etpinard commented Feb 5, 2019

@antoinerg or @archmoj could one of you review this thing at some point today?

Thank you!

@archmoj archmoj self-requested a review February 5, 2019 19:29
@@ -134,7 +134,7 @@ exports.convertToTspans = function(_context, gd, _callback) {
else if(svgClass[0] === 'l') {
newSvg.attr({x: _context.attr('x'), y: dy - (newSvgH / 2)});
}
else if(svgClass[0] === 'a') {
else if(svgClass[0] === 'a' && svgClass.indexOf('atitle') === -1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use

if(svgClass === 'atitle') {

here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would that be better?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering of an edge case where 'atitle' appears in other position in the text.
Though I was not able find any word containing those letters in order!
Anyway could we use this condition instead to test if atitle only occurs at the start of the string?

else if(svgClass[0] === 'a' && svgClass.indexOf('atitle') !== 0) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Done in -> c7878b3

@archmoj
Copy link
Contributor

archmoj commented Feb 5, 2019

Thanks @etpinard for fixing this bug.
💃 when the tests were passed.

@etpinard etpinard merged commit 5fd2b80 into master Feb 5, 2019
@etpinard etpinard deleted the ternary-aaxis-title-mathjax-fix branch February 5, 2019 22:15
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.

Bad placement of aaxis LaTeX title in a scatterternary plot #1399
2 participants