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.

Sign up for GitHub

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

Jump to bottom

fix(radar): typing for custom functions #1089

Merged
merged 3 commits into from
Oct 30, 2020
Merged

fix(radar): typing for custom functions #1089

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5eddfbc
fix(radar): typing for custom functions
AlaaZorkane Aug 5, 2020
3a52c62
fix(radar): linting
AlaaZorkane Aug 5, 2020
6f8fc90
Merge branch 'master' into patch-1
AlaaZorkane Oct 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 20 additions & 8 deletions packages/radar/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Contributor Author

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?

Copy link
Owner

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.

export type CustomDotSymbol = React.FC<DotSymbolProps>
export type CustomDotLabel = (...args: any[]) => React.ReactNode
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think these (CustomDotLabel and CustomFormatter) should be strictly typed too, I never used them and I honestly don't have enough knowledge about them to type them.

Copy link
Owner

Choose a reason for hiding this comment

The 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
Expand All @@ -33,7 +45,7 @@ declare module '@nivo/radar' {

gridLevels?: number
gridShape?: 'circular' | 'linear'
gridLabel?: GridLabelCustomFunction
gridLabel?: CustomGridLabel
gridLabelOffset?: number

enableDots?: boolean
Expand All @@ -55,7 +67,7 @@ declare module '@nivo/radar' {
isInteractive?: boolean
tooltipFormat?: string | CustomFormatter

legends: LegendProps[]
legends?: LegendProps[]
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, they're not, and lacking doc :/

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. They are optional.

role?: string
}

Expand Down