tradingview
`ISeriesPrimitiveAxisView` requires `fixedCoordinate()` to be specified
Lightweight Charts™ Version: 5.0.9
Steps/code to reproduce:
When adding axis views for price, the fixedCoordinate() method is marked as optional in ISeriesPrimitiveAxisView. However, if this is not specified, then the recalculateOverlapping() function logic is broken. This is because it thinks it finds overlap when this is not true (example below). Even just above the calling location of this function, I'm not sure this logic works with plugins because we cannot set the fixed coordinate of plugin axis views in this way.
See the following plugin branch to demonstrate: https://github.com/tpunt/lwc-plugin-countdown-to-close/tree/no-fixed-coordinate
Screenshot:
The axis label from createPriceLine() at price 700 is always moved to the top of the price axis.
Using this example, the top variable (used in the first call to recalculateOverlapping()) has the following two views:
[
"view: label = '680.00, coord = 217.21754385964914, fixed coordinate = 0, height = 17",
"view: label = '700.00, coord = 150.76140350877193, fixed coordinate = 150.76140350877193, height = 17"
]
After the call to recalculateOverlapping(), it has the following:
[
"view: label = '680.00, cood = 217.21754385964914, fixed coordinate = 0, height = 17",
"view: label = '700.00, cood = 150.76140350877193, fixed coordinate = 7.5, height = 17"
]
This is because recalculateOverlapping() finds overlap between the two labels, even though this is not true. The logic to check for overlap in this case is coordinate > prevFixedCoordinate - height, which is 150.76140350877193 > 0 - 17.
I'm not entirely sure what the fix should be here for this. It has taken quite some time to debug things this far. Any suggestions? Otherwise, I can have more of a think about how to resolve things this week.
Thanks for reporting this. It is likely a regression which was introduced in either #1621 (most likely this one) or #1630 and it was not picked up in any of our test cases.
I asked an LLM to have a look at the .patch for #1621 and your description of the issue. This is the response. Without checking properly, I do think it is on the right track.
Summary
Yes — the patch introduces an assumption that every price-axis view has an initial fixed coordinate, which breaks plugins that intentionally omit fixedCoordinate(). That assumption changes the overlap detection from using the raw coordinate() to getFixedCoordinate(), and with missing fixed coordinates it falls back to 0, causing false-positive overlap and forcing labels to be stacked to the top or bottom.
Below is what changed, why it causes the reported behavior, and how to fix it without regressing the alignment improvements.
What changed in the patch
The core behavioral change is in how overlap is computed after the refactor into recalculateOverlapping():
-
Before: overlap used the next view’s dynamic coordinate() and the previous view’s getFixedCoordinate()
- top side: overlap if coordinate() > prevFixedCoordinate - height
- bottom side: overlap if coordinate() < prevFixedCoordinate + height
-
After (from Patch 3/5 and hoisted as a free function in Patch 5/5):
- It now reads the next view’s position from view.getFixedCoordinate() instead of view.coordinate():
- const coordinate = view.getFixedCoordinate();
- overlap for top: coordinate > prevFixedCoordinate - height
- overlap for bottom: coordinate < prevFixedCoordinate + height
- It now reads the next view’s position from view.getFixedCoordinate() instead of view.coordinate():
This requires all views to have a sensible fixed coordinate before overlap detection runs. If a plugin view does not set fixedCoordinate() (allowed by ISeriesPrimitiveAxisView’s optional method), getFixedCoordinate() returns a default (effectively 0), so coordinate becomes 0. That triggers false overlap any time prevFixedCoordinate - height < 0 for top, or prevFixedCoordinate + height > 0 for bottom. In practice, it collapses the first affected label “group” toward the nearest edge and then the group-shift logic amplifies the effect.
This exactly matches the user report:
- “fixed coordinate = 0” for the 680 label
- Overlap check is 150.76 > 0 - 17 → true (incorrect)
- Label at 700 gets pushed to 7.5, i.e., near the top edge
Additionally, the new “group shift” logic assumes a well-defined initial anchor for each group based on the first item’s fixed coordinate:
- spaceBeforeCurrentGroup is computed from views[0].getFixedCoordinate()
- If that’s 0 because fixed is missing, the algorithm thinks there’s negative/insufficient space and starts shifting whole groups unnecessarily.
Why this causes the reported issue
- Plugins can omit fixedCoordinate() by design.
- The refactored algorithm assumes getFixedCoordinate() is the authoritative position even before any fixing happens.
- Missing fixedCoordinate() → default 0 → overlap always detected against the previous label → labels get stacked to the edge.
In other words, the algorithm no longer works with the “optional fixed coordinate” contract.
Concrete fixes
You can keep the improved grouping/shift logic but restore correct behavior for views that don’t predefine fixed coordinates. Options:
- Fallback to dynamic coordinate() whenever fixed is undefined
- Treat “no fixed coordinate yet” as “use coordinate()”.
- Implementation sketch:
- When reading the current view’s position for overlap, use:
- const coord = view.hasFixedCoordinate?.() ? view.getFixedCoordinate() : view.coordinate();
- For the first label in a group (spaceBeforeCurrentGroup), use the same fallback for views[0].
- When reading the current view’s position for overlap, use:
- This maintains the new algorithm but preserves backward compatibility with plugins.
- Initialize fixed coordinates from coordinate() before recalculation
- Prior to calling recalculateOverlapping on a list, set each view’s fixed coordinate to its dynamic coordinate if it doesn’t have one:
- for (const v of views) { if (!v.hasFixedCoordinate?.()) v.setFixedCoordinate(v.coordinate()); }
- Then use only getFixedCoordinate() in the overlap logic.
- This approach simplifies overlap logic but mutates state earlier; ensure it doesn’t conflict with other layout passes.
- Loosen overlap condition when the previous label’s fixed coordinate was itself derived from a missing fixed
- If prev was “unfixed” and we’re only using defaults, defer applying overlap until there’s a genuine conflict in dynamic space.
- This is more complex and not necessary if you adopt (1) or (2).
I recommend (1) because:
- It’s minimal and preserves the algorithmic structure.
- It avoids side effects from mass-initializing fixed coordinates.
- It respects ISeriesPrimitiveAxisView’s optional fixedCoordinate() contract.
Example patch sketch
Where the function currently does:
- const initLabelHeight = views[0].height(rendererOptions, true);
- let spaceBeforeCurrentGroup = direction === 1 ? scaleHeight / 2 - (views[0].getFixedCoordinate() - initLabelHeight / 2) : views[0].getFixedCoordinate() - initLabelHeight / 2 - scaleHeight / 2;
Change to:
- const pos0 = views[0].hasFixedCoordinate?.() ? views[0].getFixedCoordinate() : views[0].coordinate();
- let spaceBeforeCurrentGroup = direction === 1 ? scaleHeight / 2 - (pos0 - initLabelHeight / 2) : pos0 - initLabelHeight / 2 - scaleHeight / 2;
And in the loop:
- const prevPos = prev.getFixedCoordinate(); // prev is always fixed by construction as we go
- const currPos = view.hasFixedCoordinate?.() ? view.getFixedCoordinate() : view.coordinate();
Then compute overlap and only setFixedCoordinate when overlap is true (as now).
Optionally, expose hasFixedCoordinate() on IPriceAxisView to avoid duck-typing; otherwise, infer with a sentinel like Number.isFinite(view.getFixedCoordinate()) and define getFixedCoordinate to return NaN when unset.
Additional robustness
- Ensure getFixedCoordinate() returns undefined or NaN when not set, not 0. Using 0 as a sentinel collides with valid coordinates in the top half.
- Where desiredGroupShift is computed:
- For top direction 1: const desiredGroupShift = Math.max(0, -edgePoint);
- For bottom direction -1: const desiredGroupShift = Math.max(0, edgePoint - scaleHeight); This avoids the slightly odd “-1 - edgePoint” which can over-shift.
Conclusion
The regression stems from switching overlap checks to rely on getFixedCoordinate() for all labels, violating the optional nature of fixedCoordinate() in plugin views. Add a fallback to coordinate() when fixed is undefined (or initialize fixed from coordinate() before the pass) to fix the false overlap detection and restore correct placement for plugin axis labels.
Thanks for taking a look.
I think there are a couple of additional things here:
- I thought the semantics differed between coordinates versus fixed coordinates (as per the documentation). Does setting the fixed coordinates (e.g. here and then here) unintentionally change the expected/desired behaviour when resolving label overlap?
- Setting of fixed coordinates via
view.setFixedCoordinate()does not actually work for plugins. I put in a quick patch in_fixLabelOverlapto just always set the fixed coordinate for all views to1(it worked forcreatePriceLine(), but not for plugins):
--- a/src/gui/price-axis-widget.ts
+++ b/src/gui/price-axis-widget.ts
@@ -668,6 +668,10 @@ export class PriceAxisWidget implements IDestroyable {
if (coordinate > (this._size.height - halfHeight) && coordinate < this._size.height + halfHeight) {
view.setFixedCoordinate(this._size.height - halfHeight);
}
+
+ const from = view.getFixedCoordinate();
+ view.setFixedCoordinate(1);
+ console.log(`${view.text()}: set fixedCoordinate to coordinate: ${from} => ${view.getFixedCoordinate()}`);
}
recalculateOverlapping(top, 1, this._size.height, rendererOptions);
And then added some simple logging in the getting/setting of fixed coordinates:
--- a/src/views/price-axis/price-axis-view.ts
+++ b/src/views/price-axis/price-axis-view.ts
@@ -71,11 +71,14 @@ export abstract class PriceAxisView implements IPriceAxisView {
}
public getFixedCoordinate(): number {
+ console.log('getFixedCoordinate', JSON.parse(JSON.stringify(this._commonRendererData)));
return this._commonRendererData.fixedCoordinate || 0;
}
public setFixedCoordinate(value: number): void {
+ console.log('setFixedCoordinate before', JSON.parse(JSON.stringify(this._commonRendererData)));
this._commonRendererData.fixedCoordinate = value;
+ console.log('setFixedCoordinate after', JSON.parse(JSON.stringify(this._commonRendererData)));
}
public isVisible(): boolean {
The set operation clearly showed the fixed coordinate being added for the plugin label:
Before:
{
"_internal_coordinate": 237.21754385964914,
"_internal_background": "#ef5350",
"_internal_additionalPaddingBottom": 2,
"_internal_additionalPaddingTop": 2
}
After:
{
"_internal_coordinate": 237.21754385964914,
"_internal_background": "#ef5350",
"_internal_additionalPaddingBottom": 2,
"_internal_additionalPaddingTop": 2,
"_internal_fixedCoordinate": 1
}
But in the immediate get operation, the _internal_fixedCoordinate property had disappeared:
{
"_internal_coordinate": 237.21754385964914,
"_internal_background": "#ef5350",
"_internal_additionalPaddingBottom": 2,
"_internal_additionalPaddingTop": 2
}
So setting/getting the fixed coordinate of plugin labels does not appear to work in this code either.
I tried defaulting back to coordinate if fixed coordinate does not exist:
--- a/src/gui/price-axis-widget.ts
+++ b/src/gui/price-axis-widget.ts
@@ -74,6 +74,7 @@ function buildPriceAxisViewsGetter(
};
}
+// eslint-disable-next-line complexity
function recalculateOverlapping(views: IPriceAxisView[], direction: 1 | -1, scaleHeight: number, rendererOptions: Readonly<PriceAxisViewRendererOptions>): void {
if (!views.length) {
return;
@@ -81,17 +82,18 @@ function recalculateOverlapping(views: IPriceAxisView[], direction: 1 | -1, scal
let currentGroupStart = 0;
const initLabelHeight = views[0].height(rendererOptions, true);
+ const initFixedCoordinate = views[0].getFixedCoordinate() || views[0].coordinate();
let spaceBeforeCurrentGroup = direction === 1
- ? scaleHeight / 2 - (views[0].getFixedCoordinate() - initLabelHeight / 2)
- : views[0].getFixedCoordinate() - initLabelHeight / 2 - scaleHeight / 2;
+ ? scaleHeight / 2 - (initFixedCoordinate - initLabelHeight / 2)
+ : initFixedCoordinate - initLabelHeight / 2 - scaleHeight / 2;
spaceBeforeCurrentGroup = Math.max(0, spaceBeforeCurrentGroup);
for (let i = 1; i < views.length; i++) {
const view = views[i];
const prev = views[i - 1];
const height = prev.height(rendererOptions, false);
- const coordinate = view.getFixedCoordinate();
- const prevFixedCoordinate = prev.getFixedCoordinate();
+ const coordinate = view.getFixedCoordinate() || view.coordinate();
+ const prevFixedCoordinate = prev.getFixedCoordinate() || prev.coordinate();
const overlap = direction === 1
? coordinate > prevFixedCoordinate - height
@@ -107,7 +109,7 @@ function recalculateOverlapping(views: IPriceAxisView[], direction: 1 | -1, scal
const desiredGroupShift = direction === 1 ? -1 - edgePoint : edgePoint - scaleHeight;
const possibleShift = Math.min(desiredGroupShift, spaceBeforeCurrentGroup);
for (let k = currentGroupStart; k < views.length; k++) {
- views[k].setFixedCoordinate(views[k].getFixedCoordinate() + direction * possibleShift);
+ views[k].setFixedCoordinate((views[k].getFixedCoordinate() || views[k].coordinate()) + direction * possibleShift);
}
spaceBeforeCurrentGroup -= possibleShift;
}
It works fine for at least the broken case I observed:
The code around label overlapping resolution is still broken in other ways though (from my previous comment).
Thanks for the additional investigating and testing. Looks like we need to be rather thorough with this since there are a few cases which aren't working as expected. So our first task will be to create a few test cases to more accurately cover this feature; and then we will work on resolving the issue itself.
Looking some more at this, _alignLabels already sets all fixed coordinates to the coordinates:
views.forEach((view: IPriceAxisView) => view.setFixedCoordinate(view.coordinate()));
However, as mentioned above, the fixedCoordinate property keeps being unset. This is because of broken logic in _updateRendererDataIfNeeded, where this._invalidated was never set to false. This results in SeriesPrimitivePriceAxisViewWrapper._updateRendererData to keep being called, which sets the fixed coordinate from the data it has (which comes from the defined method overrides we specify in our plugins), thereby unsetting it if the plugin elides this method.
So, patching _updateRendererDataIfNeeded:
diff --git a/src/views/price-axis/price-axis-view.ts b/src/views/price-axis/price-axis-view.ts
index 4223b467b..fd6b0800b 100644
--- a/src/views/price-axis/price-axis-view.ts
+++ b/src/views/price-axis/price-axis-view.ts
public isVisible(): boolean {
@@ -122,6 +125,7 @@ export abstract class PriceAxisView implements IPriceAxisView {
this._axisRendererData.tickVisible = true;
this._paneRendererData.tickVisible = false;
this._updateRendererData(this._axisRendererData, this._paneRendererData, this._commonRendererData);
+ this._invalidated = false;
}
}
}
Now causes the fixedCoordinate to remain set when handling label overlapping, and subsequently render the label in the correct place.
Before:
After:
However, now the label from my plugin no longer renders.
Looking more at things, the coordinate from my plugin is being set correctly, but it ends up being -1 when inspecting PriceAxisViewRendererCommonData. This is because SeriesPrimitivePriceAxisViewWrapper._updateRendererData is now only called once. This is at the start when adding the pane, where things have not properly loaded yet. Given that the data can change regularly, this is incorrect (it probably went unnoticed, given the bug around this._invalidated never being unset).
So, where should PriceAxisView.update() be called to invalidate the initially loaded data for SeriesPrimitivePriceAxisViewWrapper? I'm not entirely sure what the correct answer is to this question. Placing it somewhere in _alignLabels works fine, but I'm not sure how correct it is:
diff --git a/src/gui/price-axis-widget.ts b/src/gui/price-axis-widget.ts
index f9ac8f58f..2b76f4f8d 100644
--- a/src/gui/price-axis-widget.ts
+++ b/src/gui/price-axis-widget.ts
@@ -629,6 +629,7 @@ export class PriceAxisWidget implements IDestroyable {
// crosshair individually
updateForSources(orderedSources);
+ views.forEach((view: IPriceAxisView) => view.update()); // Could filter for view.constructor.name === 'SeriesPrimitivePriceAxisViewWrapper'
views.forEach((view: IPriceAxisView) => view.setFixedCoordinate(view.coordinate()));
const options = this._priceScale.options();
The changes to price-axis-view.ts and price-axis-widget.ts seem to work fine, though it is unclear if this is the proper solution:
There is still another bug here though: because all fixed coordinates are defaulted to the coordinate, it means if we do as the docs suggest, then the labels break. E.g.
coordinate(): Coordinate {
return -10000 as Coordinate;
}
fixedCoordinate(): Coordinate | undefined {
return this._pos ?? -1 as Coordinate;
}
The label is not displayed because -10000 is taken as the coordinate. We can avoid setting the fixed coordinate if it already exists and then only work off of fixed coordinates in _fixLabelOverlap:
diff --git a/src/gui/price-axis-widget.ts b/src/gui/price-axis-widget.ts
index f9ac8f58f..880cee867 100644
--- a/src/gui/price-axis-widget.ts
+++ b/src/gui/price-axis-widget.ts
@@ -629,7 +629,12 @@ export class PriceAxisWidget implements IDestroyable {
// crosshair individually
updateForSources(orderedSources);
- views.forEach((view: IPriceAxisView) => view.setFixedCoordinate(view.coordinate()));
+ views.forEach((view: IPriceAxisView) => view.update());
+ views.forEach((view: IPriceAxisView) => {
+ if (!view.getFixedCoordinate()) {
+ view.setFixedCoordinate(view.coordinate());
+ }
+ });
const options = this._priceScale.options();
if (!options.alignLabels) {
@@ -645,22 +650,22 @@ export class PriceAxisWidget implements IDestroyable {
}
// split into two parts
- const top = views.filter((view: IPriceAxisView) => view.coordinate() <= center);
- const bottom = views.filter((view: IPriceAxisView) => view.coordinate() > center);
+ const top = views.filter((view: IPriceAxisView) => view.getFixedCoordinate() <= center);
+ const bottom = views.filter((view: IPriceAxisView) => view.getFixedCoordinate() > center);
// sort top from center to top
- top.sort((l: IPriceAxisView, r: IPriceAxisView) => r.coordinate() - l.coordinate());
+ top.sort((l: IPriceAxisView, r: IPriceAxisView) => r.getFixedCoordinate() - l.getFixedCoordinate());
// share center label
if (top.length && bottom.length) {
bottom.push(top[0]);
}
- bottom.sort((l: IPriceAxisView, r: IPriceAxisView) => l.coordinate() - r.coordinate());
+ bottom.sort((l: IPriceAxisView, r: IPriceAxisView) => l.getFixedCoordinate() - r.getFixedCoordinate());
for (const view of views) {
const halfHeight = Math.floor(view.height(rendererOptions) / 2);
- const coordinate = view.coordinate();
+ const coordinate = view.getFixedCoordinate();
if (coordinate > -halfHeight && coordinate < halfHeight) {
view.setFixedCoordinate(halfHeight);
}
This is an improvement. However, there are two important bugs here still:
- Because
getFixedCoordinatewill always return a number, it is impossible to distinguish between when the fixed coordinate is just not set, versus when we actually would like to display a label at the top of the price axis (at coordinate0) - Because we rely upon fixed coordinates, the behaviour mentioned in the docs does not apply (i.e. you cannot get two labels to render overlapping by specifying their fixed coordinates)
These are both pre-existing issues. Some more thought is needed here...
All of that aside, I'm still not sure what the best way to test this is.
I think I have a working patch now:
diff --git a/src/gui/price-axis-widget.ts b/src/gui/price-axis-widget.ts
index f9ac8f58f..7ba4049d7 100644
--- a/src/gui/price-axis-widget.ts
+++ b/src/gui/price-axis-widget.ts
@@ -82,23 +82,23 @@ function recalculateOverlapping(views: IPriceAxisView[], direction: 1 | -1, scal
const initLabelHeight = views[0].height(rendererOptions, true);
let spaceBeforeCurrentGroup = direction === 1
- ? scaleHeight / 2 - (views[0].getFixedCoordinate() - initLabelHeight / 2)
- : views[0].getFixedCoordinate() - initLabelHeight / 2 - scaleHeight / 2;
+ ? scaleHeight / 2 - (views[0].coordinate() - initLabelHeight / 2)
+ : views[0].coordinate() - initLabelHeight / 2 - scaleHeight / 2;
spaceBeforeCurrentGroup = Math.max(0, spaceBeforeCurrentGroup);
for (let i = 1; i < views.length; i++) {
const view = views[i];
const prev = views[i - 1];
const height = prev.height(rendererOptions, false);
- const coordinate = view.getFixedCoordinate();
- const prevFixedCoordinate = prev.getFixedCoordinate();
+ const coordinate = view.coordinate();
+ const prevCoordinate = prev.coordinate();
const overlap = direction === 1
- ? coordinate > prevFixedCoordinate - height
- : coordinate < prevFixedCoordinate + height;
+ ? coordinate > prevCoordinate - height
+ : coordinate < prevCoordinate + height;
if (overlap) {
- const fixedCoordinate = prevFixedCoordinate - height * direction;
+ const fixedCoordinate = prevCoordinate - height * direction;
view.setFixedCoordinate(fixedCoordinate);
const edgePoint = fixedCoordinate - direction * height / 2;
const outOfViewport = direction === 1 ? edgePoint < 0 : edgePoint > scaleHeight;
@@ -107,15 +107,15 @@ function recalculateOverlapping(views: IPriceAxisView[], direction: 1 | -1, scal
const desiredGroupShift = direction === 1 ? -1 - edgePoint : edgePoint - scaleHeight;
const possibleShift = Math.min(desiredGroupShift, spaceBeforeCurrentGroup);
for (let k = currentGroupStart; k < views.length; k++) {
- views[k].setFixedCoordinate(views[k].getFixedCoordinate() + direction * possibleShift);
+ views[k].setFixedCoordinate(views[k].coordinate() + direction * possibleShift);
}
spaceBeforeCurrentGroup -= possibleShift;
}
} else {
currentGroupStart = i;
spaceBeforeCurrentGroup = direction === 1
- ? prevFixedCoordinate - height - coordinate
- : coordinate - (prevFixedCoordinate + height);
+ ? prevCoordinate - height - coordinate
+ : coordinate - (prevCoordinate + height);
}
}
}
@@ -615,8 +615,9 @@ export class PriceAxisWidget implements IDestroyable {
const sourceViews = source.priceAxisViews(paneState, priceScale);
// never align selected sources
sourceViews.forEach((view: IPriceAxisView) => {
- view.setFixedCoordinate(null);
- if (view.isVisible()) {
+ view.update(); // Invalidate the view for fixed coordinate resetting
+
+ if (view.isVisible() && view.getFixedCoordinate() === null) {
views.push(view);
}
});
@@ -629,8 +630,6 @@ export class PriceAxisWidget implements IDestroyable {
// crosshair individually
updateForSources(orderedSources);
- views.forEach((view: IPriceAxisView) => view.setFixedCoordinate(view.coordinate()));
-
const options = this._priceScale.options();
if (!options.alignLabels) {
return;
diff --git a/src/views/price-axis/iprice-axis-view.ts b/src/views/price-axis/iprice-axis-view.ts
index 93b7b74ca..9c8a6ec46 100644
--- a/src/views/price-axis/iprice-axis-view.ts
+++ b/src/views/price-axis/iprice-axis-view.ts
@@ -6,7 +6,7 @@ import {
export interface IPriceAxisView {
coordinate(): number;
- getFixedCoordinate(): number;
+ getFixedCoordinate(): number | null;
height(rendererOptions: PriceAxisViewRendererOptions, useSecondLine?: boolean): number;
isVisible(): boolean;
isAxisLabelVisible(): boolean;
diff --git a/src/views/price-axis/price-axis-view.ts b/src/views/price-axis/price-axis-view.ts
index 4223b467b..62c5d6f80 100644
--- a/src/views/price-axis/price-axis-view.ts
+++ b/src/views/price-axis/price-axis-view.ts
@@ -70,8 +70,8 @@ export abstract class PriceAxisView implements IPriceAxisView {
);
}
- public getFixedCoordinate(): number {
- return this._commonRendererData.fixedCoordinate || 0;
+ public getFixedCoordinate(): number | null {
+ return this._commonRendererData.fixedCoordinate ?? null;
}
public setFixedCoordinate(value: number): void {
@@ -122,6 +122,7 @@ export abstract class PriceAxisView implements IPriceAxisView {
this._axisRendererData.tickVisible = true;
this._paneRendererData.tickVisible = false;
this._updateRendererData(this._axisRendererData, this._paneRendererData, this._commonRendererData);
+ this._invalidated = false;
}
}
}
Notes:
- If fixed coordinates are specified, do not try to do any overlapping resolution
- Allow for a fixed coordinate to have a
nullvalue, to distinguish between not being specified (i.e. we work off of coordinates and need overlapping resolution), versus the fixed coordinate being0 - I'm not too sure where it is best to invalidate the view to refresh the data. So for now, I am just doing this on all
viewsinPriceAxisWidget._alignLabels(), which obviously seems wrong
The way the view is invalidated will impact whether we can set the fixedCoordinate during the label overlap resolution of regular coordinates. If the views are not invalidated before each resolution, then fixedCoordinate will not be reset to null for non-fixed views. This would mean those labels will sometimes be skipped from the overlapping resolution logic. So either:
-
viewsalways need to be invalidated for the label overlap logic to work - set some other property when resolving label overlap, instead of the
fixedCoordinateproperty
Regarding (1) above, invalidating the views in this part of the logic only does not work. Running the tests, there is at least one other failure because the views need to be invalidated for other areas of the code base (e.g. tests/e2e/graphics/test-cases/applying-options/series-price-format.js fails without it).
I think to keep things simple here, I will submit a PR that simply fixes the overlapping labels issue. I will not change the invalidation logic in PriceAxisView._updateRendererDataIfNeeded() (by setting this._invalidated = false;). This part clearly needs more thought and may be more finicky to resolve (at least I don't have a solution to this).
Regarding (2), I have set a new property for label coordinates (renderCoordinate) to distinguish things better.
Thanks for submitting the PR (and all the work to reach that point). Since this appears to be a rather large task (to investigate and test), I had already created an internal task for our team to look into this in our next sprint (starting in a week). At that point, we will have a proper look at the PR and work together to resolve this. Apologies that we couldn't start earlier on this but a lot of the team is currently on vacation.
@SlicedSilver Do you have any updates on this?
Also, I'm happy to tackle some more of the open bugs. But I'm not too sure which ones you and your team are currently triaging though. Do you have a way to show which bugs are not assigned/being worked on (GitHub issue assignees doesn't seem to really be used)?
Reviewing this task & PR is still in our current sprint. We were working on the conflation bars feature first (which we merged earlier today). The other task we are looking to work on this sprint is #2000
We don't assign people on GitHub issues but we will sometimes add labels like 'help wanted' or 'good first issue' which are open for other to work on. However if there are issues you are interested in tackling then let us know on the issue and we can provide guidance. The main thing is that we don't want people to send a long time working on something and then we provide feedback which means they need to re-do everything. We would rather discuss tasks before someone starts so they are pointed in the right direction.