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.

Sign up for GitHub

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

Jump to bottom

Scriptable tick fonts #6939

Merged
merged 2 commits into from
Jan 14, 2020
Merged

Scriptable tick fonts #6939

merged 2 commits into from
Jan 14, 2020

Conversation

benmccann
Copy link
Contributor

Closes #2442

@benmccann benmccann force-pushed the scriptable-tick-fonts branch from 70ac152 to 1808cd1 Compare January 9, 2020 05:41
@benmccann benmccann mentioned this pull request Jan 9, 2020
Closed
@kurkle
Copy link
Member

kurkle commented Jan 9, 2020

It would be more efficient to use one callback for font. But thats a bit bigger change.
Something to consider before alpha through.

etimberg
etimberg reviewed Jan 10, 2020
View reviewed changes
Copy link
Member

@etimberg etimberg left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Just want to confirm that there's no major performance hit. I think it's OK now because we filter the ticks before measuring in most cases

src/core/core.scale.js Show resolved Hide resolved
@benmccann
Copy link
Contributor Author

There's no perf hit because there are so few labels - this is run only after autoSkip. I agree resolving each option individually seems slightly wasteful, but I was trying to follow the pattern from _resolveDataElementOptions and it seems like that's what's done there

@benmccann benmccann requested review from kurkle and etimberg January 10, 2020 19:18
etimberg
etimberg previously approved these changes Jan 10, 2020
View reviewed changes
@benmccann benmccann force-pushed the scriptable-tick-fonts branch from 1808cd1 to 74738ad Compare January 12, 2020 22:30
@benmccann benmccann requested a review from etimberg January 12, 2020 22:40
@benmccann
Copy link
Contributor Author

@kurkle would you be able to give this a look? thanks!

@etimberg etimberg merged commit 738ee34 into chartjs:master Jan 14, 2020
@etimberg etimberg added this to the Version 3.0 milestone Jan 14, 2020
kurkle added a commit to kurkle/Chart.js that referenced this pull request Jan 21, 2020
@kurkle
Revert "Scriptable tick fonts (chartjs#6939)"
95e29aa 
This reverts commit 738ee34.
kurkle added a commit to kurkle/Chart.js that referenced this pull request Jan 27, 2020
@kurkle
Revert "Scriptable tick fonts (chartjs#6939)"
6ad2362 
This reverts commit 738ee34.
@benmccann benmccann mentioned this pull request Jan 31, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kurkle kurkle kurkle approved these changes

@etimberg etimberg etimberg approved these changes

Assignees
No one assigned
Labels
type: enhancement
Projects
None yet
Milestone
Version 3.0
Development

Successfully merging this pull request may close these issues.

Add possibility to change style for each of the ticks
3 participants
@benmccann @kurkle @etimberg