Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(custom): fix user-defined info property was not available in the event handler #18400

Merged
merged 3 commits into from
Mar 23, 2023

Conversation

sobolewsk
Copy link
Contributor

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Fixes #15789

Fixed issues

  • 15789

Details

Before: What was the problem?

The info field (user defined data) was not available in the event handler.

After: How does it behave after the fixing?

The info field is now properly passed to the event handlers. To verify please use the test/custom-feature.html test.

Document Info

One of the following should be checked.

  • This PR doesn't relate to document changes
  • The document should be updated later
  • The document changes have been made in apache/echarts-doc#xxx

Misc

ZRender Changes

  • This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

N.A.

Others

Merging options

  • Please squash the commits into a single one when merging.

Other information

Copy link
Contributor

@Ovilia Ovilia left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Please add a test case.

@sobolewsk
Copy link
Contributor Author

@Ovilia it seems that the test case is already there: https://github.com/sobolewsk/echarts/blob/branches/fix-issue-15789/test/custom-feature.html#L124
However, it seems to be a manual test and I was having a problem trying to figure out how to test interactions like that in your Jest environment. Could you please point me to an example of a unit test which triggers click events?

@sobolewsk
Copy link
Contributor Author

NVM, I figured it out. Unit test added.

Copy link
Contributor

@Ovilia Ovilia left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. Only a small problem needs to be fixed.

@@ -69,4 +69,46 @@ describe('custom_series', function () {
expect(resultPaletteColors).toEqual(colors);
});

it('should pass user defined data to event handlers', async () => {
const data = [
[10, 16,],
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra comma here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing. Fixed!

@Ovilia Ovilia merged commit b84493a into apache:master Mar 23, 2023
@Ovilia Ovilia added this to the 5.5.0 milestone Mar 23, 2023
@sobolewsk
Copy link
Contributor Author

Thank you @Ovilia for reviewing and merging!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The series-custom.renderItem.return_rect.info look like can't work!
3 participants