Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
21 changes: 21 additions & 0 deletions src/model/series-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,13 @@ export interface LineStyleOptions {
* @defaultValue {@link LastPriceAnimationMode.Disabled}
*/
lastPriceAnimation: LastPriceAnimationMode;

/**
* Connect gaps in data.
*
* @defaultValue `true`
*/
Comment on lines +235 to +239

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.

The documentation should explain more precisely what constitutes a "gap". Users may be confused about the interaction with multiple series on the same chart. I'd suggest something like:

Suggested change
/**
* Connect gaps in data.
*
* @defaultValue `true`
*/
/**
* Whether to draw a connecting line across gaps (whitespace) in the data.
*
* When `true`, the line will be drawn continuously even if the data contains
* whitespace entries. When `false`, the line will break at each whitespace gap.
*
* Note: Gaps are only detected where this series has explicit whitespace data
* (entries with only a `time` property and no value). If another series on the
* same chart has data at different time points, this will not create gaps in
* this series; add whitespace entries to this series' data to create gaps.
*
* @defaultValue `true`
*/

Same JSDoc update needed for the identical blocks in AreaStyleOptions and BaselineStyleOptions.

connectGaps?: boolean;

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.

Suggestion: connectGaps should be required (not optional).

Every other property in this interface (e.g. pointMarkersVisible, lastPriceAnimation) is declared as required, and this property always has a default value provided in lineStyleDefaults. Declaring it as optional forces every consumer to cast with as boolean — there are currently 6 instances of options.connectGaps as boolean across the pane views.

Suggested change
connectGaps?: boolean;
connectGaps: boolean;

The JSDoc @defaultValue is also misleading alongside ?: — it implies the value is always present (via defaults), but the type says it could be undefined.

Same change needed in AreaStyleOptions (line 367) and BaselineStyleOptions (line 527).
Once this is done, all six as boolean casts in line-pane-view.ts, area-pane-view.ts, and baseline-pane-view.ts should be removed.

}

/**
Expand Down Expand Up @@ -351,6 +358,13 @@ export interface AreaStyleOptions {
* @defaultValue {@link LastPriceAnimationMode.Disabled}
*/
lastPriceAnimation: LastPriceAnimationMode;

/**
* Connect gaps in data.
*
* @defaultValue `true`
*/
connectGaps?: boolean;
}

/**
Expand Down Expand Up @@ -504,6 +518,13 @@ export interface BaselineStyleOptions {
* @defaultValue {@link LastPriceAnimationMode.Disabled}
*/
lastPriceAnimation: LastPriceAnimationMode;

/**
* Connect gaps in data.
*
* @defaultValue `true`
*/
connectGaps?: boolean;
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/model/series/area-pane-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class SeriesAreaPaneView extends LinePaneViewBase<'Area', AreaFillItem &
invertFilledArea: options.invertFilledArea,
visibleRange: this._itemsVisibleRange,
barWidth: this._model.timeScale().barSpacing(),
connectGaps: options.connectGaps as boolean,
});

this._lineRenderer.setData({
Expand All @@ -63,6 +64,7 @@ export class SeriesAreaPaneView extends LinePaneViewBase<'Area', AreaFillItem &
visibleRange: this._itemsVisibleRange,
barWidth: this._model.timeScale().barSpacing(),
pointMarkersRadius: options.pointMarkersVisible ? (options.pointMarkersRadius || options.lineWidth / 2 + 2) : undefined,
connectGaps: options.connectGaps as boolean,
});
}
}
1 change: 1 addition & 0 deletions src/model/series/area-series.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const areaStyleDefaults: AreaStyleOptions = {
crosshairMarkerBackgroundColor: '',
lastPriceAnimation: LastPriceAnimationMode.Disabled,
pointMarkersVisible: false,
connectGaps: true,
};
const createPaneView = (series: ISeries<'Area'>, model: IChartModelBase): IUpdatablePaneView => new SeriesAreaPaneView(series, model);
export const createSeries = (): SeriesDefinition<'Area'> => {
Expand Down
2 changes: 2 additions & 0 deletions src/model/series/baseline-pane-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export class SeriesBaselinePaneView extends LinePaneViewBase<'Baseline', Baselin
invertFilledArea: false,
visibleRange: this._itemsVisibleRange,
barWidth,
connectGaps: options.connectGaps as boolean,
});

this._baselineLineRenderer.setData({
Expand All @@ -81,6 +82,7 @@ export class SeriesBaselinePaneView extends LinePaneViewBase<'Baseline', Baselin
bottomCoordinate,
visibleRange: this._itemsVisibleRange,
barWidth,
connectGaps: options.connectGaps as boolean,
});
}
}
1 change: 1 addition & 0 deletions src/model/series/baseline-series.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const baselineStyleDefaults: BaselineStyleOptions = {

lastPriceAnimation: LastPriceAnimationMode.Disabled,
pointMarkersVisible: false,
connectGaps: true,
};
const createPaneView = (series: ISeries<'Baseline'>, model: IChartModelBase): IUpdatablePaneView => new SeriesBaselinePaneView(series, model);

