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

feat(Bar): Add option to chose label position #2585

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

evasseure
Copy link

@evasseure evasseure commented May 3, 2024

This adds two new props to the bar charts:

  • labelPosition: defaults to 'middle', and allows to chose at which end of the bar the label should appear (start, middle, end)
  • labelOffset: defaults to 0, and allows to chose the vertical or horizontal offset of the label. This depends on if the chart is horizontal or vertical.

➡️ DEMO

This is related to #2579.

Option Result
Vertical - "end" Screenshot 2024-05-03 at 15 38 15
Vertical - "start" Screenshot 2024-05-03 at 15 38 59
Horizontal - "end" Screenshot 2024-05-03 at 15 39 45
Horizontal - "start" Screenshot 2024-05-03 at 15 39 27
Vertical - Stacked - "start" Screenshot 2024-05-03 at 15 40 56
Vertical - "end" - no offset Screenshot 2024-05-03 at 15 46 02
Vertical - "end" - reverse Screenshot 2024-05-03 at 15 50 02

Copy link

vercel bot commented May 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nivo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 7, 2024 9:36am

@plouc
Copy link
Owner

plouc commented May 3, 2024

I noticed that for the reversed horizontal mode, end labels still have a start text anchor, I think it should be mirrored, as for the position, I was also wondering if the offset should be applied negatively as well in this mode.

@plouc
Copy link
Owner

plouc commented May 3, 2024

But I should have started with: great work! 😅 You added tests, updated the website, stories... 👍🏻

@plouc
Copy link
Owner

plouc commented May 3, 2024

Inverting the effect of the offset could also be done for the vertical reversed mode.

@plouc plouc added the 📊 bar @nivo/bar package label May 3, 2024
@@ -230,6 +233,8 @@ export type BarCommonProps<RawDatum> = {

enableLabel: boolean
label: PropertyAccessor<ComputedDatum<RawDatum>, string>
labelPosition: 'start' | 'center' | 'end'
Copy link
Owner

Choose a reason for hiding this comment

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

I remembered that we have a similar property for legendsContinuousColorsLegendProps, titleAlign, which uses 'start' | 'middle' | 'end', maybe we could use the same for consistency, what do you think?

Copy link
Author

Choose a reason for hiding this comment

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

'start' | 'middle' | 'end' is used in quite a few places already, so it could make sense to factorize it a bit. Is there a specific way you would want to do that?

Copy link
Owner

Choose a reason for hiding this comment

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

For now, I think we can just use the same values, and handle the rest in another PR.

@evasseure
Copy link
Author

labels still have a start text anchor, I think it should be mirrored

Good idea, will update!

I was also wondering if the offset should be applied negatively as well in this mode.
...
Inverting the effect of the offset could also be done for the vertical reversed mode.

That's up to you, right now its consistent with the how the x and y axis work in svg and canvas, which could remove a bit of possible confusion, but we could also have the offset work relative to the base of the bar, if you think that's better.

@plouc
Copy link
Owner

plouc commented May 3, 2024

I was also wondering if the offset should be applied negatively as well in this mode.
...
Inverting the effect of the offset could also be done for the vertical reversed mode.

That's up to you, right now its consistent with the how the x and y axis work in svg and canvas, which could remove a bit of possible confusion, but we could also have the offset work relative to the base of the bar, if you think that's better.

I think it's best to mirror the offset as we're already mirroring the position.

@evasseure
Copy link
Author

evasseure commented May 7, 2024

I made it so the offset is based on the direction and start of the bar, so for a label positioned in the center, a negative offset will go towards the start of the bar, and a positive offset will go towards the end of the bar.
Is that what you had in mind?
@plouc

@evasseure
Copy link
Author

Hello @plouc! Did you have time to look at my changes?

@plouc
Copy link
Owner

plouc commented May 20, 2024

Hi @evasseure, I've been a bit busy, and it might be hard this week because of work, but I'll try to review asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📊 bar @nivo/bar package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants