Skip to content

Chart - ScrollBar: the thumb jumps to the beginning of the bar when the argument axis has scale breaks - #35210

Open
dmlvr wants to merge 6 commits into
DevExpress:mainfrom
dmlvr:chart_scrollbar_internal_bug_26_2
Open

dmlvr wants to merge 6 commits into
DevExpress:mainfrom
dmlvr:chart_scrollbar_internal_bug_26_2

Conversation

@dmlvr

@dmlvr dmlvr commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@dmlvr dmlvr self-assigned this Sep 15, 2026
@dmlvr dmlvr added the 26_2 label Sep 15, 2026
@dmlvr dmlvr changed the title Chart scrollbar internal bug 26 2 Chart - ScrollBar: the thumb jumps to the beginning of the bar when the argument axis has scale breaks Sep 15, 2026
@dmlvr
dmlvr force-pushed the chart_scrollbar_internal_bug_26_2 branch 2 times, most recently from 78bb493 to b9167f6 Compare September 16, 2026 09:21
@dmlvr
dmlvr requested a lite review from Copilot September 16, 2026 12:54
@dmlvr
dmlvr marked this pull request as ready for review September 16, 2026 12:54
@dmlvr
dmlvr requested a review from a team as a code owner September 16, 2026 12:54

Copilot AI left a comment

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.

🟡 Changes recommended

Unresolved moderate issues affect datetime intervals, inverted/rotated ranges, and scrollbar break translation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes chart scrollbar positioning and panning across argument-axis scale breaks.

Changes:

  • Adds break-aware range and panning corrections.
  • Updates scrollbar translation and drag handling.
  • Extends mocks and regression/integration tests.
File summaries
File Summary
packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js Adds panning regression coverage; the new path is not exercised by the synthetic event pair.
packages/devextreme/testing/tests/DevExpress.viz.charts/scrollBar.tests.js Updates scrollbar initialization tests.
packages/devextreme/testing/tests/DevExpress.viz.charts/chart.part5.tests.js Updates scrollbar mock expectations.
packages/devextreme/testing/tests/DevExpress.viz.charts/chart.integration.tests.js Adds scrollbar scale-break integration tests.
packages/devextreme/testing/helpers/chartMocks.js Extends axis mock support.
packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts Applies corrected ranges during panning.
packages/devextreme/js/__internal/viz/chart_components/scroll_bar.ts Maps scrollbar positions to ranges; moderate issues remain for data ordering and break widths.
packages/devextreme/js/__internal/viz/chart_components/base_chart.ts Passes whole-range breaks to the scrollbar.
packages/devextreme/js/__internal/viz/axes/base_axis.ts Adds break-aware calculations; moderate issues remain for numeric datetime intervals and inverted ranges.
Review details

Suppressed comments (3)

packages/devextreme/js/__internal/viz/axes/base_axis.ts:1307

  • Datetime TimeIntervalConfig accepts numeric millisecond intervals, and forceUserTickInterval preserves such a number in _tickInterval. dateUtils.dateToMilliseconds(number) returns 0, so these new range-length calculations treat a user break as fully hidden instead of retaining one tick interval; the scrollbar thumb and panning scale are incorrect for datetime axes with a numeric tickInterval. Return numeric intervals unchanged and only convert interval objects.
    return this._options.dataType === 'datetime' ? dateUtils.dateToMilliseconds(tickInterval) : tickInterval;

packages/devextreme/js/__internal/viz/axes/base_axis.ts:1371

  • adjustPannedRange receives zoom.min/zoom.max directly, but Translator2D.zoom() returns them in screen order for an inverted continuous axis, so startValue can be greater than endValue. _getBreaksForRange does not normalize viewport bounds and _filterBreaks then finds no breaks, causing this early return to skip the scale-preserving correction. Panning or dragging the scrollbar across a break on an inverted axis can therefore change the visible duration; normalize the range before break detection and correction.
      || !this._getBreaksForRange(range.startValue, range.endValue).length) {

packages/devextreme/js/__internal/viz/chart_components/scroll_bar.ts:160

  • wholeRangeBreaks carries cumulativeWidth values computed from the axis breakStyle.width, but this translator is configured with breaksSize: 0. Translator2D still uses cumulativeWidth when calculating its ratio and converting positions, so user-defined breaks leave a gap in the scrollbar track and make the thumb length differ from the axis visible-range fraction. Clear or recompute cumulativeWidth for the scrollbar-specific break list before passing it to the translator.
      breaks: wholeRangeBreaks?.length ? wholeRangeBreaks : null,
  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/devextreme/js/__internal/viz/chart_components/scroll_bar.ts Outdated
Comment thread packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js Outdated
Copilot AI review requested due to automatic review settings September 16, 2026 14:51
@dmlvr
dmlvr force-pushed the chart_scrollbar_internal_bug_26_2 branch from 11c5aee to fd1e450 Compare September 16, 2026 15:04

Copilot AI left a comment

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.

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

packages/devextreme/js/__internal/viz/axes/base_axis.ts:1377

  • This correction is skipped as soon as the newly computed range no longer contains a break. If a pan starts with a break in the viewport and then moves past that break, the early return leaves the data span changed, so the chart zooms out/in instead of preserving the original visible scale. The condition should also consider breaks in storedParams.startRange (or run the correction for every continuous range whose starting range had a break).
    if (!storedParams || type === constants.discrete || type === constants.logarithmic
      || !this._getBreaksForRange(range.startValue, range.endValue).length) {
      return range;

packages/devextreme/js/__internal/viz/chart_components/scroll_bar.ts:119

  • This ordering check is not valid for discrete axes: from() returns categories in axis order, but > compares the category values themselves. For example, a non-inverted axis with categories ['B', 'A'] produces { startValue: 'A', endValue: 'B' }, and adjustRange does not normalize discrete ranges, so dragging the scrollbar can reverse the visible category range. Determine the order from the translator's inversion state (or the category indices), rather than comparing values.
    return from > to ? { startValue: to, endValue: from } : { startValue: from, endValue: to };

packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts:641

  • The end handler applies the explicit scrollbar range without checking whether argument-axis panning is enabled. Thus, even if the move branch is guarded, releasing a scrollbar drag still pans once for charts configured with argumentAxis: 'zoom' or none; retain the same zoomAndPan.options.argumentAxis.pan condition here.
            if (e.scrollRange) {
              panArgumentAxisToThumb(e, e.scrollRange);
            }
  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread packages/devextreme/js/__internal/viz/axes/base_axis.ts
Comment thread packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts Outdated
Copilot AI review requested due to automatic review settings September 16, 2026 15:06

Copilot AI left a comment

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.

🔵 Needs a closer look

Unresolved issues remain with category ordering, inverted-axis handling, and pan-option enforcement.

Review details

Suppressed comments (3)

packages/devextreme/js/__internal/viz/chart_components/scroll_bar.ts:119

  • from and to are category values for a discrete translator, so comparing them with > does not describe their order on the axis. For categories such as ['Z', 'A', 'M'], this reverses the range emitted by every real scrollbar drag and the new scrollRange path can pan to the wrong categories. Use the translator's screen/data direction to choose the endpoints instead of comparing category values.
    // on an inverted axis the bar runs against the data, so the ends are reported in data order
    return from > to ? { startValue: to, endValue: from } : { startValue: from, endValue: to };

packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts:276

  • The scrollbar range is normalized to data order above, but on an inverted axis the physical start of the thumb corresponds to scrollRange.endValue, not startValue. Passing the hard-coded 'start' anchor makes the scale-break correction preserve the wrong edge, so dragging an inverted scrollbar across a break can shift or resize the range instead of following the thumb. Select 'end' when the axis translator is inverted.
      const getRange = (axis) => axis.adjustRange(
        axis.adjustPannedRange(getVizRangeObject([scrollRange.startValue, scrollRange.endValue]), 'start'),
      );

packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts:272

  • This path now handles every real scrollbar drag, but it bypasses the options.argumentAxis.pan check used by the regular scrollbar path. When zoomAndPan.argumentAxis is "zoom" or "none", the start handler only marks the event as cancelled; the move handler still calls axisZoom for every argument axis and pans the chart. Guard this path with the pan option so dragging a scrollbar cannot enable panning that the option disables.
      const axes = getFilteredAxes(chart._argumentAxes);
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 16, 2026 15:32
@dmlvr
dmlvr force-pushed the chart_scrollbar_internal_bug_26_2 branch from fd1e450 to fd989bc Compare September 16, 2026 15:32

Copilot AI left a comment

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.

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

packages/devextreme/js/__internal/viz/axes/base_axis.ts:1325

  • This manual half-tick correction does not match the axis normalization: tick_generator snaps each user-break boundary to a nearby tick via getBaseTick before applying the ±half-interval correction, while this code shifts the raw boundaries directly. For breaks not aligned to the tick grid, the scrollbar hides a different interval from the chart axis, so thumb geometry and panning scale diverge. Reuse the normalized break values or centralize that normalization.
    return this._getBreaksForRange(businessRange.min, businessRange.max)
      .reduce((result, scaleBreak) => {
        const hidden = this._getHiddenDuration(scaleBreak, interval);
        // a gap is hidden whole, so its shift is zero and it is taken as is
        const shift = ((scaleBreak.to - scaleBreak.from) - hidden) / 2;
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts Outdated
Copilot AI review requested due to automatic review settings September 16, 2026 17:04
@dmlvr
dmlvr force-pushed the chart_scrollbar_internal_bug_26_2 branch from fd989bc to 5593d9d Compare September 16, 2026 17:04
@dmlvr
dmlvr force-pushed the chart_scrollbar_internal_bug_26_2 branch from 5593d9d to d436f6a Compare September 16, 2026 17:07

Copilot AI left a comment

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.

🟡 Changes recommended

A critical scrollbar endpoint conversion issue remains unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread packages/devextreme/js/__internal/viz/chart_components/scroll_bar.ts Outdated
Copilot AI review requested due to automatic review settings September 16, 2026 17:15

Copilot AI left a comment

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.

🔵 Needs a closer look

Three moderate issues remain in scrollbar endpoint mapping and drag handling or coverage.

Review details

Suppressed comments (3)

packages/devextreme/js/__internal/viz/chart_components/scroll_bar.ts:116

  • For a discrete argument axis, init passes stick: false when valueMarginsEnabled is at its default, so setPosition deliberately places the thumb's end at the boundary after the last visible category. Calling translator.from(start + length) with the default direction 0 converts that boundary to the next category (for example, a visible range [3, 7] becomes [3, 8]), expanding the range on every real scrollbar drag. Map each endpoint with the corresponding direction/offset used by setPosition (including the inverted case), and add a discrete scrollbar-drag regression test.
    const from = translator.from(start);
    const to = translator.from(start + length);

packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts:634

  • The new scrollRange branch is only covered by scrollbar tests using workdaysOnly. The user-defined-break cases exercise programmatic range changes and the ordinary chart pointer, but never drag the real scrollbar across argumentAxis.breaks; a regression in translating a user-defined whole-range break to scrollRange would therefore pass. Please add a direct scrollbar-drag case with a user-defined break and assert the range/scale after crossing it.
            if (e.scrollRange && options.argumentAxis.pan) {
              panArgumentAxisToThumb(e, e.scrollRange);

packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts:643

  • This remaps the axis from the scrollbar position a second time on every drag end, including a start/end gesture with no move. When the visual range boundary is inside a zero-width scale break, _getRangeAtPosition resolves that boundary to the value after the break, so simply clicking the thumb can change the chart range even though the thumb did not move. Apply the thumb range only for an actual move (or preserve the current range for a zero-offset end event).
            if (e.scrollRange && options.argumentAxis.pan) {
              panArgumentAxisToThumb(e, e.scrollRange);
            }
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 17, 2026 07:34

Copilot AI left a comment

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.

🟢 Approval recommended

The changes are covered by focused regression tests and no unresolved correctness issues were identified.

Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 18, 2026 11:26
@dmlvr
dmlvr force-pushed the chart_scrollbar_internal_bug_26_2 branch from 14ce35c to 7a137f6 Compare September 18, 2026 11:26

Copilot AI left a comment

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.

🟡 Changes recommended

Whole-range break calculations use raw option breaks instead of tick-normalized breaks, causing incorrect scrollbar sizing or panning for some numeric and non-aligned breaks.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread packages/devextreme/js/__internal/viz/axes/base_axis.ts
Copilot AI review requested due to automatic review settings September 18, 2026 14:21

Copilot AI left a comment

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.

🔵 Needs a closer look

Logarithmic argument-axis breaks are explicitly excluded from the new correction and scrollbar mapping.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

packages/devextreme/js/__internal/viz/axes/base_axis.ts:1360

  • The corresponding early return here also makes adjustPannedRange treat a logarithmic range as if it had no breaks. Even if the scrollbar is taught about the whole-range breaks above, panning across a supported logarithmic break will not preserve the visible scale, so this path needs the same log-space break-length handling.

packages/devextreme/js/__internal/viz/axes/base_axis.ts:1320

  • This guard skips all breaks for logarithmic axes, even though the axis translator and tick generator support logarithmic scale breaks and argumentAxis.breaks is a public option. A logarithmic argument axis with breaks therefore still builds a break-free scrollbar translator and can retain the jump this change is intended to fix. Please handle break lengths in the logarithmic coordinate space instead of excluding this axis type.
    if (type === constants.discrete || type === constants.logarithmic
      || !isDefined(businessRange.min) || !isDefined(businessRange.max)) {
      return [];
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants