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

Apply scale context to ticks scriptable options instead of chart context #8839

Merged
merged 5 commits into from Apr 7, 2021
Merged

Apply scale context to ticks scriptable options instead of chart context #8839

merged 5 commits into from Apr 7, 2021

Conversation

stockiNail
Copy link
Contributor

Fixes #8835

@kurkle
Copy link
Member

kurkle commented Apr 7, 2021

It might be better to set the context already here:

me.options = options;

I'd think it would prevent the mock tests failing too. Not sure if it causes any other failures though.

@stockiNail
Copy link
Contributor Author

It might be better to set the context already here:

me.options = options;

I'd think it would prevent the mock tests failing too. Not sure if it causes any other failures though.

I haven't seen it. I have fixed changed the test cases.

Anyway let me try your hint.

@stockiNail
Copy link
Contributor Author

@kurkle I think your hint is to set the scale context to options.ticks during the scale init.

If yes, are you thinking something like the following?

  init(options) {
    const me = this;
    me.options = options;
    me.options.ticks = options.ticks.setContext(me.getContext());

@kurkle
Copy link
Member

kurkle commented Apr 7, 2021

@kurkle I think your hint is to set the scale context to options.ticks during the scale init.

If yes, are you thinking something like the following?

  init(options) {
    const me = this;
    me.options = options;
    me.options.ticks = options.ticks.setContext(me.getContext());

Actually I was thinking for the whole scale options. This should be the only change required.

  init(options) {
    const me = this;
    me.options = options.setContext(me.getContext());

@stockiNail
Copy link
Contributor Author

Actually I was thinking for the whole scale options. This should be the only change required.

  init(options) {
    const me = this;
    me.options = options.setContext(me.getContext());

Done as @kurkle suggested and sounds working. Thanks a lot!

@etimberg etimberg merged commit 7ae1064 into chartjs:master Apr 7, 2021
@etimberg etimberg added this to the Version 3.1 milestone Apr 7, 2021
@stockiNail stockiNail deleted the fix8835 branch April 7, 2021 21:54
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.

Context for some ticks scriptable options
3 participants