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

dataGrouping: Anchor property is not applied to the last group when xAxis.max is out of the last group interval #19743

Closed
Manizuca opened this issue Sep 13, 2023 · 7 comments · Fixed by #19959

Comments

@Manizuca
Copy link

Expected behaviour

The anchor property is not applied to the last group

Actual behaviour

The anchor property (or lastAnchor) should be applied to the last group

Live demo with steps to reproduce

https://jsfiddle.net/nfz5cyx6/1/
Note that the last 2 series have one group less that the first one, so the 'groupedXData[groupedDataLength] >= xMax - totalRange' condition is false, and it doesn't apply the anchor property on the anchorPoints function from ts/Core/Axis/Axis.ts

Product version

Highstock v11.1.0

Affected browser(s)

@highsoft-bot highsoft-bot added this to To do in Development-Flow via automation Sep 13, 2023
@Manizuca
Copy link
Author

I see now that this bug is mentioned at #19466 (comment), though I think it is appropriate for it to have its own issue, so I will leave this open. (I didn't find it at first, since the title is not related, but then I got the bug from #19466, and i got there)

@karolkolodziej
Copy link
Contributor

Hi @Manizuca!

Thank you for bringing this to our attention! But, the issue you mention is not strictly related to this.
I'm also not sure if this should work as you described because the grouping (interval and amount of groups), is calculated globally and the same should be done with the anchors.
In my opinion, the lastAnchor should not be applied to the last series' point, only the anchor which isn't right now.
Do you agree @pawelfus?

@pawelfus
Copy link
Contributor

pawelfus commented Sep 14, 2023

I don't see a problem here. The second series and the third one have one group fewer because there are fewer points in the dataset. If we add missing points, everything looks good: https://jsfiddle.net/BlackLabel/32zhf8bt/

@karolkolodziej
Copy link
Contributor

Hmm, I was wondering how the groups should be treated.
Imagine a scenario where you have multiple series, one is significantly longer than the other- demo.
Should the firstAnchor be applied to the first point of the second/third series? Analogically to the last?
Especially check the second series and anchor of it's last point.

@pawelfus
Copy link
Contributor

pawelfus commented Sep 15, 2023

Ah, thanks for the clarification! I missed the OP's question where in fact we have two issues reported here:

(...) the last 2 series have one group less that the first one
(...) and it doesn't apply the anchor property

The first issue is not a bug, it is the expected result.
The second issue is a bit more complex. @Manizuca could you tell me a little more about your use case? Do you want to use different anchors on the same chart? Or have your datasets different amounts of points?

@Manizuca
Copy link
Author

yeah, sorry @pawelfus if my writing and example was confusing, I didn't mean that the first part was an issue, just relevant context for describing the bug.

As for my use case, I don't wanna use different anchors, I just need all of them to be at the middle. I have multiple series, and it could happen that a series has more recent data than another, so they have different amount of groups. Right now, if that happens, the last anchor of the other series is not at the middle.

The example had different anchors to show the difference in behaviours between them, but maybe its easier to see the problem when all of then should be at the middle, like its my usecase: https://jsfiddle.net/q9o78zLu/
In this case, the anchors should be at 01:00, 03:00, 05:00... etc, however the last anchor of the second serie is at 08:00, instead of 09:00

Side note: I just realized the tooltip time in my last example is incorrect, right?. For example, the second group should have data between 02:00 and 03:59, and so the anchor is at the middle: 03:00. But the tooltip says: "03:00-04:59"

@pawelfus
Copy link
Contributor

Thank you for the explanation @Manizuca , really appreciated it! Yes, I agree, the anchor is misplaced for the last group in the second series. We will fix it.

Side note: I just realized the tooltip time in my last example is incorrect, right?. For example, the second group should have data between 02:00 and 03:59, and so the anchor is at the middle: 03:00. But the tooltip says: "03:00-04:59"

Good point! Temporary workarounds:

  1. Use tooltip.headerFormat - where we can manually translate the start and the end of the period in the tooltip.

  2. Do the same in the core, without using options. Note: this workaround below doesn't work when firstAnchor and/or lastAnchor is set to a different value than anchor.

Demo:

Plugin:

(function(H) {
  H.removeEvent(H.Tooltip, 'headerFormatter');

  H.addEvent(
    H.Tooltip,
    'headerFormatter',
    function(e) {
      const chart = this.chart,
        time = chart.time,
        labelConfig = e.labelConfig,
        series = labelConfig.series,
        options = series.options,
        tooltipOptions = series.tooltipOptions,
        dataGroupingOptions = options.dataGrouping,
        xAxis = series.xAxis;

      let xDateFormat = tooltipOptions.xDateFormat,
        xDateFormatEnd,
        currentDataGrouping,
        dateTimeLabelFormats,
        labelFormats,
        formattedKey,
        formatString = tooltipOptions[
          e.isFooter ? 'footerFormat' : 'headerFormat'
        ];

      // apply only to grouped series
      if (
        xAxis &&
        xAxis.options.type === 'datetime' &&
        dataGroupingOptions &&
        H.isNumber(labelConfig.key)
      ) {

        // set variables
        currentDataGrouping = series.currentDataGrouping;
        dateTimeLabelFormats = dataGroupingOptions.dateTimeLabelFormats ||
          // Fallback to commonOptions (#9693)
          H.DataGroupingDefaults.common.dateTimeLabelFormats;

        // if we have grouped data, use the grouping information to get the
        // right format
        if (currentDataGrouping) {
          labelFormats = (dateTimeLabelFormats)[
            currentDataGrouping.unitName
          ];
          if ((currentDataGrouping).count === 1) {
            xDateFormat = labelFormats[0];
          } else {
            xDateFormat = labelFormats[1];
            xDateFormatEnd = labelFormats[2];
          }
          // if not grouped, and we don't have set the xDateFormat option, get the
          // best fit, so if the least distance between points is one minute, show
          // it, but if the least distance is one day, skip hours and minutes etc.
        } else if (!xDateFormat && dateTimeLabelFormats && xAxis.dateTime) {
          xDateFormat = xAxis.dateTime.getXDateFormat(
            labelConfig.x,
            tooltipOptions.dateTimeLabelFormats

          );
        }


        const anchorRange = {
          start: 0,
          middle: -(currentDataGrouping).totalRange / 2,
          end: -(currentDataGrouping).totalRange
        } [series.options.dataGrouping.anchor];


        // now format the key
        formattedKey = time.dateFormat(xDateFormat, labelConfig.key + anchorRange);
        if (xDateFormatEnd) {

          formattedKey += time.dateFormat(
            xDateFormatEnd,
            labelConfig.key + (currentDataGrouping).totalRange + anchorRange - 1
          );
        }

        // Replace default header style with class name
        if (series.chart.styledMode) {
          formatString = this.styledModeFormat(formatString);
        }

        // return the replaced format
        e.text = H.format(
          formatString, {
            point: H.extend(labelConfig.point, {
              key: formattedKey
            }),
            series: series
          },
          chart
        );

        e.preventDefault();


      }
    }
  );
})(Highcharts)

Internal note:
When fixing, fix both issues:

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.

5 participants