Expand Down
1 change: 1 addition & 0 deletions src/model/series/line-pane-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export class SeriesLinePaneView extends LinePaneViewBase<'Line', LineStrokeItem,
pointMarkersRadius: options.pointMarkersVisible ? (options.pointMarkersRadius || options.lineWidth / 2 + 2) : undefined,
visibleRange: this._itemsVisibleRange,
barWidth: this._model.timeScale().barSpacing(),
connectGaps: options.connectGaps as boolean,

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.

The as boolean cast here is a workaround for connectGaps being declared optional in the type interface. If connectGaps is made required in LineStyleOptions (see comment on series-options.ts), this cast should be removed.

Same applies to the casts in area-pane-view.ts (lines 56 and 67) and baseline-pane-view.ts (lines 71 and 85).

};

this._renderer.setData(data);
Expand Down
1 change: 1 addition & 0 deletions src/model/series/line-series.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const lineStyleDefaults: LineStyleOptions = {
crosshairMarkerBackgroundColor: '',
lastPriceAnimation: LastPriceAnimationMode.Disabled,
pointMarkersVisible: false,
connectGaps: true,
};

const createPaneView = (series: ISeries<'Line'>, model: IChartModelBase): IUpdatablePaneView => new SeriesLinePaneView(series, model);
Expand Down
6 changes: 4 additions & 2 deletions src/renderers/area-renderer-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export interface PaneRendererAreaDataBase<TItem extends AreaFillItemBase = AreaF
barWidth: number;

visibleRange: SeriesItemsIndexesRange | null;

connectGaps: boolean;
}

function finishStyledArea(
Expand Down Expand Up @@ -50,7 +52,7 @@ export abstract class PaneRendererAreaBase<TData extends PaneRendererAreaDataBas
return;
}

const { items, visibleRange, barWidth, lineWidth, lineStyle, lineType } = this._data;
const { items, visibleRange, barWidth, lineWidth, lineStyle, lineType, connectGaps } = this._data;
const baseLevelCoordinate =
this._data.baseLevelCoordinate ??
(this._data.invertFilledArea ? 0 : renderingScope.mediaSize.height) as Coordinate;
Expand All @@ -69,7 +71,7 @@ export abstract class PaneRendererAreaBase<TData extends PaneRendererAreaDataBas
// walk lines with width=1 to have more accurate gradient's filling
ctx.lineWidth = 1;

walkLine(renderingScope, items, lineType, visibleRange, barWidth, this._fillStyle.bind(this), finishStyledArea.bind(null, baseLevelCoordinate));
walkLine(renderingScope, items, lineType, visibleRange, barWidth, this._fillStyle.bind(this), finishStyledArea.bind(null, baseLevelCoordinate), 0, connectGaps);
}

protected abstract _fillStyle(renderingScope: BitmapCoordinatesRenderingScope, item: TData['items'][0]): CanvasRenderingContext2D['fillStyle'];
Expand Down
6 changes: 4 additions & 2 deletions src/renderers/line-renderer-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export interface PaneRendererLineDataBase<TItem extends LineItemBase = LineItemB
visibleRange: SeriesItemsIndexesRange | null;

pointMarkersRadius?: number;

connectGaps: boolean;
}

function finishStyledArea(scope: BitmapCoordinatesRenderingScope, style: CanvasRenderingContext2D['strokeStyle']): void {
Expand All @@ -43,7 +45,7 @@ export abstract class PaneRendererLineBase<TData extends PaneRendererLineDataBas
return;
}

const { items, visibleRange, barWidth, lineType, lineWidth, lineStyle, pointMarkersRadius } = this._data;
const { items, visibleRange, barWidth, lineType, lineWidth, lineStyle, pointMarkersRadius, connectGaps } = this._data;

