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

PointStyleAccessor not applying to isolated points #2460

Closed
mageshsankar opened this issue Jun 9, 2024 · 12 comments · Fixed by #2464
Closed

PointStyleAccessor not applying to isolated points #2460

mageshsankar opened this issue Jun 9, 2024 · 12 comments · Fixed by #2464

Comments

@mageshsankar
Copy link

mageshsankar commented Jun 9, 2024

Summary
In Kibana version 8.12.0, the pointStyleAccessor is not correctly applying the specified point styles to the first data point of each group in a line chart using @elastic/charts. This issue did not exist in Kibana version 7.16.3, where the point styles were applied correctly.

Steps to Reproduce

  1. Set up a line chart using @elastic/charts in Kibana version 8.12.0.
  2. Define a pointStyleAccessor to customize the appearance of data points.
  3. Populate the chart with multiple series of data points, ensuring each series has at least two data points.
  4. Observe that the pointStyleAccessor does not apply the specified styles to the first point of each series.
  5. Expected Behavior
  6. The pointStyleAccessor should consistently apply the defined styles to all data points, including the first point of each series, as it did in Kibana version 7.16.3.

Actual Behavior
The first data point of each series does not have the styles specified in the pointStyleAccessor applied. Subsequent points in the series correctly reflect the defined styles.

Code Example
Below is a simplified version of the code that demonstrates the issue:

function MyChart() {
  const dummyData = [
    { date: '2024-01-01', codeLines: 800, project: 'Project A1', version: 'v1.0' },
    { date: '2024-06-02', codeLines: 1100, project: 'Project A', version: 'v1.1' },
    { date: '2024-02-01', codeLines: 900, project: 'Project A', version: 'v1.0' },
    { date: '2024-07-02', codeLines: 950, project: 'Project B', version: 'v1.1' },
    { date: '2024-08-03', codeLines: 1000, project: 'Project C', version: 'v1.2' },
  ];
  return (
    <Chart renderer="canvas">
      <Settings showLegend legendPosition={Position.Bottom} theme={lineChartTheme} />
      <Tooltip customTooltip={toolTip} placement={Placement.Right} offset={40} />
      <Axis
        id={htmlID('left')}
        position={Position.Left}
        title="Code Lines"
        showOverlappingLabels
        tickFormat={formatNumber}
      />
      <Axis id="bottom" title="Data Capture Date" position={Position.Bottom} tickFormat={tickFormat} />
      <LineSeries
        id={htmlID('lines')}
        xScaleType={ScaleType.Ordinal}
        yScaleType={ScaleType.Linear}
        xAccessor="date"
        yAccessors={['codeLines']}
        data={dummyData}
        splitSeriesAccessors={['project']}
        pointStyleAccessor={(datum) => ({ radius: 2.75, strokeWidth: 6 })}
        lineSeriesStyle={{ line: { strokeWidth: 4, opacity: 0.2 } }}
        curve={CurveType.LINEAR}
      />
    </Chart>
  );
}

Environment Details

  1. Kibana Version: 8.12.0
  2. Elastic Charts Version: As included in Kibana 8.12.0
  3. Browser: All supported browsers
  4. OS: Linux

Additional Information

  1. This issue was not present in Kibana version 7.16.3.
  2. The same code worked correctly in version 7.16.3, with the pointStyleAccessor applying styles to all data points, including the first point of each series.

Suggested Fix
Investigate the changes between Kibana version 7.16.3 and 8.12.0 related to the @elastic/charts library, specifically focusing on the pointStyleAccessor implementation. Ensure that the pointStyleAccessor correctly applies styles to all data points, including the first point of each series. Consider adding a test case to validate this behavior in future releases.

Attachments
Screenshot of the line chart showing the issue in Kibana 8.12.0.
image

@nickofthyme
Copy link
Collaborator

Does #2277 relate to you issue? This was fixed in v63.1.0 where [email protected] was on @elastic/[email protected]. If not I can look deeper into the issue.

@mageshsankar
Copy link
Author

mageshsankar commented Jun 11, 2024

@nickofthyme I can still reproduce this issue in latest version v65.2.1, here is the code sandbox link

Can you suggest any work around solution?

@nickofthyme
Copy link
Collaborator

nickofthyme commented Jun 12, 2024

Ok I see the issue now. I opened #2464 to fix the issue.

As far as a workaround, that is not easy. You basically need to remove all isolated points as defined by this check...

function isIsolatedPoint(
index: number,
length: number,
yDefined: YDefinedFn,
prev?: DataSeriesDatum,
next?: DataSeriesDatum,
): boolean {
if (index === 0 && (isNil(next) || !yDefined(next, yAccessorForIsolatedPointCheck))) {
return true;
}
if (index === length - 1 && (isNil(prev) || !yDefined(prev, yAccessorForIsolatedPointCheck))) {
return true;
}
return (
(isNil(prev) || !yDefined(prev, yAccessorForIsolatedPointCheck)) &&
(isNil(next) || !yDefined(next, yAccessorForIsolatedPointCheck))
);
}

Which you could do by filling the missing data so that all x values across all split series value a non-nil y value, such as 0. This would then prevent application of the isolated point style overrides avoiding the problem. But this has unintended consequences such as connecting all the points with lines like shown below.

Zight Recording 2024-06-11 at 06 11 05 PM

That being said, I can backport the fix to as far as possible ~[email protected] if that would help. Let me know.

@mageshsankar
Copy link
Author

@nickofthyme Once you've completed the change, can we upgrade elastic-charts to the most recent version, which will resolve the issues while keeping the same Kibana version 8.12.0, or do we need to upgrade Kibana to a more recent version?

@nickofthyme
Copy link
Collaborator

We would need to upgrade the version of @elastic/charts in kibana, here.

@nickofthyme nickofthyme changed the title Line Chart Point Style Not Applying for First Point of Every Group in LineChart - Kibana Version 8.12.0 PointStyleAccessor not applying to isolated points Jun 13, 2024
@mageshsankar
Copy link
Author

mageshsankar commented Jun 19, 2024

@nickofthyme i can still reproduce same issue the with elastic-charts with version "65.2.0", can you check in the code sandbox link

@nickofthyme
Copy link
Collaborator

Hey @mageshsankar this was fixed in charts version v66.0.2. Did you need this in a specific kibana version? As of now it will be added to the latest kibana version when elastic/kibana#185906 is merged.

@mageshsankar
Copy link
Author

Still, I can reproduce the issue.
image

@nickofthyme
Copy link
Collaborator

nickofthyme commented Jun 20, 2024

@mageshsankar yes because the isolated point styles are still being assigned to those points but as of v66.0.2 you can override these PointStyles using the pointStyleAccessor, but you would need to override all of the styles that you want to change.

I see how this can be tedious so maybe we add a way to opt out of this isolated point styling.

@nickofthyme
Copy link
Collaborator

nickofthyme commented Jun 23, 2024

@mageshsankar based on your comments I've add the enabled option to disable the isolated point styles from being applied. See #2472. Changes are available in v66.0.4

@mageshsankar
Copy link
Author

@nickofthyme Which version of Kibana has this fix?

@nickofthyme
Copy link
Collaborator

These changes are in main as of elastic/kibana#186836, and will be released in kibana 8.15.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants