-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[feat: react-charts]: Donut annotations in V9 #35610
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
base: master
Are you sure you want to change the base?
Conversation
📊 Bundle size reportUnchanged fixtures
|
|
Pull request demo site: URL |
|
|
||
| const padding = context.viewportPadding; | ||
| const paddingLeft = | ||
| typeof padding?.left === 'number' && Number.isFinite(padding.left) && padding.left > 0 ? padding.left : 0; |
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.
reduce repeating code
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.
updated
| ? context.plotRect | ||
| : undefined; | ||
|
|
||
| const clampX = clampRect && typeof clampRect.x === 'number' && Number.isFinite(clampRect.x) ? clampRect.x : 0; |
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.
clampX clampY both values are very similar and repeating part of the code. See how we can reduce common code.
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.
updated
75c16d7 to
a63a35d
Compare
a63a35d to
0a02ca0
Compare
…ers/srmukher/v9_donut_anno
0b97f45 to
3b7be6e
Compare
| return React.useMemo<UseDonutAnnotationLayoutResult>(() => { | ||
| const annotationList = annotations ?? EMPTY_ANNOTATIONS; | ||
| const layout = resolveDonutViewportLayout(annotationList, width, height, hideLabels); | ||
| const fallbackSvgWidth = typeof width === 'number' && Number.isFinite(width) ? Math.max(width, 0) : 0; |
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.
Is any of the code in this file sharable with chart annotation layer.
| const desiredWidth = Math.max(resolvedSvgWidth + totalHorizontalPadding, 0); | ||
| const desiredHeight = Math.max(resolvedSvgHeight + totalVerticalPadding, 0); | ||
|
|
||
| if (typeof width === 'number' && Number.isFinite(width)) { |
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.

No description provided.