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

Changed rendering of baseline and area series when using scaleMargins #1010

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,6 @@ debug.html

# graphics tests out data
/tests/e2e/graphics/.gendata/

# MacOS
.DS_Store
Copy link
Contributor

Choose a reason for hiding this comment

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

If you don't mind let's revert this. This folder should in your global gitignore since it has no in common with the project itself.

3 changes: 2 additions & 1 deletion src/renderers/area-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export interface PaneRendererAreaDataBase {
lineWidth: LineWidth;
lineStyle: LineStyle;

top: Coordinate;
bottom: Coordinate;
baseLevelCoordinate: Coordinate;

Expand Down Expand Up @@ -79,7 +80,7 @@ export class PaneRendererArea extends PaneRendererAreaBase<PaneRendererAreaData>
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const data = this._data!;

const gradient = ctx.createLinearGradient(0, 0, 0, data.bottom);
const gradient = ctx.createLinearGradient(0, data.top, 0, data.bottom);
gradient.addColorStop(0, data.topColor);
gradient.addColorStop(1, data.bottomColor);
return gradient;
Expand Down
6 changes: 4 additions & 2 deletions src/renderers/baseline-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ export class PaneRendererBaselineArea extends PaneRendererAreaBase<PaneRendererB
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const data = this._data!;

const gradient = ctx.createLinearGradient(0, 0, 0, data.bottom);
const baselinePercent = clamp(data.baseLevelCoordinate / data.bottom, 0, 1);
const gradient = ctx.createLinearGradient(0, data.top, 0, data.bottom);
const base = data.baseLevelCoordinate - data.top;
const height = data.bottom - data.top;
const baselinePercent = clamp(base / height, 0, 1);

gradient.addColorStop(0, data.topFillColor1);
gradient.addColorStop(baselinePercent, data.topFillColor2);
Expand Down
14 changes: 13 additions & 1 deletion src/views/pane/area-pane-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ export class SeriesAreaPaneView extends LinePaneViewBase<'Area', LineItem> {
}

const areaStyleProperties = this._series.options();
const priceScale = this._series.priceScale();
const priceScaleProps = priceScale.options();
const isCustomScale = priceScale.id() !== 'right' && priceScale.id() !== 'left';

let top = 0;
let bottom = height;

if (isCustomScale && priceScaleProps.scaleMargins) {
bottom = height;
top = priceScaleProps.scaleMargins.top * height;
}

this._makeValid();

Expand All @@ -37,7 +48,8 @@ export class SeriesAreaPaneView extends LinePaneViewBase<'Area', LineItem> {
topColor: areaStyleProperties.topColor,
bottomColor: areaStyleProperties.bottomColor,
baseLevelCoordinate: height as Coordinate,
bottom: height as Coordinate,
top: top as Coordinate,
bottom: bottom as Coordinate,
visibleRange: this._itemsVisibleRange,
barWidth: this._model.timeScale().barSpacing(),
});
Expand Down
16 changes: 14 additions & 2 deletions src/views/pane/baseline-pane-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,23 @@ export class SeriesBaselinePaneView extends LinePaneViewBase<'Baseline', LineIte
}

const baselineProps = this._series.options();
const priceScale = this._series.priceScale();
const priceScaleProps = priceScale.options();
const isCustomScale = priceScale.id() !== 'right' && priceScale.id() !== 'left';

this._makeValid();

const baseLevelCoordinate = this._series.priceScale().priceToCoordinate(baselineProps.baseValue.price, firstValue.value);
const baseLevelCoordinate = priceScale.priceToCoordinate(baselineProps.baseValue.price, firstValue.value);
const barWidth = this._model.timeScale().barSpacing();

let top = 0;
let bottom = height;

if (baselineProps.baseValue.type === 'price' && isCustomScale && priceScaleProps.scaleMargins) {
bottom = height * (1 - priceScaleProps.scaleMargins.bottom);
top = height * priceScaleProps.scaleMargins.top;
Comment on lines +47 to +48
Copy link
Contributor

Choose a reason for hiding this comment

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

well, I don't think that it should be like that. Instead, we need to get min/max coordinates for a visible range of this series. If we'll do this we won't need to check whether it is a "custom" scale or not because it will work automatically for any series. See my comment here #1005 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

we have this._items where we can get all coordinates for each item, but please make sure that you use this._itemsVisibleRange to iterate over that array properly.

Copy link
Contributor

Choose a reason for hiding this comment

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

also, you could take a look at _convertToCoordinates method, which is used to update coordinates, probably we could change override it here and calculate the data only once and save it somewhere.

}

this._baselineAreaRenderer.setData({
items: this._items,

Expand All @@ -50,7 +61,8 @@ export class SeriesBaselinePaneView extends LinePaneViewBase<'Baseline', LineIte
lineType: baselineProps.lineType,

baseLevelCoordinate,
bottom: height as Coordinate,
top: top as Coordinate,
bottom: bottom as Coordinate,

visibleRange: this._itemsVisibleRange,
barWidth,
Expand Down