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 sure to always set the label font colour when rendering tooltip text #4766

Closed
wants to merge 1 commit into from

Conversation

etimberg
Copy link
Member

Resolves #4741

@benmccann
Copy link
Contributor

I think there's some kind of race condition that's making Travis fail

@benmccann
Copy link
Contributor

I'm curious what the plan is for 2.7.1. Do we need to tag the PRs that we want to be a part of that release with a milestone of 2.7.1 to cherrypick those commits? Or will we at that time take the latest master and merge all the commits to the release branch?

@etimberg
Copy link
Member Author

@benmccann what we've been doing so far is just merging bug fixes and holding on the features until we get an idea if we need a 2.7.1 or not

@benmccann
Copy link
Contributor

benmccann commented Sep 17, 2017

Ok, that makes sense. Thanks for filling me in!

}

var textColor = mergeOpacity(vm.labelTextColors[i], opacity);
ctx.fillStyle = textColor;
fillLineOfText(line);
Copy link
Contributor

Choose a reason for hiding this comment

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

You could combine these two lines into one:

ctx.fillStyle = mergeOpacity(vm.labelTextColors[i], opacity);

It might be better for minification / download size that way

}

var textColor = mergeOpacity(vm.labelTextColors[i], opacity);
Copy link
Member

Choose a reason for hiding this comment

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

Before introducing this bug, the fillStyle was set before all the loops, meaning that beforeBody and bodyItem.before was also supposed to be drawn using bodyFontColor. Not sure if that still work, there is no more ctx.fillStyle initialization?

@SlaunchaMan
Copy link

@etimberg Any updates on this? We’re seeing this bug in certain circumstances and I'd love to see a 2.7.1 with a fix!

@retallicka
Copy link

It would be have this released - we need the tooltips colours fixed

@etimberg
Copy link
Member Author

etimberg commented Oct 2, 2017

Done in #4783

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants