Skip to content

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
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