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

Annotations not showing up in data table #12789

Closed
oysteinmoseng opened this issue Jan 15, 2020 · 6 comments
Closed

Annotations not showing up in data table #12789

oysteinmoseng opened this issue Jan 15, 2020 · 6 comments
Assignees

Comments

@oysteinmoseng
Copy link
Member

Expected behaviour

Annotations connected to a point should be displayable in data tables, perhaps with an option in the annotations (and the individual annotation) to turn the behavior on/off.

Actual behaviour

Annotations are ignored when building the data table.

Live demo with steps to reproduce

https://jsfiddle.net/4Lvc63e9/

@KacperMadej
Copy link

Is there a detailed design for the feature?

Each annotation can have multiple labels. Each label has a position that is set by x and y values (scaled from set axes) or taken from a point (set by point id) in the chart. It is a similar problem to multiple scatter type series with multiple points with the same x values used - how should the data be sorted by default and what control should be allowed from the public API level?

Should an annotation column be added to the data table after all series? or after each series, if the point from the given series is being annotated?

If there is no detailed specification for this I would like to recommend for the initial feature implementation:

  • annotations columns added after series columns,
  • add more columns if there are more labels for the same x/category,
  • use different annotation only to track order of columns correctly,
  • include includeInDataExport on each annotation object level and on each label level.

A basic POC for the visual output could be seen under a chart in a demo here: https://jsfiddle.net/BlackLabel/xc1p96qd/

@oysteinmoseng

@oysteinmoseng
Copy link
Member Author

@KacperMadej No spec from our side, but I think your proposal is sensible behavior. Alternatively a single column for annotations per series could be used, and the annotations could be concatenated. This would be similar to what we do for screen reader accessibility.

Regardless it will definitely be important that this is behavior that can be turned off/on. I like the idea of supporting the includeInDataExport option as well, similar to what we have for series. Having it per label might be overkill, I don't think we support it per point today.

@TorsteinHonsi Thoughts?

@TorsteinHonsi
Copy link
Collaborator

I don't know... As Kacper indicates, annotations are by nature unstructured. If they were structured, I think in most cases it would be better to use data labels. This also works well with data exporting: http://jsfiddle.net/highcharts/LLExL/14423/ .

@oysteinmoseng
Copy link
Member Author

Well, looking at our example demo for CSUN for example, doing something like this with data labels would be a lot more work for the chart creator: https://highcharts.github.io/highcharts-utils/samples/#gh/fa024da5f4/sample/highcharts/accessibility/accessible-annotations

In this case, all but one of the annotations are part of the structured data, and something that would make sense to export/import across systems.

I think we have a few obvious challenges here:

  • What to do with freestanding annotations
  • What to do when an annotation is linked to more than one data point
  • What to do when an annotation does not include text
  • What to do when a data point has multiple annotations linked to it

If we want to avoid having to come up with sensible default logic, one solution could be to document an API for accessing the annotations, and leaving it to the chart creator to format it in the data table (or elsewhere).

Otherwise I think the following would be sensible default behavior:

  • Freestanding annotations: Either ignore or place on separate data row.
  • Annotation is linked to more than one data point: Include in annotation column for each linked data point.
  • Annotation does not include text: Ignore.
  • Data point has multiple annotations linked to it: Either concatenate the texts, or add in separate columns.

@KacperMadej
Copy link

I have only a few comments for the suggested defaults:

  • Freestanding annotations

Freestanding labels might also be placed using pixel position in the plot area, so those might be added to new rows at the very bottom of the table as those won't have x nor y axis values assigned when compared to other data.
Freestanding linked to an axis could be placed accordingly to the data table sorting (so ascending by x).
If a user wants to ignore them then includeInDataExport could be used.
This way we could have both behaviors while the existing option is used.

  • includeInDataExport on label level

Yes, look like an overkill - this it not needed IMHO. Currently, labels could be grouped into a group in an annotation to set the same options to them. If the includeInDataExport will not be available on the label level then those groups will have to be doubled to reflect that one group is exported and another is not. This could be nicely presented in a demo to avoid any confusion.

  • Data point has multiple annotations linked to it: Either concatenate the texts, or add in separate columns.

A new option in exporting.csv could be used to allow a chart creator to decide.
Maybe concatenateAnnotations? Defaults to true?

As soon as all defaults will be selected we could start the implementation.

@oysteinmoseng
Copy link
Member Author

Agree with your points @KacperMadej. The option for concatenating the annotations should probably indicate that it affects points with multiple annotations somehow. Maybe something like concatenatePointAnnotations is clear enough?

@TorsteinHonsi thoughts?

@highsoft-bot highsoft-bot added this to To do in Development-Flow Mar 5, 2020
@madepiet madepiet moved this from To do to In progress in Development-Flow Mar 18, 2020
madepiet added a commit that referenced this issue Jun 19, 2020
@highsoft-bot highsoft-bot moved this from In progress to Pending Review in Development-Flow Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants