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

Legend onClick callback supports arrow functions #7410

Merged
merged 4 commits into from May 24, 2020
Merged

Legend onClick callback supports arrow functions #7410

merged 4 commits into from May 24, 2020

Conversation

etimberg
Copy link
Member

Work Done

  • Legend onClick, onHover, and onLeave callbacks receive the legend as a 3rd parameter
  • Legend onClick, onHover, and onLeave callbacks receive the wrapped event as the 1st parameter
  • Chart level onClick and onHover callbacks receive the chart as a 3rd parameter
  • Chart level onClick and onHover callbacks receive the chart as a 3rd parameter

Resolves #7409

@etimberg
chartjs#7409 Legend callbacks support arrow functions
2b8425f 
The `Legend` is no longer implicitly found through `this` and instead
passed as an explicit argument to the callback functions.
@etimberg
Chart level onClick and onHover handlers receive chart argument
96d7a39
@etimberg etimberg added this to the Version 3.0 milestone May 23, 2020
@etimberg etimberg changed the title Legend on click arrow Legend onClick callback supports arrow functions May 23, 2020
@etimberg
Copy link
Member Author

@kurkle @benmccann I'm not sure the best way to document these callbacks. I've been doing a lot of typescript lately and am tempted to try typing out parts of our code. The type signature for the legend callbacks would be (event: chartEvent, legendItem: LegendItem, legend: Legend) => void;

benmccann
benmccann reviewed May 23, 2020
View reviewed changes
Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

lgtm. just one comment

I haven't used typescript enough to have much opinion on it, but I lean towards being favorable on it. I'm just starting my first project where I'm trying to write it all in TypeScript, but just started setting it up yesterday

src/plugins/plugin.legend.js Outdated Show resolved Hide resolved
kurkle
kurkle approved these changes May 24, 2020
View reviewed changes
Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

Feels like our events should always have the chart instance as a parameter. But I don't think we want to alter the callback signatures that much.
lgtm.

@etimberg etimberg merged commit 13b8924 into chartjs:master May 24, 2020
etimberg added a commit that referenced this pull request Sep 1, 2020
@etimberg
Legend onClick callback supports arrow functions (#7410)
0e9def6 
#7409 Legend callbacks support arrow functions
The `Legend` is no longer implicitly found through `this` and instead
passed as an explicit argument to the callback functions.
@etimberg etimberg deleted the legend-on-click-arrow branch October 17, 2020 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benmccann benmccann benmccann approved these changes

@kurkle kurkle kurkle approved these changes

Assignees
No one assigned
Labels
breaking change type: bug
Projects
None yet
Milestone
Version 3.0
Development

Successfully merging this pull request may close these issues.

Legend onClick option does not support arrow functions
3 participants
@etimberg @benmccann @kurkle