Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/chart/line/LineSeries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ export interface LineSeriesOption extends SeriesOption<LineStateOption<CallbackD
data?: (LineDataValue | LineDataItemOption)[]

triggerLineEvent?: boolean

triggerLineOnlyEvent?: boolean
}

class LineSeriesModel extends SeriesModel<LineSeriesOption> {
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Member

@plainheart plainheart Jun 15, 2025

Choose a reason for hiding this comment

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

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

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.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that triggerEvent is a better idea than triggerLineOnlyEvent. It would also be nice to have a feature that developers can decide whether to blur other series when hovering on the line or the line area. Would this triggerEvent also be helpful if we decide to implement such a feature in the future?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would also be nice to have a feature that developers can decide whether to blur other series when hovering on the line or the line area.

It sounds like emphasis.disabled can do that.

Copy link
Copy Markdown
Contributor Author

@sjcobb sjcobb Jun 26, 2025

Choose a reason for hiding this comment

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

This makes sense, I made the change to support triggerEvent and deprecate triggerLineEvent. As far as more control over symbol event, for my use case it's not required. When symbols are visible, we always want the event to fire (and the showSymbol, showAllSymbol options give enough flexibility already).

Also not a strong opinion since it would be a breaking change but given that v6.0 is about to release, would it make sense for triggerEvent: 'line' or triggerEvent: true to be the default instead of triggerEvent: false?

};

getLegendIcon(opt: LegendIconParams): ECSymbol | Group {
Expand Down
4 changes: 4 additions & 0 deletions src/chart/line/LineView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,10 @@ class LineView extends ChartView {
this.packEventData(seriesModel, polyline);
polygon && this.packEventData(seriesModel, polygon);
}

if (seriesModel.get('triggerLineOnlyEvent')) {
this.packEventData(seriesModel, polyline);
}
}

private packEventData(seriesModel: LineSeriesModel, el: Element) {
Expand Down
11 changes: 11 additions & 0 deletions test/polyline-gon-event.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.