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

Pass event instance to listeners #451

Merged
merged 6 commits into from
Aug 17, 2021
Merged

Pass event instance to listeners #451

merged 6 commits into from
Aug 17, 2021

Conversation

stockiNail
Copy link
Collaborator

Currently the listeners on the annotations are receiving only the context.

I think it could be helpful to get also the event instance, separately, as additional argument.

A use case where to use the event could be to check if any modifier key is pressed in order to perform own logic only when a key is pressed.

@kurkle
Copy link
Member

kurkle commented Aug 17, 2021

Why not set the event in the context? also I'd pass the ChartEvent, because it contains the canvas coordinates and provides the native event inside.

@stockiNail
Copy link
Collaborator Author

stockiNail commented Aug 17, 2021

Why not set the event in the context?

I thought that in the near future the context in the plugin will be "persistent" as in chartjs and in datalabels plugin in order to store own data and to get them in further calls.
In this case it couldn't make sense to add it to the context because you could have a property related to an obsolete object (last managed event). Of course, I can change it and add it to the context. Let me know.

also I'd pass the ChartEvent, because it contains the canvas coordinates and provides the native event inside.

Yesterday I used the ChartEvent but today I recognized that the types were not correct because I set to Event.
Of course, I'll change in next minutes.

@kurkle
Copy link
Member

kurkle commented Aug 17, 2021

I thought that in the near future the context in the plugin will be "persistent" as in chartjs and in datalabels plugin in order to store own data and to get them in further calls.
In this case it couldn't make sense to add it to the context because you could have a property related to an obsolete object (last managed event). Of course, I can change it and add it to the context. Let me know.

Makes sense

@kurkle kurkle added this to the 1.1.0 milestone Aug 17, 2021
@kurkle kurkle changed the title Passes event instance to listeners Pass event instance to listeners Aug 17, 2021
@kurkle kurkle merged commit 449eeb4 into chartjs:master Aug 17, 2021
@stockiNail stockiNail deleted the eventToListeners branch August 17, 2021 09:01
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.

2 participants