-
-
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
fix(radar): typing for custom functions #1089
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,16 +12,28 @@ import { OrdinalColorsInstruction, InheritedColorProp } from '@nivo/colors' | |
import { LegendProps } from '@nivo/legends' | ||
|
||
declare module '@nivo/radar' { | ||
type IndexByCustomFunctiono<D = any> = (datum: D) => string | number | ||
type GridLabelCustomFunction = (...args: any[]) => string | JSX.Element | ||
type CustomDotSymbol = (...args: any[]) => React.ReactNode | ||
type CustomDotLabel = (...args: any[]) => React.ReactNode | ||
type CustomFormatter = (...args: any[]) => React.ReactNode | ||
export type GridLabelProps = { | ||
id: string | ||
anchor: "start" | "middle" | "end" | ||
angle: number | ||
} | ||
export type DotSymbolProps = { | ||
size: number | ||
color: InheritedColorProp | ||
borderWidth: number | ||
borderColor: InheritedColorProp | ||
} | ||
|
||
type IndexByCustomFunction<D = any> = (datum: D) => string | number | ||
export type CustomGridLabel = React.FC<GridLabelProps> | ||
export type CustomDotSymbol = React.FC<DotSymbolProps> | ||
export type CustomDotLabel = (...args: any[]) => React.ReactNode | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, but I guess we can add this later, what you updated is already a good addition. |
||
export type CustomFormatter = (...args: any[]) => React.ReactNode | ||
|
||
interface CommonRadarProps<Datum = any> { | ||
data: object[] | ||
keys: (string | number)[] | ||
indexBy: number | string | IndexByCustomFunctiono<Datum> | ||
indexBy: number | string | IndexByCustomFunction<Datum> | ||
maxValue?: 'auto' | number | ||
|
||
margin?: Box | ||
|
@@ -33,7 +45,7 @@ declare module '@nivo/radar' { | |
|
||
gridLevels?: number | ||
gridShape?: 'circular' | 'linear' | ||
gridLabel?: GridLabelCustomFunction | ||
gridLabel?: CustomGridLabel | ||
gridLabelOffset?: number | ||
|
||
enableDots?: boolean | ||
|
@@ -55,7 +67,7 @@ declare module '@nivo/radar' { | |
isInteractive?: boolean | ||
tooltipFormat?: string | CustomFormatter | ||
|
||
legends: LegendProps[] | ||
legends?: LegendProps[] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't find anything in the docs about whether the legends are optional or not, but there are some examples in the storybook with no legends (https://nivo.rocks/storybook/?path=/story/radar--custom-label-component) and I am already using them without legends in the project I am working on, actually typescript threw me an error because of this one lol. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, they're not, and lacking doc :/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. They are optional. |
||
} | ||
|
||
export type RadarProps = CommonRadarProps & MotionProps | ||
|
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.
In the docs this must return an SVG element, I didn't want to strictly type that, maybe we can a have a discussion about this?
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.
As the implementation is SVG only, enforcing an SVG element would be good, but I think it's fine to use the generic
ReactNode
type.