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

Wrong 'Tooltip.pointFormatter' type definition after parsing to d.ts #12145

Closed
Denyllon opened this issue Oct 4, 2019 · 8 comments · Fixed by #12364
Closed

Wrong 'Tooltip.pointFormatter' type definition after parsing to d.ts #12145

Denyllon opened this issue Oct 4, 2019 · 8 comments · Fixed by #12364
Assignees
Labels
Product: TypeScript Issues & enhancements related to TS/TypeScript Type: Enhancement

Comments

@Denyllon
Copy link
Contributor

Denyllon commented Oct 4, 2019

Expected behaviour

Context should indicate the Point object, and all series properties should exist on this.series within formatter function.

Actual behaviour

The Tooltip.pointFormatter definition is not correct after parsing to highcharts.d.ts file.
Before it looks like:

pointFormatter?: FormatterCallbackFunction<Point>;

when the FormatterCallbackFunction is:

interface FormatterCallbackFunction<T> {
    (this: T): string;
}

but after parse it looks like not complete or overwritten by something else:

pointFormatter?: () => void;

This cause errors within custom formatter function, a few 'does not exist' warnings, and wrong context
image

@Denyllon Denyllon added Product: TypeScript Issues & enhancements related to TS/TypeScript Type: Bug labels Oct 4, 2019
@Denyllon
Copy link
Contributor Author

Denyllon commented Oct 4, 2019

@bre1470 ☝️

@sebastianbochan
Copy link
Contributor

Live demo:

@KacperMadej
Copy link

KacperMadej commented Oct 4, 2019

I did the testing for the issue and something was not refreshed correctly.
The actual problem is as you could see in the live demo, so in tooltip.pointFormatter the context is a point that has series as one of its properties. That series is of a type that doesn't allow accessing color property - neither directly (series.color), or through options (series.options.color).

The fastest workaround is, as usual, to cast to any like (this.series as any).color

@bre1470
Copy link
Contributor

bre1470 commented Oct 4, 2019

You have to define the this parameter of your formatter function to be of type Highcharts.Point. Additionally you try to cast the series to an option interface. You can find the options under series.options.

That said, there is no color property on the series instance on runtime. I do not see how this is a TypeScript bug.

@KacperMadej
Copy link

You can find the options under series.options.

color isn't there, but opacity is.

@bre1470
Copy link
Contributor

bre1470 commented Oct 7, 2019

The color option is not supported by every series, therefor the series options have to be casted to a specific interface and then color options checked agains undefined.

@bre1470
Copy link
Contributor

bre1470 commented Oct 9, 2019

Further investigation shows, that the supertrend indicator is the only series, that does not support the color option. Maybe we can change this and then color becomes automatically a common series option.

@pawelfus
Copy link
Contributor

pawelfus commented Oct 9, 2019

I agree, we can color to supertrend (can be the same as risingTrendColor or fallingTrendColor). We have similar case for candlestick where we have color and upColor.

@wchmiel FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product: TypeScript Issues & enhancements related to TS/TypeScript Type: Enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants