plainheart commented on code in PR #21001:
URL: https://github.com/apache/echarts/pull/21001#discussion_r2147965904
##########
src/chart/line/LineSeries.ts:
##########
@@ -213,7 +215,11 @@ class LineSeriesModel extends
SeriesModel<LineSeriesOption> {
divideShape: 'clone'
},
- triggerLineEvent: false
+ // Whether to trigger event when hovering on either line or area.
+ triggerLineEvent: false,
+
+ // Whether to trigger event when hovering only on line.
+ triggerLineOnlyEvent: false
Review Comment:
To be honest, `triggerLineOnlyEvent` is more confusing. I don't know what it
means until I see this comment. If we want to refine the `triggerEvent`
behavior to split it into the line event and area event, I would suggest
introducing a new option `triggerEvent` - the `triggerEvent` option is used
repeatedly. For instance, `title.triggerEvent`, `xAxis.triggerEvent`, etc.
So I think we can merge both options into a single option `triggerEvent`
with the boolean type and constant string `'line'/'area'` allowed.
For backward compatibility, the new option `triggerEvent` should work with
both line and area unless it is explicitly specified to a single target.
The changes look like this,
**_src/chart/line/LineSeries.ts_**
```ts
export interface LineSeriesOption extends
SeriesOption<LineStateOption<CallbackDataParams>, LineStateOptionMixin & {
// ...
/**
* @deprecated Use `triggerEvent: 'line'` for only line event or
`triggerEvent: true` for both line and area event.
*/
triggerLineEvent?: boolean
/**
* Whether to trigger event when hovering on the line or the area
* @since v6.0.0
*/
triggerEvent?: boolean | 'line' | 'area'
}
class LineSeriesModel extends SeriesModel<LineSeriesOption> {
// ...
/**
* @deprecated
*/
triggerLineEvent: false,
triggerEvent: false
};
```
**_src/chart/line/LineView.ts_**
Note that `triggerLineEvent` has higher priority than `triggerEvent` for
compatibility.
```ts
const triggerEvent = seriesModel.get('triggerEvent');
const triggerLineEvent = seriesModel.get('triggerLineEvent');
const shouldTriggerLineEvent = triggerLineEvent === true || triggerEvent ===
true || triggerEvent === 'line';
const shouldTriggerAreaEvent = triggerLineEvent === true || triggerEvent ===
true || triggerEvent === 'area';
shouldTriggerLineEvent && this.packEventData(seriesModel, polyline);
shouldTriggerAreaEvent && polygon && this.packEventData(seriesModel,
polygon);
```
But `triggerEvent` does not control whether to trigger the symbol event,
which is triggered by default. This may be a confusing point, so it's better to
add some explanation. I would like to hear what you think about this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
