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

[DTRA] Maryia/feat: vertical scroll behavior updates for isVerticalScrollEnabled functionality when disabled #1628

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ Props marked with `*` are **mandatory**:
| isConnectionOpened | Sets the connection status. If set, upon reconnection smartcharts will either patch missing tick data or refresh the chart, depending on granularity; if not set, it is assumed that connection is always opened. Defaults to `undefined`. |
| onMessage | SmartCharts will send notifications via this callback, should it be provided. Each notification will have the following structure: `{ text, type, category }`. |
| isAnimationEnabled | Determine whether chart animation is enabled or disabled. It may needs to be disabled for better performance. Defaults to `true`. |
| isVerticalScrollEnabled | Determine whether verticall scroll on the chart outside Y-axis is disabled. It may need to be disabled for mobile app version to scroll the page up or down instead of the chart. Defaults to `true`. |
| isVerticalScrollEnabled | Determine whether verticall scroll on the chart outside Y-axis is disabled while it is forced on the nearest scrollable parent instead. It may need to be disabled for mobile app version to scroll the page up or down instead of the chart. In this case, when scroll delta exceeds 10px, the page will be force-scrolled fully in a respective direction. Defaults to `true`. |
| showLastDigitStats | Shows last digits stats. Defaults to `false`. |
| scrollToEpoch | Scrolls the chart to the leftmost side and sets the last spot/bar as the first visible spot/bar in the chart. Also, it disables scrolling until the chart reaches the 3/4 of the width of the main pane of the chart. Defaults to `null`. |
|
Expand Down
58 changes: 41 additions & 17 deletions src/store/ChartAdapterStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,14 @@ export default class ChartAdapterStore {
bottomIndex: 0,
};
touchValues: {
deltaYTotal?: number;
x?: number;
y?: number;
yOnTouchEnd?: number;
} = {};
} = {
deltaYTotal: 0,
x: 0,
y: 0,
};

isOverFlutterCharts = false;
enableVerticalScrollTimer?: ReturnType<typeof setTimeout>;
Expand Down Expand Up @@ -237,38 +241,58 @@ export default class ChartAdapterStore {
}

onTouch(e: TouchEvent) {
const chartNode = this.mainStore.chart.chartNode;
// Prevent vertical scroll on the chart for touch devices by forcing scroll on a scrollable parent of the chart:
const chartNode = this.mainStore.chart.chartNode;
if (
chartNode &&
this.scrollableChartParent &&
!this.mainStore.state.isVerticalScrollEnabled &&
e.touches.length === 1
e.changedTouches.length === 1
) {
const { pageX, screenX, screenY } = e.touches[0];
if (['touchstart', 'touchend'].includes(e.type)) {
this.touchValues = e.type === 'touchstart' ? { x: screenX, y: screenY } : { yOnTouchEnd: screenY };
} else if (e.type === 'touchmove') {
const { pageX, pageY } = e.changedTouches[0];

if (['touchmove', 'touchend'].includes(e.type)) {
const nonScrollableAreaWidth = chartNode.offsetWidth - this.mainStore.chart.yAxisWidth;
const { left } = chartNode.getBoundingClientRect();

if (this.touchValues.x && this.touchValues.y) {
const deltaX = Math.abs(screenX - this.touchValues.x);
const deltaY = Math.abs(screenY - this.touchValues.y);
const deltaX = Math.abs(pageX - this.touchValues.x);
const deltaY = Math.abs(pageY - this.touchValues.y);
this.touchValues.deltaYTotal = (this.touchValues.deltaYTotal ?? 0) + (this.touchValues.y - pageY);
const isVerticalScroll = deltaY > deltaX;
const x = pageX - left;
if (x < nonScrollableAreaWidth && isVerticalScroll && !this.scrollChartParentOnTouchTimer) {
this.touchValues.yOnTouchEnd = undefined;
this.scrollChartParentOnTouchTimer = setTimeout(() => {
this.scrollableChartParent?.scrollBy({
top: screenY - Number(this.touchValues.yOnTouchEnd ?? this.touchValues.y),
const isForcedScrollArea = x < nonScrollableAreaWidth;
const shouldForceMaxScroll =
Math.abs(Number(this.touchValues.deltaYTotal)) > 10 && e.type === 'touchend';
Copy link
Contributor

Choose a reason for hiding this comment

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

Sometimes we get touchcancel instead of touchend, do we need to handle this case too?

Copy link
Contributor Author

@maryia-deriv maryia-deriv Aug 12, 2024

Choose a reason for hiding this comment

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

no, we don't need to do that. also, we won't get touchcancel here ever because we don't trigger onTouch action for this event type.

if (isForcedScrollArea) {
if (shouldForceMaxScroll) {
clearTimeout(this.scrollChartParentOnTouchTimer);
this.scrollableChartParent?.scrollTo({
top:
Number(this.touchValues.deltaYTotal) < 0
? 0
: this.scrollableChartParent.scrollHeight,
behavior: 'smooth',
});
this.scrollChartParentOnTouchTimer = undefined;
}, 300);
this.touchValues = { ...this.touchValues, deltaYTotal: 0 };
} else if (isVerticalScroll && !this.scrollChartParentOnTouchTimer) {
this.scrollChartParentOnTouchTimer = setTimeout(() => {
this.scrollableChartParent?.scrollBy({
top: this.touchValues.deltaYTotal,
behavior: 'smooth',
});
this.scrollChartParentOnTouchTimer = undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Inside of timeout timer can be cleared only by undefined assignment or this will work too:⤵️? clearTimeout(this.scrollChartParentOnTouchTimer);

Copy link
Contributor Author

@maryia-deriv maryia-deriv Aug 12, 2024

Choose a reason for hiding this comment

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

we don't want to clear the timer inside it because it has already been executed, it won't be needed.
the purpose of this.scrollChartParentOnTouchTimer = undefined; (id removal) is to ensure that we initiate the next setTimeout execution only after the current one has finished execution (not let the if condition be satisfied).

this.touchValues = { ...this.touchValues, deltaYTotal: 0 };
}, 100);
}
}
}
this.touchValues = { x: screenX, y: screenY };
this.touchValues = { ...this.touchValues, x: pageX, y: pageY };
}
if (['touchstart', 'touchend'].includes(e.type)) {
this.touchValues =
e.type === 'touchstart' ? { x: pageX, y: pageY } : { deltaYTotal: this.touchValues.deltaYTotal };
}
}
}
Expand Down
Loading