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

(Rebase) Move drawing of gridLines into separate plugin and add border functionality #5480

Closed
wants to merge 5 commits into from

Conversation

okcoker
Copy link

@okcoker okcoker commented May 8, 2018

Rebase fixes for #4117

JSFiddle: https://jsfiddle.net/y0whvnvk/78/

@okcoker okcoker changed the title T 5001 (Rebase) Move drawing of gridLines into separate plugin and add border functionality May 8, 2018
glColor: lineColor,
glBorderDash: borderDash,
glBorderDashOffset: borderDashOffset,
tmWidth: lineWidth,
Copy link
Contributor

Choose a reason for hiding this comment

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

It took me awhile to figure out, but I'm guessing tm stands for tick mark. I would just use tick instead since it's not much longer and is much clearer

var MODEL_KEY = '$gridlines';

// This is copied from core.scale.js, which is also required here and im not sure where it should be placed for both of them to access it
function getLineValue(scale, index, offsetGridLines) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is now called getPixelForGridLine. The code no longer quite matches. I'd recommend making it an attribute of a class and marking it private by prefixing it with _ and adding a @private jsdoc tag so that it can be reused rather than copied here

@benmccann
Copy link
Contributor

This PR has been inactive for quite awhile and border functionality was added in #6883 so I'm going to close this. If you'd still like to get some of the additional changes from this PR in, we'd be happy to review them, but please join the #dev Slack channel to propose the changes ahead of time so that we can give early feedback to help make the process as painless as possible

@benmccann benmccann closed this Jan 10, 2020
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.

3 participants