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

lowColor is available on the dumbbell series options, but doesn't appear in the api documentation or typescript type definitions #20403

Closed
ryanewtaylor opened this issue Jan 9, 2024 · 7 comments · Fixed by #21168

Comments

@ryanewtaylor
Copy link

ryanewtaylor commented Jan 9, 2024

Expected behaviour

  1. lowColor should be specified on the dumbbell series documentation
  2. lowColor should be specified on the TypeScript type SeriesDumbbellOptions

Actual behaviour

  1. It is not documented as being available on the series object

  2. When applying lowColor on the series, we get the a typescript error despite lowColor being available on the DumbbellSeries type

    Object literal may only specify known properties, and 'lowColor' does not exist in type 'SeriesDumbbellOptions'.ts(2322)

    The following code demonstrates this

    const chart = Highcharts.chart({
      chart: {
        type: "dumbbell",
      },
      title: {
        text: "Fruit Consumption",
      },
      xAxis: {
        categories: ["Apples", "Bananas", "Oranges"],
      },
      yAxis: {
        title: {
          text: "Fruit eaten",
        },
      },
      series: [
        {
          type: "dumbbell",
          name: "Jane",
          // 👇 Object literal may only specify known properties, and 'lowColor' does not exist in type 'SeriesDumbbellOptions'
          lowColor: "red", 
          data: [{ name: "thing", low: 1, high: 5 }],
        },
      ],
    });

    By casting the series object as any the type error goes away and the desired color is applied, but I then lose all type safety.

Live demo with steps to reproduce

JavaScript example

https://codepen.io/iamrewt/pen/zYbBWvK

Product version

  • Highstock v11.2.0

Affected browser(s)

N/A. This is with regards to the documentation and typescript types.

@highsoft-bot highsoft-bot added this to To do in Development-Flow via automation Jan 9, 2024
@ryanewtaylor ryanewtaylor changed the title lowColor is available for us on the dumbbell series options, but doesn't appear in the api documentation or typescript type definitions lowColor is available on the dumbbell series options, but doesn't appear in the api documentation or typescript type definitions Jan 9, 2024
@karolkolodziej
Copy link
Contributor

Hi @ryanewtaylor!

Thank you for reporting the issue! You are right it should be added to the dumbbell API documentation.

@ryanewtaylor
Copy link
Author

ryanewtaylor commented Jan 11, 2024

@karolkolodziej, the TypeScript type for lowColor on the serries is also missing forcing me to cast it as any to avoid compiler warnings. Would this GitHub issue cover both cases (i.e., the documentation and the missing type definition)?

@jedrzejruta
Copy link
Contributor

@ryanewtaylor as a workaround, you can use the lowMarker.fillColor property, which provides the type safety and has the same result: https://stackblitz.com/edit/typescript-v7bvof?file=index.ts

It is quite confusing, as in the docs it is stated that alternatively, the lower dot color can be changed by series.lowColor: https://www.highcharts.com/docs/chart-and-series-types/dumbbell-series whereas the primary choice should still be the lowMarker. Any thoughts on this @TorsteinHonsi?

@TorsteinHonsi
Copy link
Collaborator

Yes I agree, a bit confusing. I am not too familiar with it, but I would assume that the higher-level options color and lowColor, should be prefered. And the marker and lowMarker options should be used only when a specific marker setting is required?

@magdalena-gut
Copy link
Member

This is probably related to the recent changes in the dumbbell #19184 .

  1. When it comes to the documentation, I believe it is structured this way because the series.lowMarker provides complete customization of the dot.
    However, I noticed one illogical part here:
fillColor series.marker.fillColor - color for the upper dot.
lowColor series.lowColor - color for the lower dot.

I think it should be changed to:

color series.color - color for the upper dot.
lowColor series.lowColor - color for the lower dot.
  1. When it comes to the the API, I checked previous versions and it seems that only dumbbell.data.lowColor was included, however, I believe dumbbell.lowColor should be included as well.

@jedrzejruta
Copy link
Contributor

I think it should be changed to:
color series.color - color for the upper dot.
lowColor series.lowColor - color for the lower dot.

I believe it shouldn't be changed, since the series.color changes not only the upper dot color, but also the line connecting dots.

@magdalena-gut
Copy link
Member

I think it should be changed to:
color series.color - color for the upper dot.
lowColor series.lowColor - color for the lower dot.

I believe it shouldn't be changed, since the series.color changes not only the upper dot color, but also the line connecting dots.

You're right. Just like we discussed, I've modified docs slightly.

Development-Flow automation moved this from To do to Done May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

6 participants