-
Notifications
You must be signed in to change notification settings - Fork 19.8k
fix(matrix): trigger click event on matrix cells #21390
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
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for your contribution! Please DO NOT commit the files in dist, i18n, and ssr/client/dist folders in a non-release pull request. These folders are for release use only. |
Ovilia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Matrix is a layout so its text in the header or body should not trigger 'click' event by default. If you need this information, you need to add a new option matrix.triggerEvent (default false) like https://echarts.apache.org/en/option.html#yAxis.triggerEvent.
|
@Ovilia I've updated the implementation to add the triggerEvent option (default: false) following the same pattern as I would appreciate it if you could take another look. |
Ovilia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add test cases in test/matrix.html.
src/component/matrix/MatrixView.ts
Outdated
| name: textValue != null ? textValue + '' : null, | ||
| coord: xyLocator.slice() | ||
| }; | ||
| getECData(cellRect).eventData = eventData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that events only trigger when there is text, correct me if I'm wrong. So what's the meaning of setting eventData of cellRect here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brief Information
This pull request is in the type of:
What does this PR do?
Fixes an issue where click events were not triggered on matrix component cells.
Fixed issues
Details
Before: What was the problem?
Even when
matrix.body.silentwas set tofalse(and the cell was made hit-testable, e.g., by settingitemStyle.colorto'transparent'), click events on matrix cells were not being emitted by the ECharts instance. This was because the underlying graphic elements (rects and texts) created inMatrixViewlacked the necessaryeventData(ECData) to identify them as part of the matrix component.After: How does it behave after the fixing?
Click events are now correctly triggered when clicking on matrix cells. The event parameters include
componentType: 'matrix',componentIndex, and the cell'sname.This is achieved by:
getECDatainsrc/component/matrix/MatrixView.ts.eventDatato the cell's text elements during creation increateMatrixCell.events-on-matrix-cell-text.mov
Document Info
One of the following should be checked.
Misc
Security Checking
ZRender Changes
Related test cases or examples to use the new APIs
Added a new test case in
test/ut/spec/component/matrix/event.test.tsto verify the fix.Merging options
Other information