-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I noticed that for the reversed horizontal mode, end labels still have a |
But I should have started with: great work! 😅 You added tests, updated the website, stories... 👍🏻 |
Inverting the effect of the offset could also be done for the vertical reversed mode. |
@@ -230,6 +233,8 @@ export type BarCommonProps<RawDatum> = { | |||
|
|||
enableLabel: boolean | |||
label: PropertyAccessor<ComputedDatum<RawDatum>, string> | |||
labelPosition: 'start' | 'center' | 'end' |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Good idea, will update!
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. |
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. |
Hello @plouc! Did you have time to look at my changes? |
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. |
This adds two new props to the bar charts:
➡️ DEMO
This is related to #2579.