Skip to content

Introduce ability to annotate a single bar#132

Merged
kurkle merged 11 commits intochartjs:v2from
renuo:master
Oct 23, 2020
Merged

Introduce ability to annotate a single bar#132
kurkle merged 11 commits intochartjs:v2from
renuo:master

Conversation

@lukasbischof
Copy link

@lukasbischof lukasbischof commented Sep 24, 2018

This PR introduces the ability to annotate only a single bar (implementing #101):
image

It also supports horizontal bars:
image

The PR adds two new options to the line annotation:

  • onlyForDataIndex which specifies the data index which should be annotated
  • linePadding which adds a customisable padding to the start and end of the line
  • datasetIndex which specifies on which dataset the data should be annotated

@sammeel
Copy link

sammeel commented Oct 3, 2018

Hi, I came across your answer as I was searching for the same thing. I noticed that your label is always aligned to the graph, not to the line. Is this the intended behavior? For a line across the whole chart, this is the intended behavior, but I would have expected the label to be aligned to line.

For example a center aligned label for dataindex 0 gives me this chart:
image

I also noticed some variables are being set on the window object, which is probably left from debugging

@lukasbischof
Copy link
Author

Hi, thanks for your input. However, I'm not able to reproduce the problem. Centering should actually be supported and if I try to reproduce, it yields this result:
image

Could you point out how exactly you received this behaviour and where variables on window are set?

@sammeel
Copy link

sammeel commented Oct 4, 2018

mmm, I must have done something wrong. I just tried reproducing it again, and now it works fine.

My appologies :)

EDIT: found out why. copied the wrong version while going through the commits

@Alex-Sichkar
Copy link

Nice feature, thanks! Is there a way to remove line padding at all? If I set linePadding to 0 some padding still there.

@lukasbischof
Copy link
Author

@ygwain Actually it should be already like that. Could you provide your code to reproduce?

@Alex-Sichkar
Copy link

Alex-Sichkar commented Dec 8, 2018

@benmccann benmccann closed this Dec 14, 2018
@benmccann benmccann reopened this Dec 14, 2018
@benmccann
Copy link
Collaborator

@lukasbischof this PR is currently failing our style guidelines. If you rebase against master you can run gulp lint locally to ensure that everything passes

@lukasbischof
Copy link
Author

@ygwain Should be fixed by now. I also inverted the padding, because I think it may be more natural if the padding is an inset spacing considering the behaviour applied in CSS.

@faraonkicu
Copy link

Hello, when you are planning to merge this feature?

@Obleo33
Copy link

Obleo33 commented Jun 13, 2019

I would love to be able to use this feature. Please merge soon.

@benmccann
Copy link
Collaborator

benmccann commented Jun 24, 2019

I went back and read through the thread at #141 again. The author of that PR and I had decided a hoverColor option would be a better solution for what he's trying to accomplish. That's fairly different from the original problem being discussed, so sorry for the confusion. Let's consider these PRs separately

I'm going to delete the comments discussing this PR vs #141 to keep the thread clean since they no longer seem relevant

@chartjs chartjs deleted a comment from lukasbischof Jun 24, 2019
@chartjs chartjs deleted a comment from Obleo33 Jun 24, 2019
Copy link
Collaborator

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is too specific to the bar chart. Could you pass a start and end tick and then use scale.getPixelForTick instead of using the bar controller and model?

kurkle
kurkle previously approved these changes Oct 23, 2020
kurkle
kurkle previously approved these changes Oct 23, 2020
@stockiNail
Copy link
Collaborator

@kurkle I have seen that these new features are not available on version 1 of the plugin.
Make sense to think implementing them in version1 as well?

@kurkle
Copy link
Member

kurkle commented Nov 26, 2021

There is a discussion about this: #484

I would not like to add bar specific configuration when its quite easily achieved already.

@kurkle
Copy link
Member

kurkle commented Nov 26, 2021

But samples would be good.

@stockiNail
Copy link
Collaborator

@kurkle thank you ! I haven't seen the discussion.

@stockiNail
Copy link
Collaborator

But samples would be good.

I think makes sense. Do you already a list of additional samples (apart this one) you'd like to have in the samples doc section?

@kurkle
Copy link
Member

kurkle commented Dec 4, 2021

But samples would be good.

I think makes sense. Do you already a list of additional samples (apart this one) you'd like to have in the samples doc section?

No, I don't have a list

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants