-
-
Notifications
You must be signed in to change notification settings - Fork 81
[HxEChart] Added OnAxisPointerUpdated to HxEchart #1109
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
Conversation
@crdo I've disscused this with Alex and ended up deciding to use double for the eventcallback. As javascript's number value type corresponds to c# double. |
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.
Pull Request Overview
This PR adds support for axis pointer update events to the HxEChart component, enabling external labels to display values when users hover over the chart. This functionality is required for a specific project requirement.
Key changes:
- Added OnAxisPointerUpdated event callback parameter to capture axis pointer movements
- Implemented JavaScript event handler with debouncing to handle 'updateAxisPointer' events
- Added C# method to invoke the event callback with the extracted data value
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
HxEChart.js | Added axis pointer event subscription and handler with timeout debouncing |
HxEChart.razor.cs | Added OnAxisPointerUpdated parameter and corresponding JSInvokable handler method |
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's unclear what the
double
refers to here. As a component consumer, I can only assume it relates to theValue
. Does this make sense for all chart types whereupdateAxisPointer
is triggered? - We're discarding all the details provided by the original
updateAxisPointer
event (see apache/echarts-doc#230).
...we should use EChartAxisPointerUpdatedEventArgs
, similar to how EChartClickEventArgs
is used for the OnClick
event. Resolving the Value
from the event details is likely not the component's responsibility, but rather the caller's or something handled via an extension or helper method.
(The implementation proposed is appropriate simplification for a project-specific usage. Unfortunately, we decided to have HxEChart
as an universal component and for such case we have to provide a more sophisticated solution.)
Maybe i should not include value in the EChartAxisPointerUpdatedEventArgs. And the user of this callback would need to find the value from his data based on DataIndex and SeriesIndex. Since by default AxisPointerUpdate does not have value and i need to do a custom js logic to find said Value. Would appreciate your feedback. |
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.
TBH, I'm not getting the idea behind List<EChartAxisPointerUpdatedEventArgs>
.
According to apache/echarts-doc#230, the event itself holds this shape of data (does it?):
{
"type": "updateaxispointer",
"seriesIndex": 0,
"dataIndexInside": 37,
"dataIndex": 336,
"axesInfo": [
{
"axisDim": "x",
"axisIndex": 0,
"value": 336
},
{
"axisDim": "y",
"axisIndex": 0,
"value": 5188.092254685122
},
{
"axisDim": "x",
"axisIndex": 1,
"value": 336
}
]
}
Which means we should produce something like
EventCallback<EChartAxisPointerUpdatedEventArgs>
:
public record EChartAxisPointerUpdatedEventArgs
{
public int SeriesIndex { get; set; }
public int DataIndexInside { get; set; }
public int DataIndex { get; set; }
public List<EChartAxisPointerUpdatedAxisInfo> AxesInfo { get; } = new();
}
public record EChartAxisPointerUpdatedAxisInfo
{
public string AxisDim { get; set; }
public int AxisIndex { get; set; }
public object Value { get; set; }
public T GetValue<T>()
}
The Value
is already present in the event data or is it another value?
If it is another value than we need, I would prefer the consumer code or extension method to resolve the value in Blazor rather than the current JS approach (which seems a little scenario-bound).
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.
see comments, adjusted on my own
Added support for OnAxisPointerUpdated to HxEchart. In order to support external label showing the value in the graph. Needed for a project that uses this Project.