Skip to content
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

[Request] Modify amCharts XYChart to Allow Page Scrolling When Inner Scroll Reaches Limit #1776

Open
denniscalazans opened this issue Oct 26, 2024 · 1 comment
Labels
enhancement Feature request in next release Implemented. Available soon.

Comments

@denniscalazans
Copy link

denniscalazans commented Oct 26, 2024

Description

Currently, the amCharts XYChart does not allow the page to continue scrolling when the inner scroll reaches its limit.
I'm not sure if that is a bug or a feature.

The desired behavior is to ensure that the page continues scrolling when the inner scroll of the XYChart reaches its limit.
Would that be possible?

Steps to Reproduce

  1. Create page which document generates a scroll bar, for example 3000px tall
  2. Create an XYChart with horizontal and-or vertical scrollbars.
  3. Scroll the chart to its limit using the mouse wheel.
  4. Observe that the page does not continue scrolling when the inner scroll reaches its limit.

Relevant Code

The relevant code can be found in src/.internal/charts/xy/XYChart.ts, specifically lines 626-701.
The handleWheel method needs to be modified to detect when the inner scroll is over and allow the page to continue scrolling.

Additional Context

To detect when the inner scroll is over, the rangechanged event of the Scrollbar can be used.
This event is triggered whenever the scroll range changes.
The start and end properties of the Scrollbar can be checked to determine if the scroll is at the beginning or end of the range.
Implement logic to handle the scroll behavior when the inner scroll is over.

I believe that the following lines should be run conditionally

// Ignore wheel event if it is happening on a non-chart element, e.g. if
// some page element is over the chart.
if ($utils.isLocalEvent(wheelEvent, this)) {
wheelEvent.preventDefault();
}
else {
return;
}

Perhaps they should only be applied in those situations:

if ((wheelX === "zoomY" || wheelX === "zoomXY") && shiftX != 0) {
this.yAxes.each((axis) => {
if (axis.get("zoomY")) {
let start = axis.get("start")!;
let end = axis.get("end")!;
let position = axis.fixPosition(plotPoint.y / plotContainer.height());
if (wheelZoomPositionY != null) {
position = wheelZoomPositionY;
}
let maxDeviation = axis.get("maxDeviation", 0);
let newStart = Math.min(1 + maxDeviation, Math.max(-maxDeviation, start - wheelStep * (end - start) * shiftX * position));
let newEnd = Math.max(-maxDeviation, Math.min(1 + maxDeviation, end + wheelStep * (end - start) * shiftX * (1 - position)));
if (1 / (newEnd - newStart) < axis.getPrivate("maxZoomFactor", Infinity) / axis.get("minZoomCount", 1)) {
this._handleWheelAnimation(axis.zoom(newStart, newEnd));
}
}
})
}
if ((wheelY === "zoomY" || wheelY === "zoomXY") && shiftY != 0) {
this.yAxes.each((axis) => {
if (axis.get("zoomY")) {
let start = axis.get("start")!;
let end = axis.get("end")!;
let position = axis.fixPosition(plotPoint.y / plotContainer.height());
if (wheelZoomPositionY != null) {
position = wheelZoomPositionY;
}
let maxDeviation = axis.get("maxDeviation", 0);
let newStart = Math.min(1 + maxDeviation, Math.max(-maxDeviation, start - wheelStep * (end - start) * shiftY * position));
let newEnd = Math.max(-maxDeviation, Math.min(1 + maxDeviation, end + wheelStep * (end - start) * shiftY * (1 - position)));
if (1 / (newEnd - newStart) < axis.getPrivate("maxZoomFactor", Infinity) / axis.get("minZoomCount", 1)) {
this._handleWheelAnimation(axis.zoom(newStart, newEnd));
}
}
})
}
if ((wheelX === "panX" || wheelX === "panXY") && shiftX != 0) {
this.xAxes.each((axis) => {
if (axis.get("panX")) {
let start = axis.get("start")!;
let end = axis.get("end")!;
let delta = this._getWheelSign(axis) * wheelStep * (end - start) * shiftX;
let newStart = start + delta;
let newEnd = end + delta;
let se = this._fixWheel(newStart, newEnd);
newStart = se[0];
newEnd = se[1];
this._handleWheelAnimation(axis.zoom(newStart, newEnd));
}
})
}
if ((wheelY === "panX" || wheelY === "panXY") && shiftY != 0) {
this.xAxes.each((axis) => {
if (axis.get("panX")) {
let start = axis.get("start")!;
let end = axis.get("end")!;
let delta = this._getWheelSign(axis) * wheelStep * (end - start) * shiftY;
let newStart = start + delta;
let newEnd = end + delta;
let se = this._fixWheel(newStart, newEnd);
newStart = se[0];
newEnd = se[1];
this._handleWheelAnimation(axis.zoom(newStart, newEnd));
}
})
}
if ((wheelX === "panY" || wheelX === "panXY") && shiftX != 0) {
this.yAxes.each((axis) => {
if (axis.get("panY")) {
let start = axis.get("start")!;
let end = axis.get("end")!;
let delta = this._getWheelSign(axis) * wheelStep * (end - start) * shiftX;
let newStart = start + delta;
let newEnd = end + delta;
let se = this._fixWheel(newStart, newEnd);
newStart = se[0];
newEnd = se[1];
this._handleWheelAnimation(axis.zoom(newStart, newEnd));
}
})
}
if ((wheelY === "panY" || wheelY === "panXY") && shiftY != 0) {
this.yAxes.each((axis) => {
if (axis.get("panY")) {
let start = axis.get("start")!;
let end = axis.get("end")!;
let delta = this._getWheelSign(axis) * wheelStep * (end - start) * shiftY;
let newStart = start - delta;
let newEnd = end - delta;
let se = this._fixWheel(newStart, newEnd);
newStart = se[0];
newEnd = se[1];
this._handleWheelAnimation(axis.zoom(newStart, newEnd));
}
})
}

But I'm not sure if that's the right way to do it.

"@amcharts/amcharts5-fonts": "^5.0.1",
Chrome - Version 130.0.6723.70 (Official Build) (arm64)

Considerations

Control + scroll is not what is needed.

Listening for "wheel" events on xyChart.plotContainer is possible,
Checking if the inner scroll has reached its limit can be done.

However, setting "wheelX" back to "none" does not resolve the issue of the event trap.

@denniscalazans
Copy link
Author

I opened a Draft PR pointing to where I believe the improvement could be done.
https://github.dev/amcharts/amcharts5/pull/1775/

@martynasma martynasma added the enhancement Feature request label Oct 28, 2024
@martynasma martynasma added the in next release Implemented. Available soon. label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request in next release Implemented. Available soon.
Projects
None yet
Development

No branches or pull requests

2 participants