if (visibleRange === null) {
return;
Expand All @@ -63,7 +65,7 @@ export abstract class PaneRendererLineBase<TData extends PaneRendererLineDataBas
const dashPatternLength = getDashPatternLength(dashPattern);

if (lineType !== undefined) {
walkLine(renderingScope, items, lineType, visibleRange, barWidth, styleGetter, finishStyledArea, dashPatternLength);
walkLine(renderingScope, items, lineType, visibleRange, barWidth, styleGetter, finishStyledArea, dashPatternLength, connectGaps);
}

if (pointMarkersRadius) {
Expand Down
22 changes: 16 additions & 6 deletions src/renderers/walk-line.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { BitmapCoordinatesRenderingScope } from 'fancy-canvas';

import { Coordinate } from '../model/coordinate';
import { SeriesItemsIndexesRange } from '../model/time-data';
import { SeriesItemsIndexesRange, TimePointIndex } from '../model/time-data';

import { LinePoint, LineType } from './draw-line';

Expand All @@ -10,7 +10,7 @@ function distanceByCoordinates(p1x: number, p1y: number, p2x: number, p2y: numbe
}

// eslint-disable-next-line max-params, complexity
export function walkLine<TItem extends LinePoint, TStyle extends CanvasRenderingContext2D['fillStyle' | 'strokeStyle']>(
export function walkLine<TItem extends LinePoint & { time?: TimePointIndex }, TStyle extends CanvasRenderingContext2D['fillStyle' | 'strokeStyle']>(

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.

The { time?: TimePointIndex } intersection is unnecessary and over-permissive. Every caller passes items that extend TimedValue (via LineItemBase or AreaFillItemBase), and TimedValue already has time: TimePointIndex as a required property. This optional annotation is what forces the defensive !== undefined checks on line 79.

Suggested change
export function walkLine<TItem extends LinePoint & { time?: TimePointIndex }, TStyle extends CanvasRenderingContext2D['fillStyle' | 'strokeStyle']>(
export function walkLine<TItem extends LinePoint & TimedValue, TStyle extends CanvasRenderingContext2D['fillStyle' | 'strokeStyle']>(

With TimedValue (already exported from ../model/time-data), the import can stay the same, TimedValue already brings in TimePointIndex. And the gap check on line 79 can be simplified to remove the undefined guards (see comment on that line).

renderingScope: BitmapCoordinatesRenderingScope,
items: readonly TItem[],
lineType: LineType,
Expand All @@ -20,7 +20,8 @@ export function walkLine<TItem extends LinePoint, TStyle extends CanvasRendering
// so if styleGetter returns objects, then styleGetter should return the same object for equal styles
styleGetter: (renderingScope: BitmapCoordinatesRenderingScope, item: TItem) => TStyle,
finishStyledArea: (renderingScope: BitmapCoordinatesRenderingScope, style: TStyle, areaFirstItem: LinePoint, newAreaFirstItem: LinePoint) => void,
dashPatternLength: number = 0
dashPatternLength: number = 0,
connectGaps: boolean = true
): void {
if (items.length === 0 || visibleRange.from >= items.length || visibleRange.to <= 0) {
return;
Expand Down Expand Up @@ -74,19 +75,29 @@ export function walkLine<TItem extends LinePoint, TStyle extends CanvasRendering
const currentY = currentItem.y * verticalPixelRatio;
const itemStyle = styleGetter(renderingScope, currentItem);

const prevItem = items[i - 1];
const isGap = !connectGaps && currentItem.time !== undefined && prevItem.time !== undefined && currentItem.time - prevItem.time > 1;

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.

Two issues with this gap detection:

  1. undefined checks are unnecessary. As noted in the comment on the generic constraint, time is always present on these items. The currentItem.time !== undefined && prevItem.time !== undefined guards are dead code.
  2. time - prevItem.time > 1 is too broad — it catches gaps from other series, not just whitespace TimePointIndex is a global index shared across all series on a chart. When Series A has data at times [Jan 1, Jan 3] and Series B has data at [Jan 1, Jan 2, Jan 3], the merged time scale gives indices [0, 1, 2]. Series A's items will be [{time: 0}, {time: 2}] — a difference of 2, which this check treats as a gap. But Series A never provided whitespace; the index gap comes from Series B contributing a time point that Series A doesn't have.

The intended behaviour (per discussion) is: only break the line where this series has explicit whitespace data between two points. If the user wants gaps for timestamps that only exist in another series, they should add whitespace entries to this series.

This means the gap detection can't be done purely at the renderer level by comparing TimePointIndex values. The information about whether whitespace existed between two points is lost by the time we reach walkLine — whitespace rows are filtered out in DataLayer._setRowsToSeries().

One approach: during item construction in the pane view (e.g. in LinePaneViewBase), compute a boolean flag like gapBefore on each item by checking whether the series' original data contained whitespace rows between consecutive plot rows. This information is available at that layer and could be passed through to the renderer without adding complexity to walkLine's parameters.


if (isGap) {
finishStyledArea(renderingScope, currentStyle, currentStyleFirstItem, prevItem);
ctx.beginPath();
currentStyle = itemStyle;
currentStyleFirstItem = currentItem;
ctx.moveTo(currentX, currentY);
continue;
}
Comment on lines +81 to +88

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.

Bug: accumulatedDistance is not reset when breaking at a gap.

When the line breaks at a gap, finishStyledArea is called and a new path begins, but accumulatedDistance (used for dash pattern offset tracking) is not reset. Compare with the changeStyle helper above (lines 52-65), which correctly handles this:

if (shouldTrackDashOffset) {
    const offset = accumulatedDistance % dashPatternLength;
    ctx.lineDashOffset = offset;
    accumulatedDistance = offset;
}

This means that when connectGaps: false is combined with a dashed or dotted line style, the dash pattern after a gap will start at the wrong offset rather than resetting cleanly.

Suggested fix: add after ctx.beginPath();:

if (shouldTrackDashOffset) {
    accumulatedDistance = 0;
    ctx.lineDashOffset = 0;
}


switch (lineType) {
case LineType.Simple: {
ctx.lineTo(currentX, currentY);
if (shouldTrackDashOffset) {
const prevItem = items[i - 1];
const prevX = prevItem.x * horizontalPixelRatio;
const prevY = prevItem.y * verticalPixelRatio;
accumulatedDistance += distanceByCoordinates(prevX, prevY, currentX, currentY);
}
break;
}
case LineType.WithSteps: {
const prevItem = items[i - 1];
const prevY = prevItem.y * verticalPixelRatio;
ctx.lineTo(currentX, prevY);
if (shouldTrackDashOffset) {
Expand Down Expand Up @@ -120,7 +131,6 @@ export function walkLine<TItem extends LinePoint, TStyle extends CanvasRendering
);

if (shouldTrackDashOffset) {
const prevItem = items[i - 1];
const prevX = prevItem.x * horizontalPixelRatio;
const prevY = prevItem.y * verticalPixelRatio;
const chord = distanceByCoordinates(prevX, prevY, currentX, currentY);
Expand Down
69 changes: 69 additions & 0 deletions tests/e2e/graphics/test-cases/series/connect-gaps.js

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.

The test covers the connectGaps: false case well for all three series types, but some additional coverage would be valuable:

  • connectGaps: true (default) with whitespace data — verifying the line connects across gaps (regression test for the default behaviour).
  • Dashed/dotted line styles with connectGaps: false — this exercises the accumulatedDistance / lineDashOffset code path which currently has a bug (see comment on walk-line.ts).
  • LineType.Curved with connectGaps: false — the bezier control point calculation in getControlPoints() uses adjacent items. After a gap break + moveTo, the first curved segment of the new sub-path might produce unexpected control points since getControlPoints still sees the pre-gap item in the array.
  • Multiple series with different time ranges and connectGaps: false — verifying that a series doesn't get spurious gaps from another series' timestamps (this relates to the gap detection logic concern in walk-line.ts).

Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
function generateData() {
const res = [];
const time = new Date(Date.UTC(2018, 0, 1, 0, 0, 0, 0));
for (let i = 0; i < 30; ++i) {
const currentTime = time.getTime() / 1000;
time.setUTCDate(time.getUTCDate() + 1);

// Create a gap by pushing WhitespaceData (an empty point with just a time).
// This ensures the time scale has indices for these days, creating a gap in the series data's TimePointIndex array.
if (i > 10 && i < 20) {
res.push({ time: currentTime });
continue;
}

res.push({
time: currentTime,
value: 10 + Math.sin(i / 5) * 5,
});
}
return res;
}

function runTestCase(container) {
const chart = window.chart = LightweightCharts.createChart(container, {
layout: {
attributionLogo: false,
},
timeScale: {
barSpacing: 20,
},
});

const data = generateData();

// Line Series with connectGaps: false
const lineSeries = chart.addSeries(LightweightCharts.LineSeries, {
color: '#2196F3',
lineWidth: 2,
connectGaps: false,
title: 'Line (No Gaps)',
});
lineSeries.setData(data);

// Area Series with connectGaps: false (offset for visibility)
const areaSeries = chart.addSeries(LightweightCharts.AreaSeries, {
topColor: 'rgba(33, 150, 243, 0.4)',
bottomColor: 'rgba(33, 150, 243, 0.1)',
lineColor: '#2196F3',
connectGaps: false,
title: 'Area (No Gaps)',
});
areaSeries.setData(data.map(d => d.value !== undefined ? { ...d, value: d.value + 10 } : d));

// Baseline Series with connectGaps: false
const baselineSeries = chart.addSeries(LightweightCharts.BaselineSeries, {
baseValue: { type: 'price', price: 30 },
topFillColor1: 'rgba(38, 166, 154, 0.28)',
topFillColor2: 'rgba(38, 166, 154, 0.05)',
topLineColor: 'rgba(38, 166, 154, 1)',
bottomFillColor1: 'rgba(239, 83, 80, 0.05)',
bottomFillColor2: 'rgba(239, 83, 80, 0.28)',
bottomLineColor: 'rgba(239, 83, 80, 1)',
connectGaps: false,
title: 'Baseline (No Gaps)',
});
baselineSeries.setData(data.map(d => d.value !== undefined ? { ...d, value: d.value + 20 } : d));

chart.timeScale().fitContent();
}
Loading